Add connection-aware impact analysis to Query subsystem - #54
Merged
Conversation
Add an opt-in `--include-connections` flag to `query impact`, backed by `QueryOptions.IncludeConnections`. Default impact semantics are unchanged. When enabled, `Connect` and `Binding` edges are traversed undirected, since `SysmlEdge` documents their source/target ordering as carrying no causal direction. All other edge kinds keep the existing reverse-only directed traversal. Connection endpoints are rolled up through containment in both directions, reusing the `Connections` verb's prefix logic (extracted as `IsSelfOrNestedUnder`), so a subject part matches endpoints nested inside it and each far endpoint is attributed to its nearest owning declaration. Connector hops are bounded by `WalkDepth ?? 1` via a per-frontier-item hop counter, guarding against the transitive degeneracy of shared buses. Reference edge depth semantics are unchanged. Add structured, machine-readable members to `QueryResultEntry` for programmatic clients: `int? Depth`, `SysmlEdgeKind? Relation`, and `string? ViaQualifiedName`, all omitted from JSON when null and registered in the source-generated serializer context to stay AOT-safe. `Detail` remains populated for human-readable Markdown, but the structured fields are authoritative so clients need not parse "depth N" from English text. All changes are additive and source-compatible. Requirements, design, verification docs, README, and the user guide are updated so the feature is fully traced. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Connect/Binding edges are published into the semantic index alongside ordinary reference edges, so the impact verb's reference pass followed every connector a second time: directed instead of undirected, attributed to the raw nested-port endpoint instead of its owning declaration, and outside the connector hop bound. Exclude connector edge kinds from CollectImpactReferences so CollectImpactConnections is their single attribution path. This bounds connector chains to the hop limit the summary already claims, and stops one connector producing two entries when the port sits on the connector's source side. Replace the impact cycle guard's membership set with a minimum-hop dictionary and a shared TryReach helper. Connector-hop cost is not monotonic in breadth-first depth, so an element first reached over a connector may later be reached over a pure reference path with its hop budget restored. Re-expand on a strictly cheaper arrival without emitting a second entry, so nothing within the bound is dropped while each element keeps its first-arrival depth and relation attribution. The flag-absent default now reports fewer rows than released versions on models with declared-endpoint connectors. This is a defect correction, not a feature removal: the previous behavior contradicted the documented default and emitted qualified names that cannot be used as query subjects. It also makes --include-connections a strict superset of the default. The change is called out in the user guide, README, and design constraints. Rewrite two mis-named tests onto fixtures that can actually fail, since every existing connector fixture puts both endpoints on nested ports and therefore never reaches the defective path. Add fixtures and tests for both user-reported models and for the hop-minimality case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restate SysML2Tools-Core-Query-ImpactHopMinimality as observable behavior rather than cycle-guard mechanism, and move the guard, minimum-hop record, and re-expansion detail out of its justification while keeping the connector-hop non-monotonicity rationale. Scope the "not resolvable as query subjects" claim to nested-port endpoints, since declared endpoint names such as Model::System::b do resolve, and replace the appeal to a "documented default contract" with an intrinsic characterization of the defect, because that contract text never shipped in a released user guide. Reframe the user guide upgrade note, which anachronistically described prior releases as following connectors "even without --include-connections" when that flag did not exist, while retaining the warning strength. Restate the verification acceptance criterion to match the requirement and lead the hop-minimality traceability row with its public entry point. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the compound Core -ImpactConnections requirement with five atomic requirements (-ImpactConnections, -ImpactConnectionEndpoints, -ImpactConnectionRollUp, -ImpactConnectionHopBound, -ImpactConnectionCycles) and the compound -EntryTraversalMetadata requirement with three (-EntryTraversalMetadata, -EntryMetadataJsonOmission, -EntryRelationSerialization), redistributing every test registration. The "never additionally as a reference edge" and "exactly once" clauses are dropped rather than re-homed because -Impact and -ImpactHopMinimality already own them; the exactly-once test moves to -ImpactHopMinimality. Split the Tool -IncludeConnections requirement into -IncludeConnections and -IncludeConnectionsHelp, expressed purely as observable CLI behavior with Core-internal detail removed from both title and justification. Close the general-help traceability gap with a new Tool test asserting that 'query --help' documents --include-connections. Correct the Tool design's help-block description, which named a resx key that does not exist, and correct the user guide's claim that the flag can never alter an existing row: a connector can re-attribute a row to a lower depth and a Connect relation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 introduces and documents the new
--include-connectionsoption for theimpactquery verb, clarifies and corrects the traversal semantics for connector edges, and adds new traversal-metadata fields to query results. It also updates documentation to reflect these changes, corrects previous inaccuracies about connector edge handling, and ensures help text and CLI behavior are consistent across subsystems.Key changes include:
Impact Query Semantics and Traversal
--include-connectionsoption to theimpactquery, allowing opt-in, undirected traversal ofconnect/bindedges, bounded by--walk-depth(default 1 hop), and clarified that defaultimpacttraversal is reference-only. [1] [2] [3] [4]SemanticIndex.AllEdges, but are now only traversed forimpactwhen explicitly requested, preventing unbounded and misattributed connector traversal. [1] [2]Query Result Structure
QueryResultEntry:Depth(traversal depth),Relation(relationship kind, string-serialized), andViaQualifiedName(actual endpoint for rolled-up connections), all omitted from JSON when null to preserve payload shape for non-traversing verbs. [1] [2] [3]CLI and Tooling Integration
--include-connectionsas a pass-through option in the Tool CLI, with help text sourced from resources and tested for parity with Core, ensuring discoverability and consistent documentation. [1] [2]Documentation and Traceability
impactandused-bysemantics post-correction.These changes make connector traversal in
impactexplicit, bounded, and correctly attributed, improving both the accuracy of results and the clarity of the API and CLI.