diff --git a/README.md b/README.md index 5841de53..93657eea 100644 --- a/README.md +++ b/README.md @@ -258,8 +258,8 @@ sysml2tools help [lint|render|query []|export] | `--element `, `-e ` | Qualified name of the target element; required for every verb except `list`/`find` | | `--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: follow `connect`/`bind` edges undirected (1 hop); off = not followed | +| `--walk-depth <#>` | Max impact-walk depth (`impact` only); bounds all kinds equally; unlimited if omitted | +| `--include-connections` | `impact` only: follow `connect`/`bind` edges undirected; bounded by `--walk-depth` | | `--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 8d209c62..3b98d4aa 100644 --- a/docs/design/sysml2-tools-core/query.md +++ b/docs/design/sysml2-tools-core/query.md @@ -112,17 +112,16 @@ 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 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: + by `WalkDepth`, with a visited-set guard preventing infinite loops on cyclic graphs. Each + frontier item is a bare qualified name, 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 + emits one entry per newly-reached source. 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. + of its owning declaration, and without `ViaQualifiedName`. 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 @@ -138,40 +137,47 @@ flowchart TD 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 - `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. + endpoint itself first and returns it unchanged when it is already a declared qualified name + **and is not an endpoint-only construct**, so a connector naming a sibling part directly + (`connect alpha to beta;`) reports `beta` rather than the enclosing definition that also owns + the subject. Trailing `::` segments are stripped, one at a time, for endpoints absent from + `Declarations` — typically ports inherited through a typed usage — and equally for endpoints + that *are* declared but are classified endpoint-only by `IsEndpointOnlyDeclaration`, so a port + endpoint such as `System::hub::J1` is attributed to `System::hub` under either modeling style. + `IsEndpointOnlyDeclaration` classifies structurally, from `SysmlFeatureNode.FeatureKeyword` + (`port`) and `SysmlDefinitionNode.DefinitionKeyword` (`port def`), never from element names. + Skipping declared ports is required because whether a port endpoint is itself a key in + `Declarations` is an artifact of modeling style rather than meaning: a port declared on a + definition and reached through a typed usage (`part def Hub { port J1; } ... part hub : Hub;`) + yields the undeclared path `System::hub::J1`, whereas the same port declared inline on the + usage (`part hub { port J1; }`) yields a path that *is* a declaration key. Stopping at the + first declared ancestor would therefore answer a `part`-level question with a port for the + second spelling only — and, because the reported name is also the name enqueued onto the next + frontier, it would dead-end the walk at the port rather than continuing through the owning + part. Probing the endpoint before stripping is what keeps `impact` and `connections` in + agreement about the same connector's topology; note that `connections` reports raw endpoints + and performs no roll-up of its own, so this policy is `impact`-specific and has a single + caller. 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 suitable declared ancestor is reported unchanged rather than + dropped. + One uniform bound is applied. `WalkDepth` bounds the walk in relationships, counting a + connector relationship exactly like a reference relationship, and `null` means unlimited for + both. `IncludeConnections` selects **which edges exist in the graph**, never how far the walk + goes. A single shared ordinal `visited` set spans both helpers and is seeded with the subject + so the subject never reports itself; a name is admitted to the next frontier, and emits an + entry, only on the call to `visited.Add` that first inserts it. + That membership-only guard is sufficient because the traversal is uniform-cost and + level-synchronous: both helpers append only to the *next* level's frontier, and neither can + reach an element without advancing a level, so there are no zero-cost edges. `IsSelfOrNestedUnder` + and `RollUpToNearestDeclaration` are pure string operations that normalize the endpoints of a + *single* connector rather than traversing a second edge, so they introduce no free hops. In a + uniform-cost level-order search first arrival is by construction the shortest relationship + distance, so `Depth` — which is simply the breadth-first level — is exactly that distance, and + a later arrival can never be cheaper. `Relation`, `Detail`, `Notes`, and `ViaQualifiedName` + likewise describe the shortest path. Termination is guaranteed because each name is admitted + to `visited` at most once, so total enqueues are bounded by the number of declarations; this + holds on cyclic connector topologies with no bound in force at all. 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. @@ -302,9 +308,9 @@ flowchart TD 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 made connector traversal an unbounded, direction-sensitive + previously followed them as ordinary incoming reference edges: attributed to the raw + nested-port endpoint rather than its owning declaration, and + without any relation metadata. That made connector traversal a 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 @@ -315,13 +321,18 @@ flowchart TD - `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. + legitimately diverge once connectors receive first-class, rolled-up 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 - `WalkDepth`. +- **The connection-aware `impact` result changed relative to the `0.2.0-beta.1` tag.** Connector + relationships were previously bounded to one hop per traversal path unless `WalkDepth` was + supplied, which meant "unspecified" silently meant "one" for connectors while meaning + "unlimited" for references. `WalkDepth` is now the single depth control and applies uniformly + to every edge kind, so `--include-connections` with no `--walk-depth` reaches an entire + connector chain rather than one hop. Real connector graphs are dense meshes, so on a + hub-and-spoke assembly an unbounded connection-aware walk can still reach the whole assembly — + that is the reason a user may want to pass `--walk-depth`, but it is guidance on when to bound + rather than a description of the default. - `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 @@ -339,9 +350,12 @@ flowchart TD | 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-ImpactConnectionPortSkip | `RollUpToNearestDeclaration`; `IsEndpointOnlyDeclaration` | +| SysML2Tools-Core-Query-ImpactConnectionContainerDecline | `RollUpToNearestDeclaration`; `IsSelfOrNestedUnder` | +| SysML2Tools-Core-Query-ImpactConnectionPortContinuation | `CollectImpactConnections` frontier append | +| SysML2Tools-Core-Query-ImpactUniformDepth | `QueryEngine.Impact` breadth-first level loop; `WalkDepth` | +| SysML2Tools-Core-Query-ImpactConnectionCycles | `QueryEngine.Impact` `visited` guard | +| SysML2Tools-Core-Query-ImpactHopMinimality | `QueryEngine.Impact`; `CollectImpactConnections` | | SysML2Tools-Core-Query-EntryTraversalMetadata | `QueryResultEntry.Depth`/`Relation`/`ViaQualifiedName` | | SysML2Tools-Core-Query-EntryMetadataJsonOmission | `JsonIgnore(WhenWritingNull)`; `QueryResultRenderer.RenderJson` | | SysML2Tools-Core-Query-EntryRelationSerialization | `JsonStringEnumConverter`; `QueryResultSerializerContext` | diff --git a/docs/design/sysml2-tools-tool.md b/docs/design/sysml2-tools-tool.md index d35ac0d8..15127674 100644 --- a/docs/design/sysml2-tools-tool.md +++ b/docs/design/sysml2-tools-tool.md @@ -68,7 +68,8 @@ self-testing, and uses `PathHelpers` to construct safe temporary file paths. subcommand (equivalent to `--help`/`-?`/`-h`) that prints general or per-subcommand usage text. Accepts `query [options] ` (12 verbs; `--element`/`-e`, `--format`, `--direction`, `--kind`, `--name`, `--include-stdlib`, `--walk-depth` (bounded-traversal depth, - meaningful only for the `impact` verb) options). Accepts + meaningful only for the `impact` verb, applied uniformly to every relationship kind and + unlimited when omitted) options). Accepts `export [options] ` (`--format json|jsonl`, `--output `, `--include-stdlib` options) which dumps the resolved semantic model (declarations, edges, diagnostics) as JSON or JSON Lines. Returns diff --git a/docs/design/sysml2-tools-tool/query.md b/docs/design/sysml2-tools-tool/query.md index 1b76cbd9..cf9e4212 100644 --- a/docs/design/sysml2-tools-tool/query.md +++ b/docs/design/sysml2-tools-tool/query.md @@ -98,8 +98,8 @@ flowchart TD ```text --include-connections Also follow connect/bind edges, undirected - (default: reference edges only). Connector - hops are bounded by --walk-depth, else 1. + (default: reference edges only). Distance + is bounded by --walk-depth like any edge. ``` All four keys live in `QueryStrings.resx` with matching `QueryStrings` accessor properties, as diff --git a/docs/reqstream/sysml2-tools-core/query.yaml b/docs/reqstream/sysml2-tools-core/query.yaml index 278ed846..0868702b 100644 --- a/docs/reqstream/sysml2-tools-core/query.yaml +++ b/docs/reqstream/sysml2-tools-core/query.yaml @@ -102,18 +102,18 @@ sections: "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. + `IncludeConnections` is explicitly supplied. The bound is `WalkDepth`, unlimited when + it is not supplied, and applies uniformly to every relationship kind the walk traverses. 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: 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. + every connector as a plain incoming reference — directed rather than undirected, and + attributed to the raw nested-port endpoint rather than its owning declaration. That was + a defect: connector traversal became a direction-sensitive side effect of the reference + walk instead of the 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: - Impact_DepthOne_OnlyReachesDirectReferences - Impact_Unbounded_ReachesTransitiveClosure @@ -124,16 +124,18 @@ sections: - 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, 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. + resolved `Connect` and `Binding` connector relationships, in addition to reference + relationships and subject to the same depth bound, without regard to their 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 reverse-only reference closure reports nothing for a part joined to the rest of the 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. + because it broadens the answer beyond the reference-only default. The option decides + only which relationships are part of the graph, never how far the walk goes. tests: - Impact_IncludeConnections_ReachesConnectedSiblingPart - Impact_IncludeConnections_QueriedFromFarEndpoint_ReachesOriginatingPart @@ -159,7 +161,7 @@ sections: 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. + when the endpoint is already a declared element that is a valid impact subject. 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 @@ -172,54 +174,123 @@ sections: - Impact_IncludeConnections_DeclaredEndpointConnector_ReportsSiblingPartNotOwningDefinition - Impact_IncludeConnections_SourceSidePortEndpoint_ReportsOwnerNotRawPort - - id: SysML2Tools-Core-Query-ImpactConnectionHopBound + - id: SysML2Tools-Core-Query-ImpactConnectionPortSkip 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. + The `impact` verb shall roll a port endpoint up to the nearest enclosing non-port + declaration that does not contain the connector's opposite endpoint, regardless of + whether the port endpoint is itself a declared element. 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. + A port has no behavior or state of its own to be impacted; it is the attachment point + through which its owning part is impacted. Whether a port endpoint is itself a declared + element is an artifact of modeling style — a port declared on a part definition and + reached through a typed usage produces an undeclared endpoint path, while the same port + declared inline on a part usage produces a declared one — and both styles are legal, so + an answer that depends on which was used is not trustworthy. Reporting a port also + truncates the analysis, because the reported element is the one the walk continues + from, so everything wired beyond that port is silently omitted from the answer. tests: - - Impact_IncludeConnections_NoWalkDepth_BoundsConnectionHopsToOne + - Impact_IncludeConnections_InlineUsagePortEndpoint_ReportsOwningPartUsage + - Impact_IncludeConnections_PortDeclarationStyles_ReportNoPortEntries + - Impact_IncludeConnections_PortEndpoints_RollUpToOwningPartUsage + + - id: SysML2Tools-Core-Query-ImpactConnectionContainerDecline + title: >- + The `impact` verb shall not roll a connector endpoint up to a declaration that contains + the connector's opposite endpoint, reporting the endpoint itself instead. + justification: | + A declaration that contains the opposite endpoint is a container of the element being + expanded, not a peer of it, so reporting it would attribute containment as a + connection. It would also curtail the analysis: because a connector is only followed + when exactly one of its ends lies within the element being expanded, every connector + interior to that container is discarded once the container itself is expanded, hiding + elements genuinely wired to the subject. This arises for a proxy port declared on an + enclosing definition, where the nearest non-port owner is the definition itself, and + for a port declared directly in a package. Retaining the endpoint keeps the real shared + attachment point in the answer and keeps its neighbors reachable. + tests: + - Impact_IncludeConnections_ProxyPortOnOwningDefinition_ReportsPortNotContainer + - Impact_IncludeConnections_ProxyPortOnOwningDefinition_ReachesSiblingAcrossPort + - Impact_IncludeConnections_PackageScopedPort_DoesNotTreatPackageAsConnected + + - id: SysML2Tools-Core-Query-ImpactConnectionPortContinuation + title: >- + The `impact` verb shall continue its traversal from the declaration a port endpoint + rolled up to, so elements connected beyond that declaration are reached at their true + relationship distance. + justification: | + Reporting and traversing must agree: if the walk advanced from the raw port instead of + the owning part, a hub wired to several parts would yield exactly one row and every + part on the far side of the hub would be unreachable at any depth. That is a silent + under-report of impact, the most dangerous failure mode for a change-impact analysis, + because the caller cannot tell an empty answer from a truncated one. + tests: + - Impact_IncludeConnections_InlineUsagePortEndpoint_ContinuesTraversalBeyondPortOwner + - Impact_IncludeConnections_HubTopology_SpokeReachesOtherSpokeAtDepthTwo + + - id: SysML2Tools-Core-Query-ImpactUniformDepth + title: >- + The `impact` verb shall apply `WalkDepth` uniformly to every relationship it traverses, + counting each reference relationship and each connector relationship as one unit of + depth, and shall impose no depth bound when `WalkDepth` is not supplied. + justification: | + Users reason about proximity to the subject — "what is within three relationships of + this?" — and not about per-relationship-class allowances; nobody asks for two reference + hops and one connector hop. The class of each relationship is already reported on every + result entry, so a caller who cares about it can group or filter on it after the fact, + which is a presentation concern rather than a traversal budget. + + A class-specific budget also made the reported depth disagree with the true distance + from the subject, because the two counters advanced at different rates: an element could + be reached at a larger traversal depth but a smaller connector cost, so the depth shown + on a row was not the number of relationships separating it from the subject. One budget + removes that disagreement by construction. + tests: + - Impact_IncludeConnections_NoWalkDepth_ReachesEntireConnectorChain + - Impact_IncludeConnections_LongChain_NoWalkDepth_ReachesFarEnd - Impact_IncludeConnections_WalkDepthTwo_ReachesSecondConnectionHop - - Impact_IncludeConnections_ChainedDeclaredConnectors_StopsAtOneConnectorHop - Impact_IncludeConnections_ChainedDeclaredConnectors_WalkDepthTwo_ReachesSecondHopOnly + - Impact_IncludeConnections_NoWalkDepth_ReachesEntireConnectorChainAtShortestDistance + - Impact_IncludeConnections_WalkDepthThree_BoundsConnectorChainToThreeHops + - Impact_IncludeConnections_MixedPaths_WalkDepthBoundsBothEdgeClassesIdentically + - Impact_Summary_UnboundedWithConnections_OmitsHopClause - id: SysML2Tools-Core-Query-ImpactConnectionCycles title: >- The `impact` verb shall terminate and return a result for models whose connector - relationships form cycles. + relationships form cycles, including when no depth bound is supplied. 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. + user-visible obligation in its own right and not merely an implementation nicety. Since + a depth that is not supplied is unlimited for connector relationships as well as + reference ones, + termination must hold with no bound in force at all and cannot rest on a bound to + provide it. tests: - Impact_IncludeConnections_CyclicConnections_TerminatesWithoutDuplicates + - Impact_IncludeConnections_RingTopology_NoDepthBound_Terminates + - Impact_IncludeConnections_RingTopology_NoDepthBound_TerminatesWithShortestDistances - id: SysML2Tools-Core-Query-ImpactHopMinimality title: >- - 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. + The `impact` verb shall report each impacted element exactly once, at its shortest + relationship distance from the subject and the relation of the path achieving that + distance, independently of the order in which paths are discovered. justification: | - 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. + A change-impact answer that varies with traversal order cannot be relied on or + reproduced, so the reported set — and the distance shown against each row — must be a + function of the model and the requested bound alone. Reporting the shortest distance is + what makes that number meaningful to a reader: "depth 2" must mean "two relationships + away", not "two levels into whichever route the walk happened to take first". + + Reporting each element exactly once matters equally: an element reachable by several + paths is still one element, and duplicate rows would both overstate the size of the + blast radius and force callers to de-duplicate an answer that should already be a set. tests: - - Impact_IncludeConnections_CheaperReferencePath_ReExpandsConnectorsFromReReachedElement - - Impact_IncludeConnections_ReReachedAtLowerHopCount_KeepsFirstArrivalDepthAndAttribution + - Impact_IncludeConnections_ReachedByTwoPaths_ReportsShortestDistance + - Impact_IncludeConnections_ReachableByReferenceAndConnector_ReportedOnceAtShorterDistance + - Impact_IncludeConnections_HubTopology_SpokeReachesOtherSpokeAtDepthTwo - Impact_IncludeConnections_SourceSidePortEndpoint_ProducesExactlyOneEntry - id: SysML2Tools-Core-Query-EntryTraversalMetadata diff --git a/docs/user_guide/introduction.md b/docs/user_guide/introduction.md index d6d4fe57..335bf1be 100644 --- a/docs/user_guide/introduction.md +++ b/docs/user_guide/introduction.md @@ -474,15 +474,26 @@ because a connector is not a reference. > **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 +> `connect b to a;` or `connect hub.J1 to motorA;`). Those connectors 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 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 -> owning part and within the documented hop bound. +> owning part. +> +> **Upgrade note — connection-aware depth changed since `0.2.0-beta.1`.** In `0.2.0-beta.1`, +> `--include-connections` followed at most **one** connector per traversal path unless you +> supplied `--walk-depth`, so leaving `--walk-depth` off silently meant "one hop" for connectors +> while meaning "unlimited" for references. `--walk-depth` is now the single depth control and +> counts every relationship equally, so `--include-connections` with no `--walk-depth` now +> follows a connector chain all the way to its end. A command you have not changed may therefore +> report **more** rows than it did in `0.2.0-beta.1`. There is no longer an exact equivalent of +> the old behavior, because the one-hop bound applied to connectors only while references stayed +> unlimited; pass the proximity you actually want instead, such as `--walk-depth 1` for immediate +> neighbors of every kind. Adding `--include-connections` makes `impact` follow `connect` and `bind` relationships as well. Three rules apply: @@ -499,11 +510,18 @@ well. Three rules apply: 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. +- **One uniform depth.** `--walk-depth ` counts every relationship equally — a `connect` + hop, a `bind` hop, a specialization, and a typing reference each cost exactly one. So + `--walk-depth 3` means "everything within 3 relationships of this element", whatever kinds of + relationship those turn out to be. Omitting `--walk-depth` means *unlimited* for connectors + exactly as it always has for references. `--include-connections` decides only *which* + relationships exist in the graph, never how far the walk goes. + +Because real models connect many parts to a shared hub or bus, an unlimited connection-aware +walk on such an assembly can reach the whole assembly. That is worth knowing when you want +proximity rather than reachability: pass `--walk-depth ` to ask for the neighborhood you +actually care about. Every reported element carries the distance at which it was found, and +that distance is always the *shortest* number of relationships separating it from your subject. 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, @@ -515,15 +533,6 @@ 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 | @@ -558,7 +567,7 @@ the same order, so either format can be relied on for automated comparisons. | `--element `, `-e ` | Qualified name of the target element; required for every verb except `list`/`find` | | `--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) | +| `--walk-depth <#>` | Max impact-walk depth (`impact` only); all relationship kinds count equally, unlimited default | | `--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) | diff --git a/docs/verification/sysml2-tools-core/query.md b/docs/verification/sysml2-tools-core/query.md index 57a6e560..0fccdc06 100644 --- a/docs/verification/sysml2-tools-core/query.md +++ b/docs/verification/sysml2-tools-core/query.md @@ -76,17 +76,23 @@ scenarios listed under "Test Scenarios (Tool Test Project)" below run via `dotne 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. The bound holds - for connector chains whose endpoints are declared part usages, not only for port endpoints. +- One uniform depth bounds the walk: `WalkDepth` counts every traversed relationship equally, + so a `--walk-depth N` cuts a connector chain and a reference chain at exactly the same + distance, and a `WalkDepth` that is not supplied leaves connector traversal unlimited just as + it leaves + reference traversal unlimited. A connector chain is therefore reached end to end when no + `WalkDepth` is supplied. This 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. -- 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. +- Every reported element is reported exactly once, at its shortest relationship distance from + the subject and with the relation of the path achieving that distance, so the reported set and + the depth shown against each row do not depend on the order in which paths are discovered. + This holds when an element is reachable by both a reference path and a connector path, + whichever of the two is shorter. +- A cyclic connector topology terminates and reports each impacted element exactly once, with + no depth bound supplied at all. - `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 @@ -242,16 +248,77 @@ whole-list `Assert.Single` overload is used deliberately: the predicate overload 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. +**`Impact_IncludeConnections_ReachedByTwoPaths_ReportsShortestDistance`**: +Uses the minimum-hop fixture, in which `b` is reachable from `s` over `connect b to s` in one +relationship and over the subsetting chain `b :> s2 :> s` in two. Verifies that `b` appears +exactly once at `Depth` 1 with `Relation` of `SysmlEdgeKind.Connect` — the shorter of the two +paths — and that `z`, one further connector beyond `b`, is reported with `Relation` of +`SysmlEdgeKind.Connect` at `Depth` **2**. Depth 2 is the true shortest distance: `s` → `b` +(`connect b to s`) → `z` (`connect z to b`) is two relationships. The previous expectation of 3 +was an artifact of the separate connector budget, which delayed `b`'s connector expansion until +`b` had been re-reached over the reference detour. + +**`Impact_IncludeConnections_NoWalkDepth_ReachesEntireConnectorChainAtShortestDistance`**: Uses +the six-part, five-connector chain fixture and queries from `a` with `IncludeConnections` set +and no `WalkDepth`. Verifies that exactly five entries are produced and that `b` through `f` +carry `Depth` 1 through 5 respectively, each with `Relation` of `SysmlEdgeKind.Connect` — +proving connector traversal is unlimited by default and that reported depth is true hop +distance. + +**`Impact_IncludeConnections_WalkDepthThree_BoundsConnectorChainToThreeHops`**: Uses the same +chain fixture with `WalkDepth` of 3. Verifies that exactly `b`, `c`, and `d` are reported and +that neither `e` nor `f` is, proving that `--walk-depth 3` means "everything within three +relationships" for connector edges exactly as it does for reference edges. + +**`Impact_IncludeConnections_MixedPaths_WalkDepthBoundsBothEdgeClassesIdentically`**: Uses the +mixed fixture, which places a three-link subsetting chain and a three-hop connector chain on the +same subject, with `WalkDepth` of 2. Verifies that `ref1`/`ref2` and `con1`/`con2` are all +reported at `Depth` 1 and 2, while `ref3` and `con3` are both absent — proving a single budget +cuts both relationship classes at identical distance. + +**`Impact_IncludeConnections_ReachableByReferenceAndConnector_ReportedOnceAtShorterDistance`**: +Uses the dual-path fixture, in which `viaConnector` is one connector but two references away +and `viaReference` is one reference but two connectors away. Verifies that each appears exactly +once at `Depth` 1, `viaConnector` with `Relation` of `SysmlEdgeKind.Connect` and `viaReference` +with `Relation` of `SysmlEdgeKind.Supertype` — proving the shorter path wins irrespective of +which relationship class achieves it, and that the reported relation names the winning path. + +**`Impact_IncludeConnections_RingTopology_NoDepthBound_TerminatesWithShortestDistances`**: Uses +the four-part connector ring and queries from `r1` with no depth bound. Verifies that the walk +terminates, that exactly three entries are produced, and that `r2` and `r4` are at `Depth` 1 +while `r3` — reachable around either side of the ring — is at `Depth` 2, proving termination +rests on the visited-set guard rather than on any bound. + +**`Impact_IncludeConnections_HubTopology_SpokeReachesOtherSpokeAtDepthTwo`**: Uses the +connected-parts hub fixture and queries from spoke `motorA` with no depth bound. Verifies the +shared `hub` at `Depth` 1 and the sibling spoke `motorB` at `Depth` 2, proving a second spoke +reached through the hub is reported rather than suppressed by an exhausted per-class budget. + +**`Impact_IncludeConnections_InlineUsagePortEndpoint_ReportsOwningPartUsage`**: Regression for a +far-endpoint roll-up that stopped at the first declared ancestor. Uses the inline-usage-ports +fixture, whose ports are declared directly on the `hub` part usage (`part hub { port J1; port +J2; }`) rather than on a part definition reached through a typed usage. That shape — and only +that shape — makes the connector endpoint path `M::S::hub::J1` itself a key in +`SysmlWorkspace.Declarations`, which is why every pre-existing connector fixture, all of which +use the definition-side style, left the defect undetected. Queries `M::S::motorA` with +`IncludeConnections` set and verifies the entry is `M::S::hub` at `Depth` 1 with `Relation` of +`SysmlEdgeKind.Connect` and `ViaQualifiedName` of `M::S::hub::J1`, and that no entry names the +raw port. + +**`Impact_IncludeConnections_InlineUsagePortEndpoint_ContinuesTraversalBeyondPortOwner`**: +Regression proving the roll-up result is what the walk advances from, not merely what is +reported. Uses the same inline-usage-ports fixture and subject and verifies that `M::S::motorB` +— on the far side of the hub — is reported at `Depth` 2 with `Relation` of +`SysmlEdgeKind.Connect` and a null `ViaQualifiedName`. Before the fix the frontier held the port +rather than the hub, so `motorB` was unreachable at any depth; asserting reachability *and* the +exact depth is what distinguishes a continued walk from a coincidentally longer one. + +**`Impact_IncludeConnections_PortDeclarationStyles_ReportNoPortEntries`**: Negative assertion run +as a `[Theory]` across **both** port-declaration styles — the inline-usage-ports fixture and the +definition-side connected-parts fixture. Verifies the result is non-empty (so the assertion +cannot pass vacuously) and that no entry has a `Kind` of `port`. Varying the model shape rather +than the assertion is the point: the two rows exercise structurally different roll-up paths that +must produce the same class of answer. **`WriteMarkdown_HappyPath_MatchesRendererOutput`**: Verifies that `WriteMarkdown` writes the same Markdown text produced by `QueryResultRenderer.RenderMarkdown`. @@ -372,8 +439,8 @@ scenario is the lock on the corrected default: connector edge kinds are excluded 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 -bound in its summary. +`--include-connections` reaches the connected hub part usage and reports connection inclusion in +its summary. **`Impact_IncludeConnections_QueriedFromFarEndpoint_ReachesOriginatingPart`**: Undirected proof — verifies that querying from the hub (the endpoint named second in every connector) reaches @@ -383,23 +450,40 @@ 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`**: Uses the +**`Impact_IncludeConnections_NoWalkDepth_ReachesEntireConnectorChain`**: 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. +`--include-connections`; verifies exactly `3 element(s) transitively impacted`, that the `b`, +`c`, and `d` rows are all present, and that the summary reads `including connections`. 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 be attributed twice. 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 -the raised bound in the summary. +`--walk-depth 2` bounds the walk to two relationships of any kind, reaching the second motor +through the hub, and reports connection inclusion 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. +with no depth bound at all; verifies the query terminates and reports the hub exactly once. + +**`Impact_IncludeConnections_RingTopology_NoDepthBound_Terminates`**: Uses the +`QueryTestFixtures.RingConnectors` four-part ring and queries `Model::System::r1` with +`--include-connections` and no depth bound; verifies exactly three impacted elements and that +`r2`, `r3`, and `r4` are each reported, proving an unbounded walk terminates on a cycle at the +CLI level. + +**`Impact_IncludeConnections_LongChain_NoWalkDepth_ReachesFarEnd`**: Uses the +`QueryTestFixtures.LongDeclaredConnectorChain` five-connector chain and queries +`Model::System::a` with `--include-connections`; verifies exactly five impacted elements and +that the far end `f` is among them, distinguishing an unlimited default from a merely generous +one. + +**`Impact_Summary_UnboundedWithConnections_OmitsHopClause`**: Verifies the exact unbounded +connection-aware summary literal +`3 element(s) transitively impacted by a change to 'Model::System::a', including connections.` +and that the output contains no `connection hops` substring, locking the two-case summary +wording. **`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 @@ -412,13 +496,6 @@ 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 @@ -426,8 +503,8 @@ 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. +not, and the summary reads `including connections` — proving the walk is cut at exactly the +requested distance. **`Impact_IncludeConnections_SourceSidePortEndpoint_ReportsOwnerNotRawPort`**: Uses the `QueryTestFixtures.SourceSidePortConnector` fixture (`connect hub.J1 to motorA;`, the nested port @@ -440,15 +517,14 @@ 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`**: +**`Impact_IncludeConnections_ReachedByTwoPaths_ReportsShortestDistance`**: 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. +connector in one relationship), `s2` (reached over a reference edge), and `z`, one further +connector beyond `b`. `z` is reported at depth 2, the true shortest distance +`s` → `b` → `z`, and the rendered output is checked for that depth text. Under the previous +separate connector budget `z` appeared at depth 3, because `b` could not expand its connectors +until it had been re-reached over the longer reference detour. **`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 3340d0bb..bc559e3d 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs @@ -43,10 +43,10 @@ public static class QueryEngine /// 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 . + /// time as a plain incoming reference — directed, unrolled, and unattributed. 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 = [ @@ -55,14 +55,18 @@ public static class QueryEngine ]; /// - /// 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. + /// The value identifying a port usage. + /// Named here rather than repeated inline so the endpoint-only classification in + /// reads as a structural model test rather than a + /// bare string comparison. /// - private const int DefaultConnectionHopLimit = 1; + private const string PortFeatureKeyword = "port"; + + /// + /// The value identifying a port + /// definition, the definition-side counterpart of . + /// + private const string PortDefinitionKeyword = "port def"; /// /// Dispatches to the verb method selected by , the single @@ -237,40 +241,40 @@ public static QueryResult Dependencies(SysmlWorkspace workspace, SysmlNode eleme /// closure of , bounded by when /// specified (unlimited otherwise), optionally extended with undirected connector /// (connect/bind) traversal when - /// is set. + /// is set. The bound applies uniformly to + /// reference and connector relationships alike — one relationship is one unit of depth + /// regardless of its kind. /// /// 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. + /// A single depth budget is applied, because users reason about proximity to the subject + /// rather than about per-relationship-class allowances; the class of each relationship is + /// reported on the entry () for clients to group or + /// filter on after the fact. therefore + /// selects which edges exist in the graph, never how far the walk goes. /// /// 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. + /// , rolled up to its owning declaration — 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. + /// Because every traversed relationship costs exactly one breadth-first level (both + /// collectors append only to the next level's frontier, and neither can reach an element + /// without advancing a level), this is a uniform-cost level-order search. First arrival is + /// therefore provably the shortest relationship distance, so a plain membership set + /// suffices as the cycle guard and is exactly that + /// shortest distance. Termination on cyclic and densely connected graphs follows from + /// each element being admitted to the guard at most once. /// /// public static QueryResult Impact(SysmlWorkspace workspace, SysmlNode element, QueryOptions options) { var qualifiedName = QualifiedNameOf(element, options); - var bestHops = new Dictionary(StringComparer.Ordinal) { [qualifiedName] = 0 }; + var visited = new HashSet(StringComparer.Ordinal) { qualifiedName }; var entries = new List(); // Collected once per call, and only when requested, rather than once per frontier item. @@ -278,24 +282,23 @@ public static QueryResult Impact(SysmlWorkspace workspace, SysmlNode element, Qu options.IncludeConnections ? CollectConnectorEdges(workspace, ImpactConnectorEdgeKinds) : []; - var connectionHopLimit = options.WalkDepth ?? DefaultConnectionHopLimit; - var frontier = new List<(string Name, int ConnectionHops)> { (qualifiedName, 0) }; + var frontier = new List { qualifiedName }; var depth = 0; while (frontier.Count > 0 && (options.WalkDepth is not { } maxDepth || depth < maxDepth)) { depth++; - var next = new List<(string Name, int ConnectionHops)>(); + var next = new List(); - foreach (var (current, hops) in frontier) + foreach (var current in frontier) { - CollectImpactReferences(workspace, options, current, hops, depth, bestHops, entries, next); + CollectImpactReferences(workspace, options, current, depth, visited, entries, next); - if (options.IncludeConnections && hops < connectionHopLimit) + if (options.IncludeConnections) { CollectImpactConnections( - workspace, options, current, hops, depth, connectorEdges, bestHops, entries, next); + workspace, options, current, depth, connectorEdges, visited, entries, next); } } @@ -303,9 +306,7 @@ 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; + var connectionSuffix = options.IncludeConnections ? ", including connections" : string.Empty; return new QueryResult { Verb = "impact", @@ -322,50 +323,48 @@ public static QueryResult Impact(SysmlWorkspace workspace, SysmlNode element, Qu /// /// 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 frontier for the following depth level. /// /// /// 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. + /// declaration, and without attribution. + /// 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 minimum-connector-hop cycle guard. + /// + /// The shared cycle guard, holding every qualified name already reached (seeded with the + /// subject so it never reports itself). Updated in place. + /// /// 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, - Dictionary bestHops, + HashSet visited, List entries, - List<(string Name, int ConnectionHops)> next) + List next) { foreach (var edge in workspace.Index.GetIncomingEdges(current)) { if (IsImpactConnectorKind(edge.Kind) || edge.SourceQualifiedName is not { Length: > 0 } source || - !TryReach(bestHops, source, hops, out var isFirstArrival)) + !visited.Add(source)) { continue; } - next.Add((source, hops)); + next.Add(source); - // 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)) + if (!IsVisible(source, workspace, options.IncludeStdlib)) { continue; } @@ -386,27 +385,28 @@ private static void CollectImpactReferences( /// 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. + /// declaration. Costs one depth level per reached element, exactly like a reference edge. /// /// 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 minimum-connector-hop cycle guard. + /// + /// The shared cycle guard, holding every qualified name already reached (seeded with the + /// subject so it never reports itself). Updated in place. + /// /// 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, - Dictionary bestHops, + HashSet visited, List entries, - List<(string Name, int ConnectionHops)> next) + List next) { foreach (var (source, target, keyword, kind) in connectorEdges) { @@ -422,17 +422,15 @@ private static void CollectImpactConnections( var near = nearIsSource ? source : target; var far = nearIsSource ? target : source; - var owner = RollUpToNearestDeclaration(workspace, far); - if (!TryReach(bestHops, owner, hops + 1, out var isFirstArrival)) + var owner = RollUpToNearestDeclaration(workspace, far, near); + if (!visited.Add(owner)) { continue; } - next.Add((owner, hops + 1)); + next.Add(owner); - // 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)) + if (!IsVisible(owner, workspace, options.IncludeStdlib)) { continue; } @@ -1046,7 +1044,7 @@ candidate is not null && /// /// Determines whether an edge kind is one of the connector kinds the impact verb - /// handles through its dedicated, hop-bounded, rolled-up connector pass. + /// handles through its dedicated, rolled-up connector pass. /// /// The edge kind to test. /// @@ -1058,84 +1056,73 @@ 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. + /// Rolls a connector endpoint up to the nearest owning declaration that is a meaningful + /// impact subject. The endpoint itself is probed first and returned unchanged when it is + /// present in and is not an endpoint-only + /// construct (for example a directly connected sibling part usage). Endpoints that are + /// absent from — frequently ports inherited + /// through a typed usage — and endpoints that are declared but endpoint-only, per + /// , have trailing :: segments stripped + /// until such a declaration is found. A candidate that contains the connector's + /// near endpoint is also rejected, so the walk can never roll up into one of its own + /// containers. /// /// - /// 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. + /// + /// Skipping declared endpoint-only constructs matters because whether a port endpoint is + /// itself a key in is an artifact of modeling + /// style, not of meaning. A port declared on a definition (part def Hub { port J1; }) + /// reached through a typed usage yields the path System::hub::J1, which is not a + /// declaration key, whereas the same port declared inline on the usage + /// (part hub { port J1; }) yields a path that is a declaration key. Both + /// forms are legal and equivalent, so stopping at the first declared ancestor would make + /// the answer depend on the model's spelling: it would report a port — never an + /// actionable answer to "what parts are impacted" — and, because the reported name is + /// also the name enqueued onto the traversal frontier, it would dead-end the walk at the + /// port instead of continuing through the owning part. + /// + /// + /// Rolling up must nonetheless stop short of any container of the connector's near + /// endpoint. A proxy port declared on the enclosing definition + /// (part def Mid { port mp; part l1; part l2; connect l1.p to mp; }) has no owning + /// part beside the subject — the nearest non-port owner is Mid itself. Reporting + /// Mid would attribute containment as a connection and, once Mid reached the + /// frontier, every connector interior to it would be discarded by the both-ends-nested + /// guard, hiding l2 at every depth. Keeping the port in that case reports the real + /// shared attachment point and leaves it traversable, so peers across the port stay + /// reachable. + /// /// - /// - /// 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 - /// (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. + /// + /// The opposite endpoint of the same connector — the one belonging to the element the walk + /// is currently expanding. Candidates that contain it are rejected as described above. + /// /// - /// 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. + /// itself when it is declared, is not endpoint-only, and + /// does not contain ; otherwise the nearest declared owning + /// qualified name meeting those conditions, or unchanged + /// when no such ancestor exists, so a connection is never silently dropped. /// - private static string RollUpToNearestDeclaration(SysmlWorkspace workspace, string qualifiedName) + private static string RollUpToNearestDeclaration( + SysmlWorkspace workspace, string qualifiedName, string nearEndpoint) { var probe = qualifiedName; while (true) { - if (workspace.Declarations.ContainsKey(probe)) + // Accept the probe only when it names a declaration that a caller could act on and + // usefully continue the impact walk from - a declared port is neither, and neither is + // a container of the near endpoint, which would convert containment into connectivity. + if (workspace.Declarations.TryGetValue(probe, out var declaration) && + !IsEndpointOnlyDeclaration(declaration) && + !IsSelfOrNestedUnder(nearEndpoint, probe)) { return probe; } + // Strip one containment segment and retry; exhausting the path yields the original + // endpoint rather than nothing, so the connection is still reported. var index = probe.LastIndexOf("::", StringComparison.Ordinal); if (index < 0) { @@ -1146,6 +1133,34 @@ private static string RollUpToNearestDeclaration(SysmlWorkspace workspace, strin } } + /// + /// Determines whether a declaration is an endpoint-only construct: an element that exists + /// solely to be named as a connector endpoint and is therefore never a valid subject of an + /// impact answer. + /// + /// + /// Ports are the only such construct in the modeled node hierarchy. A port has no + /// behavior or state of its own to be impacted - it is the attachment point through which + /// its owning part is impacted - so an impact row naming a port is a name the caller can + /// neither act on nor feed back into another query. Classified structurally, from + /// and + /// , so the decision follows the parsed + /// model rather than any naming convention in the source text. + /// + /// The resolved declaration to classify. Must not be null. + /// + /// when is a port usage or port + /// definition; otherwise . + /// + private static bool IsEndpointOnlyDeclaration(SysmlNode declaration) => declaration switch + { + SysmlFeatureNode feature => + string.Equals(feature.FeatureKeyword, PortFeatureKeyword, StringComparison.Ordinal), + SysmlDefinitionNode definition => + string.Equals(definition.DefinitionKeyword, PortDefinitionKeyword, StringComparison.Ordinal), + _ => false + }; + /// /// 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 f42e0d85..75e1c398 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Query/QueryOptions.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Query/QueryOptions.cs @@ -47,11 +47,11 @@ public sealed record QueryOptions /// /// /// Only meaningful for , where it bounds the transitive - /// 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 - /// ). + /// impact walk; means unlimited. The bound applies + /// uniformly to every relationship kind the walk traverses, including connector + /// edges when is set: one relationship is one unit of + /// depth regardless of its class, and means unlimited in all + /// cases. /// public int? WalkDepth { get; init; } @@ -116,9 +116,11 @@ public sealed record QueryOptions /// 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 . + /// Because connector graphs are dense meshes, an unbounded connection-aware walk on a + /// hub-and-spoke assembly can reach the whole assembly; supply + /// when proximity rather than reachability is wanted. This flag controls only + /// which edges are present in the graph, never how far the walk goes — distance + /// is governed by alone. /// public bool IncludeConnections { get; init; } } diff --git a/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx b/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx index cb8aac84..c4e6b1f8 100644 --- a/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx +++ b/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx @@ -182,16 +182,16 @@ --element <name>, -e <name> Qualified name of the target element (required) - --walk-depth <#> Maximum impact-walk depth (default: unlimited) + --walk-depth <#> Max impact-walk depth, all edge kinds (default: unlimited) --include-connections Also follow connect/bind edges, undirected - (default: reference edges only). Connector + (default: reference edges only). Distance - hops are bounded by --walk-depth, else 1. + is bounded by --walk-depth like any edge. --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 index c70a0414..8fa0a7a8 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Query/QueryEngineImpactTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Query/QueryEngineImpactTests.cs @@ -92,9 +92,10 @@ part def System { """; /// - /// 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. + /// Topology in which b is reachable from s both over a connector + /// (connect b to s, one relationship) and over the subsetting chain + /// b :> s2 :> s (two relationships), so the shorter of the two must win, and + /// z sits one further connector beyond b. /// private const string MinimumHopFixture = """ package Model { @@ -110,6 +111,182 @@ part def Assembly { } """; + /// + /// Connector chain of five connectors joining six declared sibling part usages, so + /// a is one connector hop from b, two from c, three from d, + /// four from e, and five from f. Every endpoint is a declared part usage + /// rather than a nested port, so a reference pass that failed to exclude connector kinds + /// would be detected here as duplicate entries or entries attributed to the wrong + /// connector. The chain is long enough to distinguish an unlimited budget from a merely + /// generous one and to observe a bound of three stopping the walk partway. + /// + private const string LongConnectorChainFixture = """ + package Model { + part def System { + part a; + part b; + part c; + part d; + part e; + part f; + + connect b to a; + connect c to b; + connect d to c; + connect e to d; + connect f to e; + } + } + """; + + /// + /// Cyclic connector topology: four part usages joined into a ring. Traversal from + /// r1 must terminate with no depth bound at all, and r3 — reachable + /// around either side of the ring — must be attributed its minimum ring distance rather + /// than a traversal-order-dependent one. + /// + private const string RingConnectorFixture = """ + package Model { + part def System { + part r1; + part r2; + part r3; + part r4; + + connect r2 to r1; + connect r3 to r2; + connect r4 to r3; + connect r1 to r4; + } + } + """; + + /// + /// Topology placing a three-link pure-reference chain and a three-hop pure-connector + /// chain on the same subject, so a single depth bound can be observed cutting both + /// chains at exactly the same distance. + /// + private const string MixedReferenceAndConnectorFixture = """ + package Model { + part def Assembly { + part ref0; + part ref1 :> ref0; + part ref2 :> ref1; + part ref3 :> ref2; + part con1; + part con2; + part con3; + + connect con1 to ref0; + connect con2 to con1; + connect con3 to con2; + } + } + """; + + /// + /// Topology in which two elements are each reachable from origin by both a + /// reference path and a connector path, with the shorter path being of a different class + /// in each case: viaConnector is one connector away but two references away, and + /// viaReference is one reference away but two connectors away. Under one uniform + /// depth, each must be reported once, at the shorter distance, carrying the relation of + /// the path that achieved it. + /// + private const string DualPathFixture = """ + package Model { + part def Assembly { + part origin; + + part link :> origin; + part viaConnector :> link; + connect viaConnector to origin; + + part viaReference :> origin; + part relay; + connect relay to origin; + connect viaReference to relay; + } + } + """; + + /// + /// Hub-and-spoke connector topology whose ports are declared inline on the part usage + /// (part hub { port J1; port J2; }) rather than on a part definition reached through + /// a typed usage, as in . + /// + /// The distinction is the whole point of this fixture and is invisible in the rendered + /// output: declaring the ports inline makes the connector endpoint path + /// M::S::hub::J1 itself a key in SysmlWorkspace.Declarations, whereas the + /// definition-side style leaves the endpoint path undeclared (the declaration is + /// Hub::J1). A far-endpoint roll-up that stops at the first declared ancestor + /// therefore reports the raw port for this shape only — and, because the reported name is + /// also the name enqueued onto the traversal frontier, dead-ends the walk at the port so + /// motorB is never reached at any depth. Both modeling styles are legal, so both + /// shapes must be covered. + /// + /// + private const string InlineUsagePortsFixture = """ + package M { + part def S { + part hub { + port J1; + port J2; + } + + part motorA; + part motorB; + + connect hub.J1 to motorA; + connect hub.J2 to motorB; + } + } + """; + + /// + /// A proxy-port topology: mp is declared on the enclosing definition and both + /// nested parts connect to it, the standard SysML v2 interface-delegation idiom. The + /// nearest non-port owner of mp is Mid, which contains the query subject, + /// so roll-up must decline it and keep the port. Rolling up to Mid would attribute + /// containment as a connection and, because every connector interior to Mid has + /// both ends nested under it, would hide l2 at every depth. + /// + private const string ProxyPortFixture = """ + package Model { + part def Leaf { + port p; + } + + part def Mid { + port mp; + part l1 : Leaf; + part l2 : Leaf; + + connect l1.p to mp; + connect l2.p to mp; + } + } + """; + + /// + /// A port declared directly in a package, whose only non-port ancestor is the package + /// itself. Reporting the package would make every element it contains appear connected, + /// because the both-ends-nested test treats containment as membership. + /// + private const string PackageScopedPortFixture = """ + package Top { + package M { + port P; + part x; + part y; + } + + part outsider; + + connect M::P to M::x; + connect M::y to outsider; + } + """; + /// /// 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 @@ -300,20 +477,25 @@ public async Task Impact_IncludeConnections_SourceSidePortEndpoint_ProducesExact } /// - /// 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. + /// An element reachable by both a connector path and a longer reference path is reported + /// exactly once, at the shorter of the two distances, and the elements beyond it are + /// attributed relative to that shorter distance. /// /// - /// 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. + /// z is expected at depth 2, not 3: with one uniform budget the shortest path is + /// s → b (the connector connect b to s) → z (the connector + /// connect z to b), two relationships. The two-relationship subsetting detour + /// b :> s2 :> s no longer delays b's connector expansion, because + /// there is no second budget for a connector hop to exhaust. /// [Fact] - public async Task Impact_IncludeConnections_ReReachedAtLowerHopCount_KeepsFirstArrivalDepthAndAttribution() + public async Task Impact_IncludeConnections_ReachedByTwoPaths_ReportsShortestDistance() { + // Arrange: 'b' is one connector from 's' and two subsettings from 's'; 'z' is one + // further connector beyond 'b' var (workspace, element) = await LoadAsync(MinimumHopFixture, "Model::Assembly::s"); + // Act: walk with connections enabled and no depth bound var result = QueryEngine.Impact( workspace, element, @@ -324,12 +506,379 @@ public async Task Impact_IncludeConnections_ReReachedAtLowerHopCount_KeepsFirstA IncludeConnections = true }); + // Assert: 'b' is reported once at the shorter connector distance, and 'z' one beyond it 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(2, z.Depth); Assert.Equal(SysmlEdgeKind.Connect, z.Relation); } + + /// + /// With no walk depth supplied the walk is unlimited for connector edges just as it is + /// for reference edges, so an entire connector chain is reported, each element at its + /// shortest relationship distance from the subject. + /// + [Fact] + public async Task Impact_IncludeConnections_NoWalkDepth_ReachesEntireConnectorChainAtShortestDistance() + { + // Arrange: a six-part chain joined by five connectors, queried from one end + var (workspace, element) = await LoadAsync(LongConnectorChainFixture, "Model::System::a"); + + // Act: enable connections and supply no depth bound at all + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::System::a", + IncludeConnections = true + }); + + // Assert: every element in the chain is reported exactly once, at its true hop distance + Assert.Equal(5, result.Entries.Count); + var expectedDepths = new Dictionary(StringComparer.Ordinal) + { + ["Model::System::b"] = 1, + ["Model::System::c"] = 2, + ["Model::System::d"] = 3, + ["Model::System::e"] = 4, + ["Model::System::f"] = 5 + }; + foreach (var (name, expectedDepth) in expectedDepths) + { + var entry = Assert.Single(result.Entries, e => e.QualifiedName == name); + Assert.Equal(expectedDepth, entry.Depth); + Assert.Equal(SysmlEdgeKind.Connect, entry.Relation); + } + } + + /// + /// A walk depth of three means "everything within three relationships of the subject", + /// so a connector chain is followed exactly three hops and no further. + /// + [Fact] + public async Task Impact_IncludeConnections_WalkDepthThree_BoundsConnectorChainToThreeHops() + { + // Arrange: the same six-part connector chain + var (workspace, element) = await LoadAsync(LongConnectorChainFixture, "Model::System::a"); + + // Act: bound the walk to three relationships + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::System::a", + IncludeConnections = true, + WalkDepth = 3 + }); + + // Assert: exactly b, c and d are reported — never e or f + Assert.Equal(3, result.Entries.Count); + Assert.Contains(result.Entries, e => e.QualifiedName == "Model::System::b"); + Assert.Contains(result.Entries, e => e.QualifiedName == "Model::System::c"); + Assert.Contains(result.Entries, e => e.QualifiedName == "Model::System::d"); + Assert.DoesNotContain(result.Entries, e => e.QualifiedName == "Model::System::e"); + Assert.DoesNotContain(result.Entries, e => e.QualifiedName == "Model::System::f"); + } + + /// + /// One walk depth bounds reference and connector relationships identically: a reference + /// chain and a connector chain of the same length are both cut at the same distance. + /// + [Fact] + public async Task Impact_IncludeConnections_MixedPaths_WalkDepthBoundsBothEdgeClassesIdentically() + { + // Arrange: a three-link reference chain and a three-hop connector chain on one subject + var (workspace, element) = await LoadAsync(MixedReferenceAndConnectorFixture, "Model::Assembly::ref0"); + + // Act: bound the walk to two relationships of any kind + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::Assembly::ref0", + IncludeConnections = true, + WalkDepth = 2 + }); + + // Assert: both chains are followed to exactly two and cut at exactly three + Assert.Equal(1, Assert.Single(result.Entries, e => e.QualifiedName == "Model::Assembly::ref1").Depth); + Assert.Equal(2, Assert.Single(result.Entries, e => e.QualifiedName == "Model::Assembly::ref2").Depth); + Assert.Equal(1, Assert.Single(result.Entries, e => e.QualifiedName == "Model::Assembly::con1").Depth); + Assert.Equal(2, Assert.Single(result.Entries, e => e.QualifiedName == "Model::Assembly::con2").Depth); + Assert.DoesNotContain(result.Entries, e => e.QualifiedName == "Model::Assembly::ref3"); + Assert.DoesNotContain(result.Entries, e => e.QualifiedName == "Model::Assembly::con3"); + } + + /// + /// An element reachable by both a reference path and a connector path is reported once, + /// at the shorter distance, carrying the relation of the path that achieved it — + /// whichever class that path happens to be. + /// + [Fact] + public async Task Impact_IncludeConnections_ReachableByReferenceAndConnector_ReportedOnceAtShorterDistance() + { + // Arrange: one element is closer over a connector, another is closer over a reference + var (workspace, element) = await LoadAsync(DualPathFixture, "Model::Assembly::origin"); + + // Act: walk with connections enabled and no depth bound + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::Assembly::origin", + IncludeConnections = true, + IncludeStdlib = false + }); + + // Assert: the connector path wins where it is shorter, keeping its Connect relation + var viaConnector = Assert.Single(result.Entries, e => e.QualifiedName == "Model::Assembly::viaConnector"); + Assert.Equal(1, viaConnector.Depth); + Assert.Equal(SysmlEdgeKind.Connect, viaConnector.Relation); + + // Assert: the reference path wins where it is shorter, keeping its reference relation + var viaReference = Assert.Single(result.Entries, e => e.QualifiedName == "Model::Assembly::viaReference"); + Assert.Equal(1, viaReference.Depth); + Assert.Equal(SysmlEdgeKind.Supertype, viaReference.Relation); + } + + /// + /// A cyclic connector topology with no depth bound at all terminates and attributes each + /// element its minimum ring distance rather than a traversal-order-dependent one. + /// + [Fact] + public async Task Impact_IncludeConnections_RingTopology_NoDepthBound_TerminatesWithShortestDistances() + { + // Arrange: four part usages joined into a connector ring + var (workspace, element) = await LoadAsync(RingConnectorFixture, "Model::System::r1"); + + // Act: walk the ring with connections enabled and no depth bound + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::System::r1", + IncludeConnections = true + }); + + // Assert: the walk terminated, reporting each neighbour once at its minimum ring distance + Assert.Equal(3, result.Entries.Count); + Assert.Equal(1, Assert.Single(result.Entries, e => e.QualifiedName == "Model::System::r2").Depth); + Assert.Equal(1, Assert.Single(result.Entries, e => e.QualifiedName == "Model::System::r4").Depth); + Assert.Equal(2, Assert.Single(result.Entries, e => e.QualifiedName == "Model::System::r3").Depth); + } + + /// + /// On a hub-and-spoke topology, a sibling spoke reached through the shared hub is + /// reported at depth two rather than suppressed, because no per-edge-class budget is + /// exhausted by the first hop into the hub. + /// + [Fact] + public async Task Impact_IncludeConnections_HubTopology_SpokeReachesOtherSpokeAtDepthTwo() + { + // Arrange: two motors each connected to a distinct port of a shared hub + var (workspace, element) = await LoadAsync(ConnectedPartsFixture, "Model::System::motorA"); + + // Act: walk from one spoke with connections enabled and no depth bound + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::System::motorA", + IncludeConnections = true + }); + + // Assert: the hub is at depth one and the sibling spoke beyond it at depth two + Assert.Equal(1, Assert.Single(result.Entries, e => e.QualifiedName == "Model::System::hub").Depth); + Assert.Equal(2, Assert.Single(result.Entries, e => e.QualifiedName == "Model::System::motorB").Depth); + } + + /// + /// Regression: a connector endpoint that is a port declared inline on a part usage — and + /// therefore itself a declaration key — is still attributed to the owning part usage, not + /// reported raw, with the port preserved in + /// . + /// + [Fact] + public async Task Impact_IncludeConnections_InlineUsagePortEndpoint_ReportsOwningPartUsage() + { + // Arrange: hub-and-spoke topology whose ports are declared inline on the hub usage + var (workspace, element) = await LoadAsync(InlineUsagePortsFixture, "M::S::motorA"); + + // Act: walk from one spoke with connections enabled and no depth bound + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "M::S::motorA", + IncludeConnections = true + }); + + // Assert: the owning part usage is reported at depth one, with the raw port recorded + var entry = Assert.Single(result.Entries, e => e.QualifiedName == "M::S::hub"); + Assert.Equal(1, entry.Depth); + Assert.Equal(SysmlEdgeKind.Connect, entry.Relation); + Assert.Equal("M::S::hub::J1", entry.ViaQualifiedName); + Assert.DoesNotContain(result.Entries, e => e.QualifiedName == "M::S::hub::J1"); + } + + /// + /// Regression: rolling an inline-declared port endpoint up to its owning part usage also + /// places that part — not the port — on the traversal frontier, so the walk continues + /// through the hub and reaches the far spoke. Stopping at the port dead-ends the walk and + /// makes the far spoke unreachable at any depth, which is what this asserts against. + /// + [Fact] + public async Task Impact_IncludeConnections_InlineUsagePortEndpoint_ContinuesTraversalBeyondPortOwner() + { + // Arrange: hub-and-spoke topology whose ports are declared inline on the hub usage + var (workspace, element) = await LoadAsync(InlineUsagePortsFixture, "M::S::motorA"); + + // Act: walk from one spoke with connections enabled and no depth bound + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "M::S::motorA", + IncludeConnections = true + }); + + // Assert: the sibling spoke beyond the hub is reached, at exactly one further hop + var entry = Assert.Single(result.Entries, e => e.QualifiedName == "M::S::motorB"); + Assert.Equal(2, entry.Depth); + Assert.Equal(SysmlEdgeKind.Connect, entry.Relation); + Assert.Null(entry.ViaQualifiedName); + } + + /// + /// Regression: when a connector endpoint's nearest non-port owner contains the subject — + /// a proxy port declared on the enclosing definition — roll-up declines that owner and + /// keeps the port, so the shared attachment point is reported rather than the container. + /// + [Fact] + public async Task Impact_IncludeConnections_ProxyPortOnOwningDefinition_ReportsPortNotContainer() + { + // Arrange: interface-delegation topology whose shared port hangs off the definition + var (workspace, element) = await LoadAsync(ProxyPortFixture, "Model::Mid::l1"); + + // Act: walk from one nested part with connections enabled and no depth bound + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::Mid::l1", + IncludeConnections = true + }); + + // Assert: the proxy port stands in for the connection, and its container is never reported + var entry = Assert.Single(result.Entries, e => e.QualifiedName == "Model::Mid::mp"); + Assert.Equal(1, entry.Depth); + Assert.Equal(SysmlEdgeKind.Connect, entry.Relation); + Assert.DoesNotContain(result.Entries, e => e.QualifiedName == "Model::Mid"); + } + + /// + /// Regression: keeping the proxy port on the frontier preserves reachability across it, + /// so the sibling sharing that port is still reported at exactly one further hop. Rolling + /// up to the containing definition instead would discard every connector interior to it + /// and make the sibling unreachable at any depth. + /// + [Fact] + public async Task Impact_IncludeConnections_ProxyPortOnOwningDefinition_ReachesSiblingAcrossPort() + { + // Arrange: interface-delegation topology whose shared port hangs off the definition + var (workspace, element) = await LoadAsync(ProxyPortFixture, "Model::Mid::l1"); + + // Act: walk from one nested part with connections enabled and no depth bound + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::Mid::l1", + IncludeConnections = true + }); + + // Assert: the sibling on the far side of the shared port is reached at two hops + var entry = Assert.Single(result.Entries, e => e.QualifiedName == "Model::Mid::l2"); + Assert.Equal(2, entry.Depth); + Assert.Equal(SysmlEdgeKind.Connect, entry.Relation); + } + + /// + /// Regression: a package is never an impact subject. Rolling a package-scoped port up to + /// its package would place a container on the frontier, and because the both-ends-nested + /// test treats containment as membership, every element inside that package would appear + /// connected to the subject — here outsider, which shares no connector with it. + /// + [Fact] + public async Task Impact_IncludeConnections_PackageScopedPort_DoesNotTreatPackageAsConnected() + { + // Arrange: a port whose only non-port ancestor is the enclosing package + var (workspace, element) = await LoadAsync(PackageScopedPortFixture, "Top::M::x"); + + // Act: walk with connections enabled and no depth bound + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Top::M::x", + IncludeConnections = true + }); + + // Assert: only the port is reported - no package row, and no element merely sharing it + var entry = Assert.Single(result.Entries); + Assert.Equal("Top::M::P", entry.QualifiedName); + Assert.Equal(1, entry.Depth); + } + + /// + /// Regression: no impact row names a port, for either port-declaration style. A port is + /// an attachment point rather than an actionable impact subject, so its presence would be + /// a name the caller can neither act on nor feed back into another query. + /// + /// The connector fixture to walk. + /// The qualified name of the spoke to query from. + [Theory] + [InlineData(InlineUsagePortsFixture, "M::S::motorA")] + [InlineData(ConnectedPartsFixture, "Model::System::motorA")] + public async Task Impact_IncludeConnections_PortDeclarationStyles_ReportNoPortEntries( + string fixture, string subject) + { + // Arrange: load the fixture and resolve the spoke to query from + var (workspace, element) = await LoadAsync(fixture, subject); + + // Act: walk with connections enabled and no depth bound + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions { Verb = QueryVerb.Impact, Element = subject, IncludeConnections = true }); + + // Assert: results are non-empty and no reported element is a port + Assert.NotEmpty(result.Entries); + Assert.DoesNotContain(result.Entries, e => e.Kind == "port"); + } } diff --git a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs index b9f7030f..ace4dd0a 100644 --- a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs +++ b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs @@ -82,7 +82,7 @@ part def System { """; /// - /// Shared inline SysML fixture for connector hop-bound scenarios: a chain of three + /// Shared inline SysML fixture for connector-distance 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. /// @@ -140,14 +140,13 @@ part def System { """; /// - /// Shared inline SysML fixture in which an element is first reached over a connector and - /// later re-reached over a cheaper pure-reference path. + /// Shared inline SysML fixture in which an element is reachable both over a connector and + /// over a longer 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. + /// Querying impact from s: the connector connect b to s reaches b in + /// one relationship, while the subsetting chain b :> s2 :> s reaches it in + /// two. Under one uniform depth the shorter path wins, so b is reported at depth + /// one and z — one further connector beyond b — at depth two. /// /// public const string MinimumHopReExpansion = """ @@ -164,6 +163,60 @@ part def Assembly { } """; + /// + /// Shared inline SysML fixture for uniform-depth scenarios needing more than three + /// hops: five connectors joining six directly declared sibling part usages, so a + /// is one connector hop from b and five from f. + /// + /// A chain this long distinguishes an unlimited walk from a merely generous one, and + /// gives room for a bound of two or three to be observed stopping the walk partway + /// rather than at the far end of the fixture. + /// + /// + public const string LongDeclaredConnectorChain = """ + package Model { + part def System { + part a; + part b; + part c; + part d; + part e; + part f; + + connect b to a; + connect c to b; + connect d to c; + connect e to d; + connect f to e; + } + } + """; + + /// + /// Shared inline SysML fixture forming a four-part connector ring, so every member is + /// reachable around either side of the cycle. + /// + /// Exists to prove that an unbounded connector walk terminates on a cyclic topology and + /// still attributes each element its minimum ring distance rather than a + /// traversal-order-dependent one. + /// + /// + public const string RingConnectors = """ + package Model { + part def System { + part r1; + part r2; + part r3; + part r4; + + connect r2 to r1; + connect r3 to r2; + connect r4 to r3; + connect r1 to r4; + } + } + """; + /// /// 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 a3bb9c62..756616dd 100644 --- a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs @@ -221,8 +221,8 @@ package Model { /// /// 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 + /// incoming references — directed and attributed to the raw endpoint rather than its + /// owning declaration. 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 @@ -258,7 +258,7 @@ public async Task Impact_IncludeConnections_ReachesConnectedSiblingPart() Assert.Equal(0, exitCode); Assert.Contains("| Model::System::hub | part |", output); - Assert.Contains("including connections (connection hops <= 1)", output); + Assert.Contains("including connections", output); } /// @@ -302,20 +302,19 @@ 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 a chain of connectors is followed exactly one hop and the - /// reported count matches the "connection hops <= 1" claim in the summary. + /// With no --walk-depth supplied the walk is unlimited for connector edges just as it is + /// for reference edges, so an entire chain of connectors is followed to its far end. /// /// /// 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. + /// precondition for a connector 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 — + /// in particular to a connector being attributed twice — rather than only to specific ones. /// [Fact] - public async Task Impact_IncludeConnections_NoWalkDepth_BoundsConnectionHopsToOne() + public async Task Impact_IncludeConnections_NoWalkDepth_ReachesEntireConnectorChain() { var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( QueryTestFixtures.ChainedDeclaredConnectors, @@ -325,15 +324,15 @@ public async Task Impact_IncludeConnections_NoWalkDepth_BoundsConnectionHopsToOn "--include-connections"); Assert.Equal(0, exitCode); - Assert.Contains("1 element(s) transitively impacted", output); + Assert.Contains("3 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); + Assert.Contains("| Model::System::c | part |", output); + Assert.Contains("| Model::System::d | part |", output); + Assert.Contains("including connections", output); } /// - /// Supplying --walk-depth raises the connector hop bound to that same value, so the + /// Supplying --walk-depth bounds the walk to that many relationships of any kind, so the /// second motor is reached through the hub at the second hop. /// [Fact] @@ -351,12 +350,13 @@ public async Task Impact_IncludeConnections_WalkDepthTwo_ReachesSecondConnection 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); + Assert.Contains("including connections", output); } /// /// A cyclic connector topology (two parts joined by two connectors in opposite textual - /// order) terminates and reports each impacted element exactly once. + /// order) terminates and reports each impacted element exactly once, with no depth bound + /// supplied at all. /// [Fact] public async Task Impact_IncludeConnections_CyclicConnections_TerminatesWithoutDuplicates() @@ -384,7 +384,7 @@ part def System { """; var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( - sysml, "impact", "--element", "Model::System::motorA", "--include-connections", "--walk-depth", "5"); + sysml, "impact", "--element", "Model::System::motorA", "--include-connections"); Assert.Equal(0, exitCode); Assert.Contains("1 element(s) transitively impacted", output); @@ -393,6 +393,67 @@ part def System { line => line.StartsWith("| Model::System::hub |", StringComparison.Ordinal)); } + /// + /// A four-part connector ring with no depth bound at all terminates, reporting every + /// other ring member exactly once. + /// + [Fact] + public async Task Impact_IncludeConnections_RingTopology_NoDepthBound_Terminates() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.RingConnectors, + "impact", + "--element", + "Model::System::r1", + "--include-connections"); + + Assert.Equal(0, exitCode); + Assert.Contains("3 element(s) transitively impacted", output); + Assert.Contains("| Model::System::r2 | part |", output); + Assert.Contains("| Model::System::r3 | part |", output); + Assert.Contains("| Model::System::r4 | part |", output); + } + + /// + /// A five-connector chain with no --walk-depth supplied is followed all the way to its + /// far end, proving the unbounded default rather than a merely generous one. + /// + [Fact] + public async Task Impact_IncludeConnections_LongChain_NoWalkDepth_ReachesFarEnd() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.LongDeclaredConnectorChain, + "impact", + "--element", + "Model::System::a", + "--include-connections"); + + Assert.Equal(0, exitCode); + Assert.Contains("5 element(s) transitively impacted", output); + Assert.Contains("| Model::System::f | part |", output); + } + + /// + /// The unbounded connection-aware summary states only that connections were included, + /// with no separate connector-hop clause, because there is only one depth budget. + /// + [Fact] + public async Task Impact_Summary_UnboundedWithConnections_OmitsHopClause() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.ChainedDeclaredConnectors, + "impact", + "--element", + "Model::System::a", + "--include-connections"); + + Assert.Equal(0, exitCode); + Assert.Contains( + "3 element(s) transitively impacted by a change to 'Model::System::a', including connections.", + output); + Assert.DoesNotContain("connection hops", output); + } + /// /// Binding connectors ('bind A = B;') are traversed undirected exactly like connection /// connectors, reachable from either bound side. @@ -450,28 +511,6 @@ 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 @@ -493,8 +532,9 @@ public async Task Impact_ChainedDeclaredConnectors_FlagAbsent_ReportsNoImpactedE } /// - /// 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. + /// Setting --walk-depth to two bounds the walk to two relationships of any kind, 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() @@ -512,7 +552,7 @@ public async Task Impact_IncludeConnections_ChainedDeclaredConnectors_WalkDepthT 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); + Assert.Contains("including connections", output); } /// @@ -555,17 +595,17 @@ public async Task Impact_SourceSidePortEndpoint_FlagAbsent_ReportsNoImpactedElem } /// - /// 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. + /// An element reachable both over a connector and over a longer pure-reference path is + /// reported at the shorter of the two distances, and the element one connector beyond it + /// is reported relative to that shorter distance. /// /// - /// '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. + /// 'z' is expected at depth 2, not 3: with one uniform budget the shortest path is + /// s -> b -> z over two connectors, and there is no separate connector budget for the + /// first hop to exhaust. /// [Fact] - public async Task Impact_IncludeConnections_CheaperReferencePath_ReExpandsConnectorsFromReReachedElement() + public async Task Impact_IncludeConnections_ReachedByTwoPaths_ReportsShortestDistance() { var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( QueryTestFixtures.MinimumHopReExpansion, @@ -578,7 +618,10 @@ public async Task Impact_IncludeConnections_CheaperReferencePath_ReExpandsConnec 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); + + // Bind the depth to the row it belongs to: a bare "depth 2" would also be satisfied by + // any other row, which is the whole point of this minimum-hop case. + Assert.Contains("| Model::Assembly::z | part | depth 2", output); } ///