Unify walk depth control for all relationship types - #55
Merged
Conversation
The `query impact` verb bounded connector relationships to `WalkDepth ?? 1` while bounding reference relationships to `WalkDepth ?? unlimited`. One knob therefore controlled two budgets with different defaults, and leaving `--walk-depth` blank silently meant "one hop" for connectors while meaning "unlimited" for references. Make `--walk-depth` / `QueryOptions.WalkDepth` the single depth control, applied uniformly to every traversed relationship: a typing, a specialization, a satisfy, a connect, and a binding each cost exactly one unit of depth, and `null` means unlimited for all of them. Users reason about proximity to the subject rather than per-relationship-class allowances, and the class of each relationship is already reported on `QueryResultEntry.Relation` for clients to group or filter on after the fact. `--include-connections` is retained unchanged; it selects which edges are in the graph, never how far the walk goes. This is a net simplification of the engine. The separate connector budget was the sole reason the walk carried a per-frontier-item hop counter alongside the breadth-first level, maintained a `bestHops` minimum-cost dictionary, and ran a three-way `TryReach` re-expansion. With one budget the traversal is a uniform-cost, level-synchronous breadth-first search: both collectors append only to the next level's frontier, and neither `IsSelfOrNestedUnder` nor `RollUpToNearestDeclaration` traverses an edge, so there are no zero-cost edges. First arrival is therefore provably the shortest relationship distance, `TryReach`'s "strictly cheaper re-arrival" branch becomes statically unreachable, and a plain `visited` set suffices. `DefaultConnectionHopLimit`, `TryReach`, `bestHops`, the hop counter, and the frontier tuple are all removed; `Depth` is now exactly the breadth-first level and exactly the shortest distance. Removing the second counter removes the class of defect in which reported depth disagreed with true distance, rather than hiding it: with one counter there is no second quantity that can disagree with traversal order. `Impact_IncludeConnections_ReachedByTwoPaths_ReportsShortestDistance` consequently expects `z` at depth 2 (s -> b -> z, two connectors) instead of 3. The result summary drops the `(connection hops <= N)` clause, leaving two cases — bounded and unbounded — while still stating whether connections were included. Requirements, design, verification, user guide, README, and `impact` verb help are updated to describe a single uniform depth that is unlimited by default. The bus-degeneration argument is kept as guidance on when a user may want to pass `--walk-depth`, not as a description of default behavior, and the behavior change relative to the `0.2.0-beta.1` tag is called out in the user guide and core design notes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Impact roll-up stopped at the first ancestor present in the workspace declarations, including ports. Whether a port endpoint is itself a declaration key is an artifact of modeling style: a port declared on a definition and reached through a typed usage is not a key, while the same port declared inline on a part usage is. The first form rolled up correctly; the second reported the raw port and, because the reported name is also the name enqueued onto the traversal frontier, dead-ended the walk so nothing beyond the port was ever reached at any depth. Roll-up now skips endpoint-only declarations, classified structurally from the parsed feature and definition keywords rather than by name matching, and keeps walking up to the owning part. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Roll-up skipped ports but accepted any other declaration, so when a port's
nearest non-port owner also contained the connector's near endpoint, the
walk rolled up into one of its own containers.
Two shapes hit this. A proxy port declared on the enclosing definition
(part def Mid { port mp; part l1; part l2; connect l1.p to mp; }) rolled up
to Mid, and because a connector is only followed when exactly one end lies
within the element being expanded, every connector interior to Mid was then
discarded - hiding l2 at every depth, including unlimited. Mid being a
definition also pulled in its usages through typing edges. A port declared
directly in a package rolled up to the package, making everything inside it
appear connected to the subject.
Roll-up now declines any candidate containing the near endpoint and reports
the endpoint itself instead, keeping the real shared attachment point in the
answer and its neighbors reachable.
Also corrects the port-skip requirement, which claimed impact excludes ports
outright, and documents that --walk-depth is unlimited when omitted.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request updates the documentation and requirements for the
impactquery verb in SysML2Tools, clarifying and simplifying how connection traversal and depth bounding work. The main changes are to make the depth bound (--walk-depth) apply uniformly to all relationship kinds, remove the default one-hop limit for connectors, and improve the explanation of endpoint roll-up and connector handling. These changes ensure that connection traversal is opt-in, consistently bounded, and more predictable for users.Impact query behavior and options:
--walk-depthoption now applies uniformly to all relationship types (references and connectors) in theimpactquery, with no default one-hop limit for connectors; if omitted, the traversal is unlimited. (README.md,docs/design/sysml2-tools-tool.md,docs/design/sysml2-tools-core/query.md,docs/reqstream/sysml2-tools-core/query.yaml) [1] [2] [3] [4] [5] [6]--include-connectionsflag now only determines which relationships are included in the graph (reference and/or connector edges), not how far the walk goes; traversal distance is always controlled by--walk-depth. (README.md,docs/design/sysml2-tools-core/query.md,docs/reqstream/sysml2-tools-core/query.yaml) [1] [2] [3] [4]Connector endpoint roll-up and reporting:
docs/design/sysml2-tools-core/query.md,docs/reqstream/sysml2-tools-core/query.yaml) [1] [2]docs/design/sysml2-tools-core/query.md,docs/reqstream/sysml2-tools-core/query.yaml) [1] [2]Documentation and requirements alignment:
--walk-depthand--include-connections. (README.md,docs/design/sysml2-tools-tool.md,docs/design/sysml2-tools-core/query.md,docs/reqstream/sysml2-tools-core/query.yaml) [1] [2] [3] [4] [5] [6] [7]These changes make the impact analysis more intuitive, consistent, and easier to understand for users.