diff --git a/docs/visualizer-msagl-layout-migration/plan.md b/docs/visualizer-msagl-layout-migration/plan.md index 4771a7fb716..b2bcb938def 100644 --- a/docs/visualizer-msagl-layout-migration/plan.md +++ b/docs/visualizer-msagl-layout-migration/plan.md @@ -52,10 +52,11 @@ This folder holds the request/notification handlers, the graph service, the diff - Add `Msagl` to central package management in `src/Directory.Packages.props`. - Reference it from `src/Bicep.LangServer/Bicep.LangServer.csproj`. -- Introduce internal services under `Features/Custom/Visualization/`, conceptually shaped as: - - `IVisualizerGraphService`: builds the canonical graph and computes diffs against a submitted client graph. - - `IVisualizerLayoutEngine`: maps canonical graph to MSAGL geometry graph and returns node positions. - - `MsaglVisualizerLayoutEngine`: invokes core MSAGL layout only; React still renders all visuals. +- Introduce internal services under `Features/Custom/Visualization/`, shaped as: + - `VisualGraphBuilder`: builds the canonical graph from the live compilation. + - `VisualGraphDiffer`: computes the topology/metadata patch delta against a submitted client graph (and exposes `HasTopologyChange` for the layout handler's validation). + - `IVisualGraphLayoutEngine`: maps canonical graph to MSAGL geometry graph and returns node positions. + - `MsaglVisualGraphLayoutEngine`: invokes core MSAGL layout only; React still renders all visuals. - Do not reference MSAGL from the React app or VS Code extension host. ### How This Maps Onto The Existing App @@ -105,12 +106,29 @@ sequenceDiagram alt a request is already in flight UI->>UI: mark dirty, wait for the in-flight response else idle - UI->>Ext: getGraphUpdate(current nodes + edges) - Ext->>LS: textDocument/visualizerGraphUpdate(current) - LS->>LS: build graph from live compilation, diff vs current, lay out changed topology - LS-->>Ext: patches (complete delta; empty if nothing changed) - Ext-->>UI: patches - UI->>UI: apply all patches; if dirty, send another request + UI->>Ext: getGraphUpdate(current topology) + Ext->>LS: textDocument/visualGraphUpdate(current topology) + LS->>LS: build graph from live compilation, diff vs current topology + LS-->>Ext: topology/metadata patches + Ext-->>UI: topology/metadata patches + UI->>UI: apply graph patches and decide whether layout may be stale + opt layout may be stale + UI->>UI: render nodes, measure actual node sizes, compare last layout input + UI->>Ext: getGraphLayout(measured current graph) + Ext->>LS: textDocument/visualGraphLayout(measured current graph) + LS->>LS: rebuild live graph and validate measured topology still matches + alt topology still matches + LS->>LS: run MSAGL with measured sizes + LS-->>Ext: setNodeLayout patches + Ext-->>UI: setNodeLayout patches + UI->>UI: apply layout patches and fit view + else graph changed while measuring/layouting + LS-->>Ext: graphChanged + Ext-->>UI: graphChanged + UI->>UI: mark dirty and restart with getGraphUpdate + end + end + UI->>UI: if dirty, send another graph update request end ``` @@ -123,13 +141,13 @@ The first draft of this design carried `graphRevision`, `layoutRevision`, a `top Together these mean responses do not have to be applied in order: if the client only ever applies the response to its newest request and discards older ones, the result is always correct. There is nothing to reconcile, so there are no conflict states. -A separate `layoutRevision` was meant to catch the case where the client holds new topology but stale positions. That cannot happen here: MSAGL layout is deterministic for a given topology, and the server always returns layout patches together with the topology change that requires them. The client can never end up with topology the server has not also laid out. The graph the client holds is therefore the only "version" the protocol needs, and the client already holds it. +A separate `layoutRevision` was meant to catch the case where the client holds new topology but stale positions. The revised protocol handles this without shared revision state by making layout an explicit second request. The layout request carries the measured graph topology and actual rendered sizes. Before running MSAGL, the server rebuilds the live canonical graph and verifies that the measured topology still matches. If it does not, the server returns `graphChanged` and the client restarts with a graph update request. The server still does not need to remember a revision; it validates the request body against the current compilation. `changeReason` is dropped for the same reason the existing `handleDiagnostics` middleware does not carry one: the notification only means "the graph may have changed, ask for an update." The server recomputes from the live compilation regardless of why it changed. ### Protocol Spec -Three messages. The webview is already bound to a single document, so the document URI is the only routing information required. +Four messages. The webview is already bound to a single document, so the document URI is the only routing information required. ```ts // Extension -> webview. "The graph may have changed; request an update when ready." @@ -139,28 +157,52 @@ interface DocumentDidChangeNotification { params: { documentUri: string }; } -// Webview -> extension -> language server. Carries the graph the webview currently displays. -// `current` is null (or empty) on first load. +// Webview -> extension -> language server. Carries the topology the webview currently displays. +// `current` is null (or empty) on first load. This request does not run layout; it only reconciles +// graph topology and metadata. The client decides whether the returned patches may stale layout. interface GetGraphUpdateRequest { method: "visualizer/getGraphUpdate"; params: { textDocument: { uri: string }; current: { - nodes: ClientGraphNode[]; + nodes: ClientGraphNodeIdentity[]; edges: ClientGraphEdge[]; } | null; }; } // Language server -> extension -> webview. -// A complete delta transforming `current` into the server's latest graph. +// A complete delta transforming `current` into the server's latest graph topology/metadata. // An empty `patches` array means nothing changed. interface GetGraphUpdateResponse { patches: GraphPatch[]; } + +// Webview -> extension -> language server. Sent only after the graph update patches have been +// applied and the webview has rendered/measured node sizes. +interface GetGraphLayoutRequest { + method: "visualizer/getGraphLayout"; + params: { + textDocument: { uri: string }; + current: { + nodes: ClientGraphNodeMeasurement[]; + edges: ClientGraphEdge[]; + }; + options?: VisualGraphLayoutOptions; + }; +} + +// Language server -> extension -> webview. +// If `status` is `ok`, `patches` contains only layout patches (`setNodeLayout`). +// If `status` is `graphChanged`, the measured topology no longer matches the live compilation; +// the client discards the layout response and immediately requests a new graph update. +interface GetGraphLayoutResponse { + status: "ok" | "graphChanged" | "layoutFailed"; + patches: GraphPatch[]; +} ``` -`ClientGraphNode` and `ClientGraphEdge` are the minimal identity the server needs to diff and to decide whether layout must run: node `id`, `kind`, `parentId`, layout-affecting `size`, and edge `id`/`sourceId`/`targetId`. The client does not send positions back; the server does not need them to compute the next layout, and omitting them keeps the request small. +`ClientGraphNodeIdentity` and `ClientGraphEdge` are the minimal identity the server needs to diff topology: node `id`, `kind`, `parentId`, and edge `id`/`sourceId`/`targetId`. `ClientGraphNodeMeasurement` extends the same node identity with layout-affecting `width` and `height`. The client does not send positions back; the server only needs topology, measured sizes, and layout options to compute the next layout. ### Concurrency: Single In-Flight Request @@ -168,16 +210,17 @@ This replaces the optimistic-concurrency machinery with the convergence pattern Per open visualizer the webview keeps: -- one in-flight `getGraphUpdate` request, and +- one in-flight visualizer request (either graph update or layout), and - a `dirty` flag. Rules: -- On `documentDidChange`: if a request is in flight, set `dirty`; otherwise send a request with the currently displayed graph. -- On response: apply all patches (the response's base is the graph that was sent, which is still what is displayed, because nothing else mutates the displayed graph between request and response). Then, if `dirty`, clear it and send a fresh request with the now-current graph. +- On `documentDidChange`: if a request is in flight, set `dirty`; otherwise send a graph update request with the currently displayed topology. +- On graph update response: apply topology/metadata patches as a unit and track whether any patch may affect layout. Topology patches (`clearGraph`, add/remove node, add/remove edge) always may affect layout. Metadata patches only may affect layout when they touch rendered-size-affecting fields such as `type`, `isCollection`, or `hasChildren`; `hasError`, source ranges, and file paths do not trigger layout. If layout may be stale, render the graph, wait for node measurement, compare the measured graph against the last graph used for layout, and send a layout request only if that measured layout input changed. If no layout is required and `dirty` is set, clear it and send a fresh graph update. +- On layout response: if `status` is `ok`, apply `setNodeLayout` patches as a unit and fit the view. If `status` is `graphChanged`, mark `dirty` and send a fresh graph update. If `status` is `layoutFailed`, keep existing positions and wait for the next graph update/manual retry. - A short debounce (the existing render debounce, ~75-150 ms) coalesces bursts of notifications into a single request. -This guarantees that when a response is applied its base equals the current displayed graph, so stale application is impossible without any version checks. If the webview additionally tags requests with a local monotonic sequence number, it can also defensively ignore a late response from a superseded request — but the single-in-flight rule already prevents that situation from arising. +This guarantees that when a response is applied its base equals the graph submitted for that request, so stale application is impossible without shared version checks. The layout response adds one validation point: if the measured graph no longer matches the live compilation, the server refuses to lay it out and asks the client to restart from graph update. ### Forward Compatibility: A Writable Webview @@ -199,9 +242,10 @@ We intentionally do **not** design that handshake now — it would be speculativ ### Canonical Graph Representation +This is the implemented contract (mirrored by the C# records under `Features/Custom/Visualization/Models` and the TypeScript interfaces in `messages.ts`). It is intentionally minimal: the node stores neither size nor position. Sizes are owned by the webview and submitted back via `RenderedGraph`; positions are applied separately via `setNodeLayout` patches. + ```ts -interface VisualizerGraph { - documentUri: string; +interface CanonicalGraph { errorCount: number; nodes: GraphNode[]; edges: GraphEdge[]; @@ -209,62 +253,57 @@ interface VisualizerGraph { interface GraphNode { id: string; - kind: "resource" | "module" | "extension" | "unknown"; - parentId?: string; + kind: "resource" | "module"; + parentId: string | null; type: string; symbolName: string; isCollection: boolean; hasChildren: boolean; hasError: boolean; - source: { - filePath: string | null; - range: Range; - }; - visual: { - size: { width: number; height: number }; - styleKey?: string; - }; - layout?: NodeLayout; - metadata?: Record; + filePath: string | null; + range: Range; } interface GraphEdge { id: string; sourceId: string; targetId: string; - kind: "dependency" | "containment" | "implicit"; - metadata?: Record; } interface NodeLayout { x: number; y: number; - width: number; - height: number; } ``` +`kind` is `"resource" | "module"` only. Earlier drafts of this plan listed an `"extension" | "unknown"` superset and a richer node (nested `source`/`visual` objects, an embedded `layout`, a free-form `metadata` bag); none of that was built. The implemented node keeps `filePath`/`range` flat and omits size/style entirely, matching the existing deployment-graph contract. Widening `kind` or adding fields later is additive and does not change the protocol envelope. + +`NodeLayout` is position-only (`x`/`y` in graph coordinates); width and height are the client's measured values, never round-tripped through the server. + Edges carry no route geometry. The current visualizer draws straight edges entirely on the client — `StraightEdge.tsx` takes the two node boxes, draws a segment between their centers, and clips it to the box boundaries — so it never consumes layout-engine edge routes (bend points/splines). Passing server routes would add protocol surface and a `setEdgeRoute` patch for geometry the client recomputes anyway, and which would otherwise need re-syncing every time a node moves. If curved or orthogonal routing is ever wanted, add `route` then. +Edges also carry no `kind`: containment (parent/child) is expressed via a node's `parentId`, and every remaining edge is a dependency, so a discriminator would be redundant today. Add one only if a second edge semantic appears. + Ports are not required for parity with the current visualizer, which uses node-to-node dependency edges. Add ports later only if edge attachment points become meaningful for sub-resources, resource properties, or grouped module boundaries. ### Patch Format -The response is a plain ordered list of typed patches — no wrapper, no revisions, no patch-set identity. A typed list is preferred over generic JSON Patch because the graph has domain-specific invariants (containment ordering, edge endpoint validity) that typed patches make explicit. +Responses carry plain ordered lists of typed patches — no revisions, no patch-set identity. A typed list is preferred over generic JSON Patch because the graph has domain-specific invariants (containment ordering, edge endpoint validity) that typed patches make explicit. The graph update response carries topology/metadata patches; the client derives whether layout may be stale while applying them. The layout response carries only `setNodeLayout` patches. ```ts type GraphPatch = | { op: "clearGraph" } | { op: "addNode"; node: GraphNode } | { op: "removeNode"; nodeId: string } - | { op: "updateNode"; nodeId: string; changes: Partial } + | { op: "updateNode"; nodeId: string; changes: GraphNodeChanges } | { op: "addEdge"; edge: GraphEdge } | { op: "removeEdge"; edgeId: string } - | { op: "updateEdge"; edgeId: string; changes: Partial } | { op: "setNodeLayout"; nodeId: string; layout: NodeLayout } | { op: "setErrorCount"; errorCount: number }; ``` +`GraphNodeChanges` is the mutable, topology-preserving subset of a node — `type`, `isCollection`, `hasChildren`, `hasError`, `filePath`, `range` — each optional; an omitted field is left unchanged. There is no `updateEdge`: edges are immutable value objects (id/source/target), so an edge change is expressed as `removeEdge` + `addEdge` rather than an in-place mutation. + The entire list for one response is applied as a unit: the client applies all patches, then re-renders. Because the list is a complete delta from the graph the client submitted, there is no per-patch precondition to check. ### Patch Ordering @@ -274,17 +313,16 @@ The server returns patches already ordered so they apply cleanly against the sub - Remove edges. - Remove nodes deepest-first. - Add/update nodes. -- Add/update edges. -- Apply node layout. +- Add edges. - Set error count. -Initial load is just the build-from-empty case of this same ordering: `addNode`, `addEdge`, then layout patches. +Initial graph update is just the build-from-empty case of this same ordering: `addNode`, `addEdge`, then `setErrorCount`. Those add patches cause the client to render and measure the nodes, then the layout request returns `setNodeLayout` patches. ### Layout-Only Versus Topology Patches The server decides per request what to include: -- Topology changed: include add/remove/update node and edge patches plus `setNodeLayout` for the new and affected nodes. +- Topology changed: graph update includes add/remove/update node and edge patches; the client treats those patches as layout-affecting, renders/measures, and requests layout when the measured layout input differs from the last one used for layout. - Topology unchanged, only metadata changed (error state, source ranges, file paths): include only `updateNode`/`setErrorCount` and omit all position patches, so the client keeps its current positions and nothing reflows. - Nothing changed: return an empty list. @@ -298,7 +336,7 @@ Drag/drop is out of scope for this migration and is not part of this protocol. T ### Layout Triggers And Topology Changes -Run MSAGL when: +Request and run MSAGL when: - Node set changes. - Edge set changes. @@ -313,20 +351,19 @@ Do not run MSAGL when: - Only source ranges change. - Only file paths change. - Diagnostics count changes without affecting node visual size. -- The submitted topology already matches the freshly built topology. +- The submitted topology already matches the freshly built topology and measured node sizes/layout options have not changed. ### MSAGL Constraints And Options - Use core MSAGL geometry graph APIs from the `Msagl` package, not Drawing, WPF, or WinForms viewer packages. -- Model Bicep resources/modules as MSAGL nodes with deterministic sizes. +- Model Bicep resources/modules as MSAGL nodes with sizes from the submitted rendered graph, falling back to shared `VisualGraphLayoutOptions.Default` values when the client has not measured a node yet. - Model dependencies as directed MSAGL edges. - Use a layered/Sugiyama-style top-to-bottom layout to match current ELK intent: - Direction: top to bottom. - Stable layer ordering by source order and node ID. - Tunable node separation, rank/layer separation, and component separation. - Edges are still modeled in MSAGL so they influence layering and node placement, but the client draws the straight edges itself. Use MSAGL's straight-line edge routing mode (`StraightLineEdges`) so layout does not spend time computing spline routes we discard; only node positions are read out. -- Represent modules/compound nodes as clusters if core MSAGL cluster support is adequate. -- If cluster behavior is not good enough initially, lay out nested module graphs independently and compose them into parent bounding boxes in the server service. +- MSAGL cluster behavior was evaluated and is not reliable enough for the initial adapter, so lay out nested module scopes independently and compose them into parent module boxes in the server service. ### Stable Layouts And Minimal Movement @@ -456,23 +493,26 @@ Do not run MSAGL when: ### Phase 6: React Applies Server Layout - Goal and scope: - - Under feature flag, React applies `setNodeLayout`; disable ELK auto-layout for server-layout sessions. + - Under feature flag, React applies `setNodeLayout`; disable ELK auto-layout for server-layout sessions; split the client/server loop into topology update, render/measure, and measured layout request. - Files/areas likely touched: - `src/vscode-bicep-ui/apps/visual-designer/src/features/layout/use-auto-layout.ts` - `src/vscode-bicep-ui/apps/visual-designer/src/features/layout/elk-layout.ts` - Graph components and edge layer under `src/vscode-bicep-ui/apps/visual-designer/src/lib/graph` + - Visualizer webview/extension/LSP protocol bridge for `getGraphLayout` / `textDocument/visualGraphLayout` - Test plan: - Vitest for patch application. + - Layout request tests for initial topology update followed by measured-size layout. - Playwright visualizer e2e for initial load, edit, delete, module graph, and fit view. - Acceptance criteria: - No client layout runs when the flag is on. + - Initial render uses measured node sizes for the final server layout rather than fallback dimensions. - Graph remains stable through rapid edits. - Fit view still works from server-provided bounds/layout. ### Phase 7: Single-Flight Hardening And Telemetry - Goal and scope: - - Add the single-in-flight + dirty-flag loop, request cancellation, debounce, layout timing, and fallback telemetry. + - Harden the graph-update + measured-layout loop with dirty-flag replay, request cancellation, debounce, layout timing, and fallback telemetry. - Files/areas likely touched: - Extension visualizer bridge. - LSP graph service. @@ -526,10 +566,10 @@ Do not run MSAGL when: - Mitigate with typed patch ops, strict ordering, whole-list application, and tests for every operation. - Node size mismatch between server and React: - - Mitigate by defining deterministic visual size in canonical graph, keeping layout independent of DOM measurement, and adding visual regression tests. + - Mitigate by using client-measured node sizes when available, keeping fallback/default metrics in shared `VisualGraphLayoutOptions.Default`, and adding visual regression tests. - MSAGL compound/module layout gaps: - - Mitigate by validating cluster support early; if needed, compose nested module layouts manually before deeper cluster investment. + - Mitigated initially by evaluating cluster support, avoiding it for Phase 5, and composing nested module scopes manually before deeper cluster investment. - Drag reconciliation complexity: - Out of scope for this migration; deferred to the future drag-n-drop work, which will revisit the protocol (see Forward Compatibility). @@ -549,8 +589,8 @@ Legend: `[ ]` not started, `[~]` in progress, `[x]` done. | 2 | Document change notification loop | [x] | #19792 | Notify-then-request flow behind the flag; old full-graph path intact. Extension forwards `documentDidChange`→webview, webview pulls via `getGraphUpdate`→`textDocument/visualGraphUpdate`; single in-flight + dirty loop; patches applied to a client graph mirror, then translated to `DeploymentGraph` so ELK still lays out. Dev playground gains a server-layout toggle + throwaway TS differ. **Reordered after 3–4** so it forwards the real LSP request instead of a throwaway TS diff. | | 3 | Server graph service | [x] | #19757 | Canonical graph built from compilation (`VisualGraphBuilder`, mirrors `BicepDeploymentGraphHandler`); no MSAGL. Combined with Phase 4 into one PR. Types renamed `Visualizer*`→`Visual*`. | | 4 | Patch diff engine | [x] | #19757 | `VisualGraphDiffer` returns `GraphPatch[]`; handler registered (shadow mode, not flag-gated); ELK still lays out on the client. Combined with Phase 3. | -| 5 | MSAGL adapter (shadow mode) | [ ] | — | Add `Msagl`, compute layout behind the flag, not applied by default. | -| 6 | React applies server layout | [ ] | — | Apply `setNodeLayout`; disable ELK for server-layout sessions. | +| 5 | MSAGL adapter (shadow mode) | [x] | — | `Msagl` 1.2.1 added to central package management + `Bicep.LangServer.csproj`. New `IVisualGraphLayoutEngine` / `MsaglVisualGraphLayoutEngine` run a layered (Sugiyama) top-to-bottom layout via `LayoutHelpers.CalculateLayout`, straight-line routing, y-flipped + origin-normalized positions. Shared defaults live in `VisualGraphLayoutOptions.Default` (default node size, node/layer spacing, module padding) so future callers such as CLI image generation do not duplicate React constants; renderer-provided options can override later. **MSAGL clusters were tried first but throw at runtime**, so containment uses the plan's sanctioned fallback: lay out each scope's siblings flat, size each module box from its subtree + label padding, then compose by offsetting children (exact because the builder only ever emits sibling edges). Failures are caught/logged (empty result keeps prior positions); cancellation is bridged to MSAGL's `CancelToken`. Engine + differ unit tests added, including a simple-graph performance smoke test (<500 ms budget). | +| 6 | React applies server layout | [x] | — | `getGraphUpdate` / `textDocument/visualGraphUpdate` returns topology/metadata patches. The webview decides whether layout may be stale while applying patches (`hasError`, source ranges, and file paths do not trigger layout), renders/measures nodes when needed, compares the measured graph with the last layout input, then sends `getGraphLayout` / `textDocument/visualGraphLayout` only when measured topology/sizes changed. The server validates measured topology against the live compilation before running MSAGL and returns `setNodeLayout` patches (`ok`), `graphChanged`, or `layoutFailed`. The webview applies layout patches, gates automatic ELK with `serverLayoutActiveAtom`, keeps Reset Layout enabled, and routes Reset Layout to a fresh measured server layout in server-layout mode. Because the server is stateless and re-sends all metadata on every `updateNode`, the client derives layout-affecting changes by comparing incoming field values against its current node (not field presence), so range-only edits never reflow. Applying a server layout springs nodes to their new positions (reusing the existing motion animation) and fits the viewport to the same bounds the Fit View button uses, so re-layout, additions, and removals animate and the two controls agree to the pixel. Dev fake channel follows the same two-step protocol. Focused validation: visualization unit tests, visual graph integration tests, app TypeScript check, and Vite bundle. | | 7 | Single-flight hardening + telemetry | [ ] | — | In-flight + dirty loop, cancellation, debounce, metrics. | | 8 | Reserved for future WYSIWYG | [ ] | — | No work planned; placeholder only. | | 9 | Default-on and ELK removal | [ ] | — | Flip default, remove `elkjs`, keep old-path fallback one release. | @@ -591,3 +631,24 @@ The guiding rule is the Per-PR Definition of Done below: split wherever a PR wou - `main` remains shippable with the feature flag in its default state. - New baselines (if any) are inspected and explained in the PR. - This table is updated (status + PR link) in the same PR. + +## I) Known Follow-ups From Implementation Review + +Findings from a review of the Phase 5/6 implementation that are intentionally deferred (the protocol absorbs them without change). Tracked here so they are not lost. + +- **O(N) metadata chatter per edit (→ Phase 7).** Because the server is stateless, every `updateNode` re-sends *all* node metadata, so a single edit returns one `updateNode` per surviving node. It is correct (the client compares values, so range-only edits do not reflow) but is O(nodes) of network/JSON per debounced edit. Add a patch-count telemetry counter and a noted ceiling; do **not** add a client metadata hash, which would break statelessness. + + *Proposed fix (O(N) → O(1)):* the only per-edit-churning field is `range` (and the `filePath` that pairs with it) — every other metadata field changes rarely. `range` exists solely so a double-click can reveal the node's source. Remove `range`/`filePath` from the node metadata the server pushes, and instead resolve the source location on demand: when the user double-clicks a node, the webview sends its node id to the language server, which maps it to a source range from the live compilation and reveals it. Then a pure whitespace edit produces *no* `updateNode` patches at all (topology and the remaining metadata are unchanged), collapsing the common case from O(nodes) to O(0). This is a protocol change (drop two fields, add a `revealNode`-style request), so it belongs with the Phase 7 hardening rather than this PR. + +- **Layout runs on the LSP request thread (→ Phase 7).** `VisualGraphLayoutHandler` runs MSAGL inline and returns `Task.FromResult`. The engine takes a `CancellationToken`, but the work is not offloaded (`Task.Run`) and superseded layout requests are not coalesced server-side. Pathological graphs could block the handler. Phase 7's "non-blocking async layout work" covers this. + +- **`measureServerLayoutBounds` transiently mutates shared atoms.** To fit the viewport to a layout's *final* bounds it briefly writes every atomic `boxAtom` to its target, reads the derived `graphBoundsAtom`, then restores. This must stay fully synchronous (no `await` between the writes and the restore) or a paint could flash. The constraint is documented at the definition; preserve it. + + *Proposed fix:* adopt the [pretext](https://github.com/chenglou/pretext) library for synchronous, DOM-free text measurement. With node sizes computable up front (resource node height is fixed; only the text-driven width needs measuring via `measureNaturalWidth` plus our known chrome constants), the client can derive each node's final box arithmetically and compute the layout's bounding box directly — no transient atom writes, no read-back of `graphBoundsAtom`, and no reliance on the set→read→restore staying synchronous. This is the same library noted as a future direction for eliminating the `waitForAnimationFrame` measure step, so the two simplifications share one dependency; track them together (Phase 8 WYSIWYG groundwork). The remaining caveats are font-load timing (`document.fonts.ready`), modeling state-dependent chrome such as the thicker error border, and keeping the chrome math in lockstep with the node CSS. + +- **Partial-layout semantics are implicit.** If MSAGL returns positions for only some nodes, the handler returns `ok` and the unpositioned nodes keep their client positions. Reasonable, but add a server test asserting it so it is not silently changed. + +- **`activeLayoutAnimations` is module-level singleton state** in `use-deployment-graph.ts`. Correct only because there is one webview instance per document; revisit if the app ever hosts multiple graph stores. + +- **Three sites encode "what affects layout."** The client centralizes its two checks in `layout-invalidation.ts` (`patchMayAffectLayout` + `renderedGraphsEqual`), which cross-references the server's `VisualGraphDiffer.HasTopologyChange`. These cannot share code across the C#/TS boundary, so they must be kept consistent by hand when layout-affecting fields change. + diff --git a/src/Bicep.LangServer.IntegrationTests/Features/Visualization/VisualGraphUpdateTests.cs b/src/Bicep.LangServer.IntegrationTests/Features/Visualization/VisualGraphUpdateTests.cs index 8a15cbfe171..384e083820f 100644 --- a/src/Bicep.LangServer.IntegrationTests/Features/Visualization/VisualGraphUpdateTests.cs +++ b/src/Bicep.LangServer.IntegrationTests/Features/Visualization/VisualGraphUpdateTests.cs @@ -26,7 +26,7 @@ public class VisualGraphUpdateTests public TestContext? TestContext { get; set; } [TestMethod] - public async Task Initial_request_with_null_current_returns_a_full_add_delta() + public async Task VisualGraphUpdate_ForNullCurrent_ReturnsFullAddDelta() { using var helper = await StartServerAndOpenAsync(); var client = helper.Helper.Client; @@ -60,11 +60,12 @@ public async Task Initial_request_with_null_current_returns_a_full_add_delta() result.Patches.OfType().Should().BeEmpty(); result.Patches.OfType().Should().BeEmpty(); result.Patches.OfType().Should().BeEmpty(); + result.Patches.OfType().Should().BeEmpty(); result.Patches.OfType().Single().ErrorCount.Should().Be(0); } [TestMethod] - public async Task Request_with_matching_current_returns_only_metadata_refresh() + public async Task VisualGraphUpdate_ForMatchingCurrent_ReturnsOnlyMetadataRefresh() { using var helper = await StartServerAndOpenAsync(); var client = helper.Helper.Client; @@ -92,11 +93,61 @@ public async Task Request_with_matching_current_returns_only_metadata_refresh() result.Patches.OfType().Should().BeEmpty(); result.Patches.OfType().Should().BeEmpty(); result.Patches.OfType().Should().BeEmpty(); + result.Patches.OfType().Should().BeEmpty(); result.Patches.OfType().Select(patch => patch.NodeId) .Should().BeEquivalentTo("res1", "res2", "mod1", "mod1::res3"); result.Patches.OfType().Single().ErrorCount.Should().Be(0); } + [TestMethod] + public async Task VisualGraphLayout_ForMeasuredMatchingCurrent_ReturnsLayoutPatches() + { + using var helper = await StartServerAndOpenAsync(); + var client = helper.Helper.Client; + + var initial = await client.SendRequest( + new VisualGraphUpdateParams(new TextDocumentIdentifier(helper.MainUri), Current: null), + default); + + var renderedNodes = initial.Patches.OfType() + .Select(patch => new RenderedGraphNode(patch.Node.Id, patch.Node.Kind, patch.Node.ParentId, 180, 72)) + .ToList(); + var renderedEdges = initial.Patches.OfType() + .Select(patch => new RenderedGraphEdge(patch.Edge.Id, patch.Edge.SourceId, patch.Edge.TargetId)) + .ToList(); + + var result = await client.SendRequest( + new VisualGraphLayoutParams( + new TextDocumentIdentifier(helper.MainUri), + new RenderedGraph(renderedNodes, renderedEdges), + Options: null), + default); + + result.Status.Should().Be(VisualGraphLayoutStatus.Ok); + result.Patches.Should().AllBeOfType(); + result.Patches.OfType().Select(patch => patch.NodeId) + .Should().BeEquivalentTo("res1", "res2", "mod1", "mod1::res3"); + result.Patches.OfType().Select(patch => patch.Layout) + .Should().OnlyContain(layout => double.IsFinite(layout.X) && double.IsFinite(layout.Y)); + } + + [TestMethod] + public async Task VisualGraphLayout_ForStaleMeasuredCurrent_ReturnsGraphChanged() + { + using var helper = await StartServerAndOpenAsync(); + var client = helper.Helper.Client; + var stale = new RenderedGraph( + Nodes: [new RenderedGraphNode("missing", GraphNodeKind.Resource, ParentId: null, Width: 180, Height: 72)], + Edges: []); + + var result = await client.SendRequest( + new VisualGraphLayoutParams(new TextDocumentIdentifier(helper.MainUri), stale, Options: null), + default); + + result.Status.Should().Be(VisualGraphLayoutStatus.GraphChanged); + result.Patches.Should().BeEmpty(); + } + private async Task StartServerAndOpenAsync() { var diagnosticsListener = new MultipleMessageListener(); diff --git a/src/Bicep.LangServer.UnitTests/Features/Visualization/GraphPatchSerializationTests.cs b/src/Bicep.LangServer.UnitTests/Features/Visualization/GraphPatchSerializationTests.cs index a34cc5e6742..1c59810ffc0 100644 --- a/src/Bicep.LangServer.UnitTests/Features/Visualization/GraphPatchSerializationTests.cs +++ b/src/Bicep.LangServer.UnitTests/Features/Visualization/GraphPatchSerializationTests.cs @@ -28,7 +28,7 @@ private static GraphPatch Deserialize(JToken token) => token.ToObject(Serializer) ?? throw new InvalidOperationException("Deserialization returned null."); [TestMethod] - public void ClearGraph_round_trips_and_emits_camelCase_op() + public void ClearGraph_WhenSerialized_RoundTripsWithCamelCaseOp() { var patch = new GraphPatch.ClearGraph(); @@ -39,7 +39,7 @@ public void ClearGraph_round_trips_and_emits_camelCase_op() } [TestMethod] - public void AddNode_round_trips_with_camelCase_fields() + public void AddNode_WhenSerialized_RoundTripsWithCamelCaseFields() { var node = new GraphNode( Id: "res:foo", @@ -62,7 +62,7 @@ public void AddNode_round_trips_with_camelCase_fields() } [TestMethod] - public void RemoveNode_round_trips() + public void RemoveNode_WhenSerialized_RoundTrips() { var patch = new GraphPatch.RemoveNode("res:foo"); @@ -74,7 +74,7 @@ public void RemoveNode_round_trips() } [TestMethod] - public void UpdateNode_round_trips_with_partial_changes() + public void UpdateNode_WithPartialChanges_RoundTrips() { var patch = new GraphPatch.UpdateNode("res:foo", new GraphNodeChanges(HasError: true)); @@ -85,7 +85,7 @@ public void UpdateNode_round_trips_with_partial_changes() } [TestMethod] - public void AddEdge_round_trips() + public void AddEdge_WhenSerialized_RoundTrips() { var patch = new GraphPatch.AddEdge(new GraphEdge("e1", "res:a", "res:b")); @@ -96,7 +96,7 @@ public void AddEdge_round_trips() } [TestMethod] - public void RemoveEdge_round_trips() + public void RemoveEdge_WhenSerialized_RoundTrips() { var patch = new GraphPatch.RemoveEdge("e1"); @@ -108,7 +108,7 @@ public void RemoveEdge_round_trips() } [TestMethod] - public void SetNodeLayout_round_trips() + public void SetNodeLayout_WhenSerialized_RoundTrips() { var patch = new GraphPatch.SetNodeLayout("res:foo", new NodeLayout(12.5, -3.25)); @@ -119,7 +119,7 @@ public void SetNodeLayout_round_trips() } [TestMethod] - public void SetErrorCount_round_trips_with_camelCase_op() + public void SetErrorCount_WhenSerialized_RoundTripsWithCamelCaseOp() { var patch = new GraphPatch.SetErrorCount(3); @@ -131,7 +131,7 @@ public void SetErrorCount_round_trips_with_camelCase_op() } [TestMethod] - public void Unknown_op_throws() + public void Deserialize_WithUnknownOp_Throws() { var json = new JObject { ["op"] = "bogus" }; diff --git a/src/Bicep.LangServer.UnitTests/Features/Visualization/MsaglVisualGraphLayoutEngineTests.cs b/src/Bicep.LangServer.UnitTests/Features/Visualization/MsaglVisualGraphLayoutEngineTests.cs new file mode 100644 index 00000000000..e502537c4aa --- /dev/null +++ b/src/Bicep.LangServer.UnitTests/Features/Visualization/MsaglVisualGraphLayoutEngineTests.cs @@ -0,0 +1,256 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Diagnostics; +using Bicep.LanguageServer.Features.Custom.Visualization; +using Bicep.LanguageServer.Features.Custom.Visualization.Models; +using FluentAssertions; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Range = OmniSharp.Extensions.LanguageServer.Protocol.Models.Range; + +namespace Bicep.LangServer.UnitTests.Features.Visualization; + +[TestClass] +public class MsaglVisualGraphLayoutEngineTests +{ + [TestMethod] + public void Layout_ForEmptyGraph_ReturnsNoPositions() + { + var engine = CreateEngine(); + + var layout = engine.Layout(new CanonicalGraph([], [], ErrorCount: 0), NoSizes, DefaultOptions, CancellationToken.None); + + layout.Should().BeEmpty(); + } + + [TestMethod] + public void Layout_ForConnectedGraph_PositionsEveryNode() + { + var engine = CreateEngine(); + var graph = new CanonicalGraph( + Nodes: [Node("a"), Node("b"), Node("c")], + Edges: [Edge("a", "b"), Edge("b", "c")], + ErrorCount: 0); + + var layout = engine.Layout(graph, NoSizes, DefaultOptions, CancellationToken.None); + + layout.Keys.Should().BeEquivalentTo("a", "b", "c"); + layout.Values.Should().OnlyContain(position => IsFinite(position)); + } + + [TestMethod] + public void Layout_ForGraph_PlacesTopLeftCornerAtOrigin() + { + var engine = CreateEngine(); + var graph = new CanonicalGraph( + Nodes: [Node("a"), Node("b"), Node("c")], + Edges: [Edge("a", "b"), Edge("a", "c")], + ErrorCount: 0); + + var layout = engine.Layout(graph, NoSizes, DefaultOptions, CancellationToken.None); + + // The graph's left-most and top-most edges sit at the origin; pan/zoom and centering stay on the client. + layout.Values.Min(position => position.X).Should().BeApproximately(0, Tolerance); + layout.Values.Min(position => position.Y).Should().BeApproximately(0, Tolerance); + } + + [TestMethod] + public void Layout_ForDependencyEdge_PlacesEndpointsOnDifferentLayers() + { + var engine = CreateEngine(); + var graph = new CanonicalGraph( + Nodes: [Node("a"), Node("b")], + Edges: [Edge("a", "b")], + ErrorCount: 0); + + var layout = engine.Layout(graph, NoSizes, DefaultOptions, CancellationToken.None); + + // A layered top-to-bottom layout separates the two endpoints vertically. + layout["a"].Y.Should().NotBe(layout["b"].Y); + } + + [TestMethod] + public void Layout_ForSameInput_ProducesIdenticalPositions() + { + var engine = CreateEngine(); + var graph = new CanonicalGraph( + Nodes: [Node("a"), Node("b"), Node("c"), Node("d")], + Edges: [Edge("a", "b"), Edge("a", "c"), Edge("b", "d"), Edge("c", "d")], + ErrorCount: 0); + + var first = engine.Layout(graph, NoSizes, DefaultOptions, CancellationToken.None); + var second = engine.Layout(graph, NoSizes, DefaultOptions, CancellationToken.None); + + second.Should().BeEquivalentTo(first); + } + + [TestMethod] + public void Layout_ForNestedModule_PositionsModuleAndChildren() + { + var engine = CreateEngine(); + var graph = new CanonicalGraph( + Nodes: + [ + Node("root"), + Node("mod", GraphNodeKind.Module, hasChildren: true), + Node("mod::child1", parentId: "mod"), + Node("mod::child2", parentId: "mod"), + ], + Edges: [Edge("root", "mod"), Edge("mod::child1", "mod::child2")], + ErrorCount: 0); + + var layout = engine.Layout(graph, NoSizes, DefaultOptions, CancellationToken.None); + + layout.Keys.Should().BeEquivalentTo("root", "mod", "mod::child1", "mod::child2"); + layout.Values.Should().OnlyContain(position => IsFinite(position)); + } + + [TestMethod] + public void Layout_WithClientMeasuredSizes_PositionsEveryNode() + { + var engine = CreateEngine(); + var graph = new CanonicalGraph( + Nodes: [Node("a"), Node("b")], + Edges: [Edge("a", "b")], + ErrorCount: 0); + var sizes = Sizes(("a", 400, 300), ("b", 400, 300)); + + var layout = engine.Layout(graph, sizes, DefaultOptions, CancellationToken.None); + + // Larger nodes simply produce a valid, fully-populated layout; sizing is an input, not an output. + layout.Keys.Should().BeEquivalentTo("a", "b"); + layout.Values.Should().OnlyContain(position => IsFinite(position)); + } + + [TestMethod] + public void Layout_WithCancelledToken_ThrowsOperationCancelled() + { + var engine = CreateEngine(); + var graph = new CanonicalGraph(Nodes: [Node("a")], Edges: [], ErrorCount: 0); + using var cancellation = new CancellationTokenSource(); + cancellation.Cancel(); + + var act = () => engine.Layout(graph, NoSizes, DefaultOptions, cancellation.Token); + + act.Should().Throw(); + } + + [TestMethod] + public void Layout_WithCustomModulePadding_OffsetsChildrenByPadding() + { + var engine = CreateEngine(); + var graph = new CanonicalGraph( + Nodes: + [ + Node("mod", GraphNodeKind.Module, hasChildren: true), + Node("mod::child", parentId: "mod"), + ], + Edges: [], + ErrorCount: 0); + var options = new VisualGraphLayoutOptions( + defaultNodeSize: new NodeSize(100, 80), + nodeSeparation: 10, + layerSeparation: 20, + modulePadding: new ModulePadding(top: 17, right: 31, bottom: 23, left: 29)); + + var layout = engine.Layout(graph, NoSizes, options, CancellationToken.None); + + layout["mod"].Should().Be(new NodeLayout(0, 0)); + layout["mod::child"].Should().Be(new NodeLayout(29, 17)); + } + + [TestMethod] + public void Layout_ForSimpleGraph_CompletesWithinSmokeBudget() + { + var engine = CreateEngine(); + var graph = BuildRepresentativeGraph(moduleCount: 4, resourcesPerModule: 4, topLevelResourceCount: 8); + var stopwatch = Stopwatch.StartNew(); + + var layout = engine.Layout(graph, NoSizes, DefaultOptions, CancellationToken.None); + + stopwatch.Stop(); + layout.Keys.Should().BeEquivalentTo(graph.Nodes.Select(node => node.Id)); + layout.Values.Should().OnlyContain(position => IsFinite(position)); + stopwatch.Elapsed.Should().BeLessThan(TimeSpan.FromMilliseconds(500)); + } + + private const double Tolerance = 1e-6; + + private static readonly VisualGraphLayoutOptions DefaultOptions = VisualGraphLayoutOptions.Default; + + private static readonly IReadOnlyDictionary NoSizes = + new Dictionary(StringComparer.Ordinal); + + private static MsaglVisualGraphLayoutEngine CreateEngine() => + new(NullLogger.Instance); + + private static bool IsFinite(NodeLayout position) => + double.IsFinite(position.X) && double.IsFinite(position.Y); + + private static IReadOnlyDictionary Sizes(params (string Id, double Width, double Height)[] entries) => + entries.ToDictionary(entry => entry.Id, entry => new NodeSize(entry.Width, entry.Height), StringComparer.Ordinal); + + private static CanonicalGraph BuildRepresentativeGraph( + int moduleCount, + int resourcesPerModule, + int topLevelResourceCount) + { + var nodes = new List(); + var edges = new List(); + + for (var index = 0; index < topLevelResourceCount; index++) + { + nodes.Add(Node($"top{index}")); + + if (index > 0) + { + edges.Add(Edge($"top{index - 1}", $"top{index}")); + } + } + + for (var moduleIndex = 0; moduleIndex < moduleCount; moduleIndex++) + { + var moduleId = $"mod{moduleIndex}"; + nodes.Add(Node(moduleId, GraphNodeKind.Module, hasChildren: true)); + + if (topLevelResourceCount > 0) + { + edges.Add(Edge($"top{moduleIndex % topLevelResourceCount}", moduleId)); + } + + for (var resourceIndex = 0; resourceIndex < resourcesPerModule; resourceIndex++) + { + var resourceId = $"{moduleId}::res{resourceIndex}"; + nodes.Add(Node(resourceId, parentId: moduleId)); + + if (resourceIndex > 0) + { + edges.Add(Edge($"{moduleId}::res{resourceIndex - 1}", resourceId)); + } + } + } + + return new CanonicalGraph(nodes, edges, ErrorCount: 0); + } + + private static GraphNode Node( + string id, + string kind = GraphNodeKind.Resource, + string? parentId = null, + bool hasChildren = false) => + new( + Id: id, + Kind: kind, + ParentId: parentId, + Type: "Test.Rp/tests", + SymbolName: id, + IsCollection: false, + HasChildren: hasChildren, + HasError: false, + FilePath: "/main.bicep", + Range: new Range(0, 0, 0, 0)); + + private static GraphEdge Edge(string sourceId, string targetId) => + new(Id: $"{sourceId}->{targetId}", SourceId: sourceId, TargetId: targetId); +} diff --git a/src/Bicep.LangServer.UnitTests/Features/Visualization/VisualGraphDifferTests.cs b/src/Bicep.LangServer.UnitTests/Features/Visualization/VisualGraphDifferTests.cs index 87a3c187a44..a2a795e117c 100644 --- a/src/Bicep.LangServer.UnitTests/Features/Visualization/VisualGraphDifferTests.cs +++ b/src/Bicep.LangServer.UnitTests/Features/Visualization/VisualGraphDifferTests.cs @@ -13,7 +13,7 @@ namespace Bicep.LangServer.UnitTests.Features.Visualization; public class VisualGraphDifferTests { [TestMethod] - public void Diff_against_null_current_adds_every_node_and_edge_parents_first() + public void Diff_AgainstNullCurrent_AddsEveryNodeAndEdgeParentsFirst() { var target = new CanonicalGraph( Nodes: @@ -39,7 +39,7 @@ public void Diff_against_null_current_adds_every_node_and_edge_parents_first() } [TestMethod] - public void Diff_removes_nodes_absent_from_target_deepest_first() + public void Diff_WhenNodesAbsentFromTarget_RemovesDeepestFirst() { var current = new RenderedGraph( Nodes: @@ -59,7 +59,7 @@ public void Diff_removes_nodes_absent_from_target_deepest_first() } [TestMethod] - public void Diff_treats_reparented_node_as_remove_then_add() + public void Diff_ForReparentedNode_EmitsRemoveThenAdd() { var current = new RenderedGraph( Nodes: @@ -85,7 +85,7 @@ public void Diff_treats_reparented_node_as_remove_then_add() } [TestMethod] - public void Diff_treats_kind_change_as_remove_then_add() + public void Diff_ForKindChange_EmitsRemoveThenAdd() { var current = new RenderedGraph( Nodes: [Rendered("x", GraphNodeKind.Resource, parentId: null)], @@ -103,7 +103,7 @@ public void Diff_treats_kind_change_as_remove_then_add() } [TestMethod] - public void Diff_refreshes_surviving_nodes_with_metadata() + public void Diff_ForSurvivingNodes_RefreshesMetadata() { var current = new RenderedGraph( Nodes: [Rendered("res", GraphNodeKind.Resource, parentId: null)], @@ -124,7 +124,7 @@ public void Diff_refreshes_surviving_nodes_with_metadata() } [TestMethod] - public void Diff_adds_and_removes_edges_by_id() + public void Diff_WhenEdgesChange_AddsAndRemovesEdgesById() { var current = new RenderedGraph( Nodes: @@ -149,7 +149,7 @@ public void Diff_adds_and_removes_edges_by_id() } [TestMethod] - public void Diff_removes_edges_before_removing_nodes() + public void Diff_WhenRemovingNodes_RemovesEdgesFirst() { var current = new RenderedGraph( Nodes: @@ -168,7 +168,7 @@ public void Diff_removes_edges_before_removing_nodes() } [TestMethod] - public void Diff_always_emits_set_error_count_last() + public void Diff_ForAnyGraph_EmitsSetErrorCountLast() { var target = new CanonicalGraph( Nodes: [Node("res", GraphNodeKind.Resource, parentId: null)], @@ -181,6 +181,171 @@ public void Diff_always_emits_set_error_count_last() .Which.ErrorCount.Should().Be(4); } + [TestMethod] + public void Diff_WithLayout_EmitsOrderedSetNodeLayoutBeforeErrorCount() + { + var target = new CanonicalGraph( + Nodes: + [ + Node("a", GraphNodeKind.Resource, parentId: null), + Node("b", GraphNodeKind.Resource, parentId: null), + ], + Edges: [], + ErrorCount: 0); + var layout = new Dictionary + { + ["b"] = new NodeLayout(30, 40), + ["a"] = new NodeLayout(10, 20), + }; + + var patches = VisualGraphDiffer.Diff(current: null, target, layout); + + patches.OfType().Select(patch => patch.NodeId).Should().Equal("a", "b"); + patches.OfType().Single(patch => patch.NodeId == "a").Layout.Should().Be(new NodeLayout(10, 20)); + + // Layout patches come after the structural patches and before the trailing error-count patch. + var lastLayout = patches.ToList().FindLastIndex(patch => patch is GraphPatch.SetNodeLayout); + var errorCount = patches.ToList().FindIndex(patch => patch is GraphPatch.SetErrorCount); + lastLayout.Should().BeLessThan(errorCount); + } + + [TestMethod] + public void Diff_WithoutLayout_EmitsNoSetNodeLayout() + { + var target = new CanonicalGraph( + Nodes: [Node("a", GraphNodeKind.Resource, parentId: null)], + Edges: [], + ErrorCount: 0); + + var patches = VisualGraphDiffer.Diff(current: null, target); + + patches.OfType().Should().BeEmpty(); + } + + [TestMethod] + public void Diff_WithPartialLayout_EmitsLayoutOnlyForPresentNodes() + { + var target = new CanonicalGraph( + Nodes: + [ + Node("a", GraphNodeKind.Resource, parentId: null), + Node("b", GraphNodeKind.Resource, parentId: null), + ], + Edges: [], + ErrorCount: 0); + var layout = new Dictionary { ["a"] = new NodeLayout(1, 2) }; + + var patches = VisualGraphDiffer.Diff(current: null, target, layout); + + patches.OfType().Select(patch => patch.NodeId).Should().Equal("a"); + } + + [TestMethod] + public void HasTopologyChange_WithNullCurrent_ReturnsTrue() + { + var target = new CanonicalGraph([Node("a", GraphNodeKind.Resource, parentId: null)], [], ErrorCount: 0); + + VisualGraphDiffer.HasTopologyChange(current: null, target).Should().BeTrue(); + } + + [TestMethod] + public void HasTopologyChange_WithIdenticalTopology_ReturnsFalse() + { + var current = new RenderedGraph( + Nodes: + [ + Rendered("a", GraphNodeKind.Resource, parentId: null), + Rendered("b", GraphNodeKind.Resource, parentId: null), + ], + Edges: [RenderedEdge("a->b", "a", "b")]); + var target = new CanonicalGraph( + Nodes: + [ + Node("a", GraphNodeKind.Resource, parentId: null), + Node("b", GraphNodeKind.Resource, parentId: null), + ], + Edges: [Edge("a->b", "a", "b")], + ErrorCount: 0); + + VisualGraphDiffer.HasTopologyChange(current, target).Should().BeFalse(); + } + + [TestMethod] + public void HasTopologyChange_WithMetadataOnlyChange_ReturnsFalse() + { + var current = new RenderedGraph( + Nodes: [Rendered("a", GraphNodeKind.Resource, parentId: null)], + Edges: []); + + // Same id, kind, and parent, but a different type and error state: a metadata-only change. + var target = new CanonicalGraph( + Nodes: [Node("a", GraphNodeKind.Resource, parentId: null, type: "Microsoft.Storage/storageAccounts", hasError: true)], + Edges: [], + ErrorCount: 1); + + VisualGraphDiffer.HasTopologyChange(current, target).Should().BeFalse(); + } + + [TestMethod] + public void HasTopologyChange_WithAddedNode_ReturnsTrue() + { + var current = new RenderedGraph(Nodes: [Rendered("a", GraphNodeKind.Resource, parentId: null)], Edges: []); + var target = new CanonicalGraph( + Nodes: + [ + Node("a", GraphNodeKind.Resource, parentId: null), + Node("b", GraphNodeKind.Resource, parentId: null), + ], + Edges: [], + ErrorCount: 0); + + VisualGraphDiffer.HasTopologyChange(current, target).Should().BeTrue(); + } + + [TestMethod] + public void HasTopologyChange_WithReparentedNode_ReturnsTrue() + { + var current = new RenderedGraph( + Nodes: + [ + Rendered("m", GraphNodeKind.Module, parentId: null), + Rendered("res", GraphNodeKind.Resource, parentId: "m"), + ], + Edges: []); + var target = new CanonicalGraph( + Nodes: + [ + Node("m", GraphNodeKind.Module, parentId: null), + Node("res", GraphNodeKind.Resource, parentId: null), + ], + Edges: [], + ErrorCount: 0); + + VisualGraphDiffer.HasTopologyChange(current, target).Should().BeTrue(); + } + + [TestMethod] + public void HasTopologyChange_WithChangedEdge_ReturnsTrue() + { + var current = new RenderedGraph( + Nodes: + [ + Rendered("a", GraphNodeKind.Resource, parentId: null), + Rendered("b", GraphNodeKind.Resource, parentId: null), + ], + Edges: []); + var target = new CanonicalGraph( + Nodes: + [ + Node("a", GraphNodeKind.Resource, parentId: null), + Node("b", GraphNodeKind.Resource, parentId: null), + ], + Edges: [Edge("a->b", "a", "b")], + ErrorCount: 0); + + VisualGraphDiffer.HasTopologyChange(current, target).Should().BeTrue(); + } + private static GraphNode Node( string id, string kind, diff --git a/src/Bicep.LangServer/Bicep.LangServer.csproj b/src/Bicep.LangServer/Bicep.LangServer.csproj index af3c850bedc..5ef46a0a703 100644 --- a/src/Bicep.LangServer/Bicep.LangServer.csproj +++ b/src/Bicep.LangServer/Bicep.LangServer.csproj @@ -18,6 +18,7 @@ + diff --git a/src/Bicep.LangServer/Features/Custom/Visualization/IVisualGraphLayoutEngine.cs b/src/Bicep.LangServer/Features/Custom/Visualization/IVisualGraphLayoutEngine.cs new file mode 100644 index 00000000000..d147ddc479a --- /dev/null +++ b/src/Bicep.LangServer/Features/Custom/Visualization/IVisualGraphLayoutEngine.cs @@ -0,0 +1,100 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Bicep.LanguageServer.Features.Custom.Visualization.Models; + +namespace Bicep.LanguageServer.Features.Custom.Visualization +{ + /// + /// The client-measured size of a node, used purely as layout input. The server's canonical graph does + /// not carry sizes (the webview owns node measurement), so the engine receives them separately and falls + /// back to a deterministic default for any node the client has not measured yet (for example on initial + /// load, where the client submits no graph at all). + /// + public record NodeSize(double Width, double Height); + + /// + /// Padding reserved inside a module box around its laid-out children. + /// + public sealed record ModulePadding + { + public ModulePadding(double top, double right, double bottom, double left) + { + this.Top = top; + this.Right = right; + this.Bottom = bottom; + this.Left = left; + } + + public double Top { get; init; } + + public double Right { get; init; } + + public double Bottom { get; init; } + + public double Left { get; init; } + } + + /// + /// Shared layout defaults and optional renderer-provided overrides. These defaults travel with the layout + /// engine API rather than a particular client, so future callers such as the CLI can use the engine without + /// duplicating webview-specific constants. + /// + public sealed record VisualGraphLayoutOptions + { + public static VisualGraphLayoutOptions Default { get; } = new( + defaultNodeSize: new NodeSize(200, 80), + nodeSeparation: 60, + layerSeparation: 50, + modulePadding: new ModulePadding(top: 50, right: 40, bottom: 40, left: 40)); + + public VisualGraphLayoutOptions( + NodeSize defaultNodeSize, + double nodeSeparation, + double layerSeparation, + ModulePadding modulePadding) + { + this.DefaultNodeSize = defaultNodeSize; + this.NodeSeparation = nodeSeparation; + this.LayerSeparation = layerSeparation; + this.ModulePadding = modulePadding; + } + + public NodeSize DefaultNodeSize { get; init; } + + public double NodeSeparation { get; init; } + + public double LayerSeparation { get; init; } + + public ModulePadding ModulePadding { get; init; } + } + + /// + /// Maps the canonical onto a layout engine's geometry model and returns a + /// position for every node, in graph coordinates. The engine computes positions only; the webview owns + /// all rendering, pan/zoom, and fit-view, so no edge routes or viewport-relative offsets are produced. + /// + public interface IVisualGraphLayoutEngine + { + /// + /// Computes a position for every node in . + /// + /// The canonical graph to lay out. + /// + /// Client-measured sizes keyed by node id. Missing or non-positive sizes fall back to a deterministic + /// default; container nodes (modules with children) are sized by the engine from their contents. + /// + /// Shared defaults and renderer-provided layout overrides. + /// Cancels the layout computation. + /// + /// A position per node id, in graph coordinates with a top-left origin and y increasing downward. An + /// empty result means no layout was produced (the graph was empty or a recoverable failure occurred); + /// callers should keep any previously known positions in that case. + /// + IReadOnlyDictionary Layout( + CanonicalGraph graph, + IReadOnlyDictionary nodeSizes, + VisualGraphLayoutOptions options, + CancellationToken cancellationToken); + } +} diff --git a/src/Bicep.LangServer/Features/Custom/Visualization/MsaglVisualGraphLayoutEngine.cs b/src/Bicep.LangServer/Features/Custom/Visualization/MsaglVisualGraphLayoutEngine.cs new file mode 100644 index 00000000000..983b954b065 --- /dev/null +++ b/src/Bicep.LangServer/Features/Custom/Visualization/MsaglVisualGraphLayoutEngine.cs @@ -0,0 +1,302 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Bicep.LanguageServer.Features.Custom.Visualization.Models; +using Microsoft.Extensions.Logging; +using Microsoft.Msagl.Core; +using Microsoft.Msagl.Core.Geometry.Curves; +using Microsoft.Msagl.Core.Layout; +using Microsoft.Msagl.Core.Routing; +using Microsoft.Msagl.Layout.Layered; +using Microsoft.Msagl.Miscellaneous; +using MsaglNode = Microsoft.Msagl.Core.Layout.Node; +using MsaglPoint = Microsoft.Msagl.Core.Geometry.Point; + +namespace Bicep.LanguageServer.Features.Custom.Visualization +{ + /// + /// Lays out the canonical graph using the core MSAGL engine (the Msagl package). It runs a layered + /// (Sugiyama) top-to-bottom layout to match the intent of the previous client-side ELK layout. + /// + /// Containment is handled by laying each scope out independently and composing the results: a module's + /// children are laid out on their own, the module is then sized to that result (plus padding for its + /// label), and the parent scope lays the module out as a single box before its children are lifted back + /// into place. This sidesteps MSAGL's compound-graph (cluster) layout, which is fragile here, and is exact + /// because the canonical graph only ever connects siblings (every dependency edge stays within one scope). + /// + /// + /// Only node positions are read back. Edges are added to each scope's geometry graph so they influence + /// layering, but MSAGL uses straight-line routing (the client draws its own straight edges, so spline + /// routes would be discarded). Positions are returned in graph coordinates with a top-left origin and y + /// increasing downward, normalized so the graph's top-left corner sits at the origin; pan/zoom and + /// fit-view stay on the client. MSAGL's own coordinate system is y-up, so the y axis is flipped on + /// read-out. Layout is best-effort: a recoverable MSAGL failure is logged and yields an empty result, so + /// the handler keeps prior positions and the view stays usable. Cancellation is observed and propagated. + /// + /// + public class MsaglVisualGraphLayoutEngine : IVisualGraphLayoutEngine + { + // Sentinel scope key for top-level nodes (those with no parent). Real node ids are never empty. + private const string RootScope = ""; + + private readonly ILogger logger; + + public MsaglVisualGraphLayoutEngine(ILogger logger) + { + this.logger = logger; + } + + public IReadOnlyDictionary Layout( + CanonicalGraph graph, + IReadOnlyDictionary nodeSizes, + VisualGraphLayoutOptions options, + CancellationToken cancellationToken) + { + if (graph.Nodes.Count == 0) + { + return EmptyLayout; + } + + cancellationToken.ThrowIfCancellationRequested(); + + try + { + var childrenByScope = GroupChildrenByScope(graph.Nodes); + var edgesByScope = GroupEdgesByScope(graph); + + using var cancelRegistration = LinkCancellation(cancellationToken, out var cancelToken); + + return this.LayoutScope(RootScope, childrenByScope, edgesByScope, nodeSizes, options, cancelToken).Positions; + } + catch (OperationCanceledException) + { + throw; + } + catch (Exception exception) + { + this.logger.LogWarning( + exception, + "MSAGL layout failed for a graph with {NodeCount} node(s) and {EdgeCount} edge(s); keeping previous positions.", + graph.Nodes.Count, + graph.Edges.Count); + + return EmptyLayout; + } + } + + /// + /// Lays out a single scope (a set of sibling nodes), recursively laying out any module among them + /// first. Returns absolute positions for the scope's whole subtree in coordinates local to the scope + /// (origin at its top-left), plus the size of the laid-out content. + /// + private (IReadOnlyDictionary Positions, double Width, double Height) LayoutScope( + string scope, + IReadOnlyDictionary> childrenByScope, + IReadOnlyDictionary> edgesByScope, + IReadOnlyDictionary nodeSizes, + VisualGraphLayoutOptions options, + CancelToken cancelToken) + { + var children = childrenByScope.TryGetValue(scope, out var scopedChildren) ? scopedChildren : []; + var sizeByChild = new Dictionary(children.Count, StringComparer.Ordinal); + var subtreeByModule = new Dictionary>(StringComparer.Ordinal); + + // Lay out module contents first so each module can be sized as a single box in this scope's layout. + foreach (var child in children) + { + if (child.HasChildren) + { + var subtree = this.LayoutScope(child.Id, childrenByScope, edgesByScope, nodeSizes, options, cancelToken); + + subtreeByModule[child.Id] = subtree.Positions; + sizeByChild[child.Id] = new NodeSize( + subtree.Width + options.ModulePadding.Left + options.ModulePadding.Right, + subtree.Height + options.ModulePadding.Top + options.ModulePadding.Bottom); + } + else + { + sizeByChild[child.Id] = ResolveSize(child.Id, nodeSizes, options); + } + } + + var edges = edgesByScope.TryGetValue(scope, out var scopedEdges) ? scopedEdges : []; + var (localPositions, width, height) = FlatLayout(children, sizeByChild, edges, options, cancelToken); + + // Lift each module's subtree into place relative to the module's slot in this scope. + var positions = new Dictionary(StringComparer.Ordinal); + + foreach (var child in children) + { + var childPosition = localPositions[child.Id]; + positions[child.Id] = childPosition; + + if (subtreeByModule.TryGetValue(child.Id, out var subtree)) + { + var offsetX = childPosition.X + options.ModulePadding.Left; + var offsetY = childPosition.Y + options.ModulePadding.Top; + + foreach (var (innerId, innerPosition) in subtree) + { + positions[innerId] = new NodeLayout(innerPosition.X + offsetX, innerPosition.Y + offsetY); + } + } + } + + return (positions, width, height); + } + + /// + /// Runs the layered MSAGL layout over a flat set of sized sibling nodes and their edges, returning each + /// node's top-left position (normalized so the content's top-left is the origin) and the content size. + /// + private static (IReadOnlyDictionary Positions, double Width, double Height) FlatLayout( + IReadOnlyList nodes, + IReadOnlyDictionary sizeByNode, + IReadOnlyList edges, + VisualGraphLayoutOptions options, + CancelToken cancelToken) + { + cancelToken.ThrowIfCanceled(); + + // A single isolated node needs no layout pass; place it at the origin to avoid layout edge cases. + if (nodes.Count == 1 && edges.Count == 0) + { + var only = nodes[0]; + var onlySize = sizeByNode[only.Id]; + + return (Single(only.Id, new NodeLayout(0, 0)), onlySize.Width, onlySize.Height); + } + + var geometryGraph = new GeometryGraph(); + var elementsById = new Dictionary(nodes.Count, StringComparer.Ordinal); + + foreach (var node in nodes) + { + var size = sizeByNode[node.Id]; + var element = new MsaglNode(CurveFactory.CreateRectangle(size.Width, size.Height, new MsaglPoint()), node.Id); + + elementsById[node.Id] = element; + geometryGraph.Nodes.Add(element); + } + + foreach (var edge in edges) + { + if (elementsById.TryGetValue(edge.SourceId, out var source) && + elementsById.TryGetValue(edge.TargetId, out var target)) + { + geometryGraph.Edges.Add(new Edge(source, target)); + } + } + + var settings = new SugiyamaLayoutSettings + { + LayerSeparation = options.LayerSeparation, + NodeSeparation = options.NodeSeparation, + EdgeRoutingSettings = { EdgeRoutingMode = EdgeRoutingMode.StraightLine }, + }; + + LayoutHelpers.CalculateLayout(geometryGraph, settings, cancelToken); + + return ReadFlatLayout(nodes, elementsById); + } + + private static (IReadOnlyDictionary Positions, double Width, double Height) ReadFlatLayout( + IReadOnlyList nodes, + Dictionary elementsById) + { + // Normalize so the content's top-left corner is the origin. MSAGL is y-up (a rectangle's Top is its + // larger y), so the topmost node maps to y = 0 and y grows downward, as the client expects. + var minLeft = double.PositiveInfinity; + var minBottom = double.PositiveInfinity; + var maxRight = double.NegativeInfinity; + var maxTop = double.NegativeInfinity; + + foreach (var element in elementsById.Values) + { + var box = element.BoundingBox; + minLeft = Math.Min(minLeft, box.Left); + minBottom = Math.Min(minBottom, box.Bottom); + maxRight = Math.Max(maxRight, box.Right); + maxTop = Math.Max(maxTop, box.Top); + } + + var positions = new Dictionary(nodes.Count, StringComparer.Ordinal); + + foreach (var node in nodes) + { + var box = elementsById[node.Id].BoundingBox; + positions[node.Id] = new NodeLayout(box.Left - minLeft, maxTop - box.Top); + } + + return (positions, maxRight - minLeft, maxTop - minBottom); + } + + private static IReadOnlyDictionary> GroupChildrenByScope(IReadOnlyList nodes) + { + var childrenByScope = new Dictionary>(StringComparer.Ordinal); + + foreach (var node in nodes) + { + var scope = node.ParentId ?? RootScope; + + if (!childrenByScope.TryGetValue(scope, out var children)) + { + children = []; + childrenByScope[scope] = children; + } + + children.Add(node); + } + + return childrenByScope.ToDictionary(pair => pair.Key, pair => (IReadOnlyList)pair.Value, StringComparer.Ordinal); + } + + private static IReadOnlyDictionary> GroupEdgesByScope(CanonicalGraph graph) + { + // Every edge connects two siblings, so the scope of an edge is the parent of its endpoints. + var scopeByNodeId = graph.Nodes.ToDictionary(node => node.Id, node => node.ParentId ?? RootScope, StringComparer.Ordinal); + var edgesByScope = new Dictionary>(StringComparer.Ordinal); + + foreach (var edge in graph.Edges) + { + if (!scopeByNodeId.TryGetValue(edge.SourceId, out var scope)) + { + continue; + } + + if (!edgesByScope.TryGetValue(scope, out var edges)) + { + edges = []; + edgesByScope[scope] = edges; + } + + edges.Add(edge); + } + + return edgesByScope.ToDictionary(pair => pair.Key, pair => (IReadOnlyList)pair.Value, StringComparer.Ordinal); + } + + private static NodeSize ResolveSize( + string id, + IReadOnlyDictionary nodeSizes, + VisualGraphLayoutOptions options) => + nodeSizes.TryGetValue(id, out var size) && size.Width > 0 && size.Height > 0 + ? size + : options.DefaultNodeSize; + + private static IReadOnlyDictionary Single(string id, NodeLayout position) => + new Dictionary(StringComparer.Ordinal) { [id] = position }; + + private static CancellationTokenRegistration LinkCancellation(CancellationToken cancellationToken, out CancelToken cancelToken) + { + // Bridge the framework token onto MSAGL's own cancellation flag, which its layout loops poll. + var msaglToken = new CancelToken(); + cancelToken = msaglToken; + + return cancellationToken.Register(() => msaglToken.Canceled = true); + } + + private static readonly IReadOnlyDictionary EmptyLayout = + new Dictionary(StringComparer.Ordinal); + } +} diff --git a/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphDiffer.cs b/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphDiffer.cs index 74385d4b41c..d4b0d704f07 100644 --- a/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphDiffer.cs +++ b/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphDiffer.cs @@ -17,13 +17,17 @@ namespace Bicep.LanguageServer.Features.Custom.Visualization /// client and never triggers a re-layout. /// /// - /// No layout () patches are produced here: positioning is still the - /// client's responsibility until the server-side layout engine is introduced. + /// Layout () patches are emitted only when a non-empty + /// layout is supplied. The handler computes layout only on topology changes (see + /// ), so metadata-only edits carry no position patches and never reflow. /// /// public static class VisualGraphDiffer { - public static IReadOnlyList Diff(RenderedGraph? current, CanonicalGraph target) + public static IReadOnlyList Diff( + RenderedGraph? current, + CanonicalGraph target, + IReadOnlyDictionary? layout = null) { var currentNodes = (current?.Nodes ?? []).ToDictionary(node => node.Id); var currentEdgeIds = (current?.Edges ?? []).Select(edge => edge.Id).ToHashSet(); @@ -41,11 +45,6 @@ public static IReadOnlyList Diff(RenderedGraph? current, CanonicalGr } } - // A node "survives" only if its id, kind, and parent are all unchanged. A change to kind or parent - // is structural (re-parenting / kind flip), so the node is removed and re-added rather than updated. - static bool Survives(RenderedGraphNode renderedNode, GraphNode targetNode) => - renderedNode.Kind == targetNode.Kind && renderedNode.ParentId == targetNode.ParentId; - // Remove nodes that are gone or structurally changed, deepest containment first so children are // removed before their parents. var removed = currentNodes.Values @@ -94,11 +93,59 @@ static bool Survives(RenderedGraphNode renderedNode, GraphNode targetNode) => } } + // Apply server-computed positions, when supplied. A global layout repositions every node, so the + // engine returns a position for each one; nodes absent from the layout keep their client position. + if (layout is { Count: > 0 }) + { + foreach (var targetNode in target.Nodes.OrderBy(node => node.Id, StringComparer.Ordinal)) + { + if (layout.TryGetValue(targetNode.Id, out var nodeLayout)) + { + patches.Add(new GraphPatch.SetNodeLayout(targetNode.Id, nodeLayout)); + } + } + } + patches.Add(new GraphPatch.SetErrorCount(target.ErrorCount)); return patches; } + /// + /// Returns whether the submitted graph and the freshly built target differ in topology — the set of + /// nodes (by id, kind, and parent) or the set of edges (by id). Metadata-only differences (error state, + /// source ranges, file paths) are not topology changes. The handler uses this to decide whether to run + /// layout, so metadata-only edits never trigger a reflow. + /// + public static bool HasTopologyChange(RenderedGraph? current, CanonicalGraph target) + { + var currentNodes = (current?.Nodes ?? []).ToDictionary(node => node.Id); + + // Counts equal plus every target node matching a surviving current node implies identical id sets. + if (currentNodes.Count != target.Nodes.Count) + { + return true; + } + + foreach (var targetNode in target.Nodes) + { + if (!currentNodes.TryGetValue(targetNode.Id, out var renderedNode) || !Survives(renderedNode, targetNode)) + { + return true; + } + } + + var currentEdgeIds = (current?.Edges ?? []).Select(edge => edge.Id).ToHashSet(); + var targetEdgeIds = target.Edges.Select(edge => edge.Id).ToHashSet(); + + return !currentEdgeIds.SetEquals(targetEdgeIds); + } + + // A node "survives" only if its id, kind, and parent are all unchanged. A change to kind or parent is + // structural (re-parenting / kind flip), so the node is removed and re-added rather than updated. + private static bool Survives(RenderedGraphNode renderedNode, GraphNode targetNode) => + renderedNode.Kind == targetNode.Kind && renderedNode.ParentId == targetNode.ParentId; + private static int ContainmentDepth(string nodeId) { var depth = 0; diff --git a/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphUpdateHandler.cs b/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphUpdateHandler.cs index 3ef02a95d80..269f83ed52d 100644 --- a/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphUpdateHandler.cs +++ b/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphUpdateHandler.cs @@ -13,7 +13,8 @@ namespace Bicep.LanguageServer.Features.Custom.Visualization { /// /// Handles textDocument/visualGraphUpdate: builds the canonical graph from the live compilation, - /// diffs it against the graph the client submitted, and returns the patch delta. No layout is computed yet. + /// diffs it against the graph topology the client submitted, and returns the topology/metadata patch delta. + /// Layout is computed later by after the client renders and measures nodes. /// public class VisualGraphUpdateHandler : IJsonRpcRequestHandler { @@ -21,7 +22,9 @@ public class VisualGraphUpdateHandler : IJsonRpcRequestHandler logger, ICompilationManager compilationManager) + public VisualGraphUpdateHandler( + ILogger logger, + ICompilationManager compilationManager) { this.logger = logger; this.compilationManager = compilationManager; @@ -45,4 +48,74 @@ public Task Handle(VisualGraphUpdateParams request, Can return Task.FromResult(new VisualGraphUpdateResult(patches)); } } + + /// + /// Handles textDocument/visualGraphLayout: validates the measured graph against the live compilation, + /// runs MSAGL with actual client-measured sizes, and returns layout patches. + /// + public class VisualGraphLayoutHandler : IJsonRpcRequestHandler + { + private readonly ILogger logger; + + private readonly ICompilationManager compilationManager; + + private readonly IVisualGraphLayoutEngine layoutEngine; + + public VisualGraphLayoutHandler( + ILogger logger, + ICompilationManager compilationManager, + IVisualGraphLayoutEngine layoutEngine) + { + this.logger = logger; + this.compilationManager = compilationManager; + this.layoutEngine = layoutEngine; + } + + public Task Handle(VisualGraphLayoutParams request, CancellationToken cancellationToken) + { + var context = this.compilationManager.GetCompilation(request.TextDocument.Uri); + + if (context is null) + { + this.logger.LogError("Visual graph layout request arrived before file {Uri} could be compiled.", request.TextDocument.Uri); + + return Task.FromResult(new VisualGraphLayoutResult(VisualGraphLayoutStatus.GraphChanged, [])); + } + + var target = VisualGraphBuilder.Build(context, request.TextDocument.Uri.ToIOUri()); + + if (VisualGraphDiffer.HasTopologyChange(request.Current, target)) + { + return Task.FromResult(new VisualGraphLayoutResult(VisualGraphLayoutStatus.GraphChanged, [])); + } + + var nodeSizes = BuildNodeSizes(request.Current); + var layout = this.layoutEngine.Layout(target, nodeSizes, request.Options ?? VisualGraphLayoutOptions.Default, cancellationToken); + + if (target.Nodes.Count > 0 && layout.Count == 0) + { + return Task.FromResult(new VisualGraphLayoutResult(VisualGraphLayoutStatus.LayoutFailed, [])); + } + + var patches = target.Nodes + .OrderBy(node => node.Id, StringComparer.Ordinal) + .Where(node => layout.ContainsKey(node.Id)) + .Select(node => new GraphPatch.SetNodeLayout(node.Id, layout[node.Id])) + .ToArray(); + + return Task.FromResult(new VisualGraphLayoutResult(VisualGraphLayoutStatus.Ok, patches)); + } + + private static IReadOnlyDictionary BuildNodeSizes(RenderedGraph current) + { + var sizes = new Dictionary(current.Nodes.Count, StringComparer.Ordinal); + + foreach (var node in current.Nodes) + { + sizes[node.Id] = new NodeSize(node.Width, node.Height); + } + + return sizes; + } + } } diff --git a/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphUpdateProtocol.cs b/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphUpdateProtocol.cs index 3c530c04d15..aa16bf5306d 100644 --- a/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphUpdateProtocol.cs +++ b/src/Bicep.LangServer/Features/Custom/Visualization/VisualGraphUpdateProtocol.cs @@ -10,8 +10,9 @@ namespace Bicep.LanguageServer.Features.Custom.Visualization { /// - /// Request sent by the webview (via the extension) asking the server to reconcile the graph it - /// currently displays with the server's latest graph. The server returns a complete patch delta. + /// Request sent by the webview (via the extension) asking the server to reconcile the topology/metadata + /// it currently displays with the server's latest graph. Layout is intentionally not computed here: when + /// topology changes, the client first renders/measures nodes and then sends . /// /// Handled by . The handler always answers (it is not gated by /// the feature flag); the flag only controls whether the extension routes the visual graph through this path, @@ -24,8 +25,34 @@ public record VisualGraphUpdateParams( RenderedGraph? Current) : ITextDocumentIdentifierParams, IRequest; /// - /// Response to a request: a complete, ordered delta transforming - /// the submitted graph into the server's latest graph. An empty list means nothing changed. + /// Response to a request: a complete, ordered topology/metadata + /// delta transforming the submitted graph into the server's latest graph. The client decides whether the + /// patches may affect rendered size and whether to send a follow-up request. /// public record VisualGraphUpdateResult(IReadOnlyList Patches); + + /// + /// Request sent after the webview has applied graph update patches and measured actual node sizes. The + /// server validates that the measured topology still matches the live compilation before running MSAGL. + /// + [Method("textDocument/visualGraphLayout", Direction.ClientToServer)] + public record VisualGraphLayoutParams( + TextDocumentIdentifier TextDocument, + RenderedGraph Current, + VisualGraphLayoutOptions? Options) : ITextDocumentIdentifierParams, IRequest; + + public static class VisualGraphLayoutStatus + { + public const string Ok = "ok"; + + public const string GraphChanged = "graphChanged"; + + public const string LayoutFailed = "layoutFailed"; + } + + /// + /// Response to a request. Successful responses contain only + /// patches. + /// + public record VisualGraphLayoutResult(string Status, IReadOnlyList Patches); } diff --git a/src/Bicep.LangServer/IServiceCollectionExtensions.cs b/src/Bicep.LangServer/IServiceCollectionExtensions.cs index f73cd521706..c4a29ab6348 100644 --- a/src/Bicep.LangServer/IServiceCollectionExtensions.cs +++ b/src/Bicep.LangServer/IServiceCollectionExtensions.cs @@ -20,6 +20,7 @@ using Bicep.LanguageServer.Completions; using Bicep.LanguageServer.Configuration; using Bicep.LanguageServer.Deploy; +using Bicep.LanguageServer.Features.Custom.Visualization; using Bicep.LanguageServer.Options; using Bicep.LanguageServer.Providers; using Bicep.LanguageServer.Registry; @@ -61,6 +62,7 @@ BicepLangServerOptions bicepLangServerOptions .AddSingleton() .AddSingleton() .AddSingleton() + .AddSingleton() .AddSingleton(bicepLangServerOptions) .AddSingleton(); } diff --git a/src/Bicep.LangServer/Server.cs b/src/Bicep.LangServer/Server.cs index 4bb26e13f74..75e98464e02 100644 --- a/src/Bicep.LangServer/Server.cs +++ b/src/Bicep.LangServer/Server.cs @@ -38,6 +38,7 @@ public Server(BicepLangServerOptions bicepLangServerOptions, Action() .WithHandler() .WithHandler() + .WithHandler() .WithHandler() .WithHandler() .WithHandler() diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index fd9932e5d0a..60b111f4c3e 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -79,6 +79,7 @@ + diff --git a/src/vscode-bicep-ui/apps/visual-designer/package.json b/src/vscode-bicep-ui/apps/visual-designer/package.json index d4626b9f8c0..8fc12e6e8db 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/package.json +++ b/src/vscode-bicep-ui/apps/visual-designer/package.json @@ -6,6 +6,8 @@ "scripts": { "dev": "vite", "build": "tsc -b && vite build", + "test": "vitest --run", + "test:watch": "vitest", "lint": "eslint . --report-unused-disable-directives --max-warnings 0", "lint:fix": "eslint . --fix", "preview": "vite preview", @@ -21,7 +23,8 @@ "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^5.1.4", "@vscode-elements/webview-playground": "^1.1.3", - "vite": "^7.3.2" + "vite": "^7.3.2", + "vitest": "^3.2.4" }, "dependencies": { "@react-hook/resize-observer": "^2.0.2", diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/App.tsx b/src/vscode-bicep-ui/apps/visual-designer/src/App.tsx index 9af36c77547..fe11c2175b7 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/App.tsx +++ b/src/vscode-bicep-ui/apps/visual-designer/src/App.tsx @@ -31,6 +31,7 @@ import { StatusBar } from "./features/status"; import { ModuleDeclaration, ResourceDeclaration } from "./features/visualization"; import { GlobalStyle } from "./GlobalStyle"; import { Canvas, Graph, nodeConfigAtom } from "./lib/graph"; +import { useFitViewToBounds } from "./lib/graph/hooks"; import { DEPLOYMENT_GRAPH_NOTIFICATION, DOCUMENT_DID_CHANGE_NOTIFICATION, @@ -88,8 +89,9 @@ function GraphContainer() { const { width, height } = getPanZoomDimensions(); return { x: width / 2, y: height / 2 }; }, [getPanZoomDimensions]); + const fitViewToBounds = useFitViewToBounds(); const applyGraph = useApplyDeploymentGraph(getViewportCenter); - const requestGraphUpdate = useGraphUpdate(getViewportCenter); + const { requestGraphUpdate, resetLayout } = useGraphUpdate(getViewportCenter, fitViewToBounds); const messageChannel = useWebviewMessageChannel(); const exportTheme = useAtomValue(effectiveExportThemeAtom); const setExportFileStem = useSetAtom(exportFileStemAtom); @@ -143,7 +145,7 @@ function GraphContainer() { return ( <> <$ControlBarContainer> - + diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/features/controls/ControlBar.tsx b/src/vscode-bicep-ui/apps/visual-designer/src/features/controls/ControlBar.tsx index 51517ea1109..96370f2a8e0 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/features/controls/ControlBar.tsx +++ b/src/vscode-bicep-ui/apps/visual-designer/src/features/controls/ControlBar.tsx @@ -72,10 +72,14 @@ const $Divider = styled.div` background-color: ${({ theme }) => theme.controlBar.border}; `; -export function ControlBar() { +interface ControlBarProps { + requestServerLayout?: () => Promise; +} + +export function ControlBar({ requestServerLayout }: ControlBarProps) { const { zoomIn, zoomOut } = usePanZoomControl(); const fitView = useFitView(); - const resetLayout = useResetLayout(); + const resetLayout = useResetLayout(requestServerLayout); const controls = useAtomValue(graphControlAvailabilityAtom); const openExportOverlay = useSetAtom(openExportOverlayAtom); diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/features/devtools/fake-graph-differ.ts b/src/vscode-bicep-ui/apps/visual-designer/src/features/devtools/fake-graph-differ.ts index 01880725c60..562bfdcde0e 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/features/devtools/fake-graph-differ.ts +++ b/src/vscode-bicep-ui/apps/visual-designer/src/features/devtools/fake-graph-differ.ts @@ -7,6 +7,7 @@ import type { GraphEdge, GraphNode, GraphPatch, + NodeLayout, RenderedGraph, } from "@/lib/messaging"; @@ -36,6 +37,49 @@ function getDepth(id: string): number { return id.split("::").length - 1; } +function buildFakeLayout(nodes: GraphNode[]): Map { + const nodesByParent = new Map(); + const layout = new Map(); + + for (const node of nodes) { + const siblings = nodesByParent.get(node.parentId) ?? []; + siblings.push(node); + nodesByParent.set(node.parentId, siblings); + } + + for (const siblings of nodesByParent.values()) { + siblings.sort((a, b) => a.id.localeCompare(b.id)); + } + + function layoutScope(parentId: string | null, offsetX: number, offsetY: number): { width: number; height: number } { + const siblings = nodesByParent.get(parentId) ?? []; + let maxWidth = 0; + let cursorY = 0; + + for (const node of siblings) { + layout.set(node.id, { x: offsetX, y: offsetY + cursorY }); + + let width = 220; + let height = 80; + + if (node.hasChildren) { + const childBounds = layoutScope(node.id, offsetX + 40, offsetY + cursorY + 50); + width = childBounds.width + 80; + height = childBounds.height + 90; + } + + maxWidth = Math.max(maxWidth, width); + cursorY += height + 80; + } + + return { width: maxWidth, height: Math.max(cursorY - 80, 0) }; + } + + layoutScope(null, 0, 0); + + return layout; +} + function toCanonicalNode(node: DeploymentGraphNode): GraphNode { return { id: node.id, @@ -53,8 +97,8 @@ function toCanonicalNode(node: DeploymentGraphNode): GraphNode { /** * Produce the patch list transforming `current` (the graph the webview displays) into `target` - * (the dev toolbar's graph). Mirrors the ordering the real server guarantees: remove edges, - * remove nodes deepest-first, add/update nodes shallowest-first, add edges, then error count. + * (the dev toolbar's graph). Mirrors the ordering the real server guarantees for graph updates: + * remove edges, remove nodes deepest-first, add/update nodes shallowest-first, add edges, then error count. */ export function diffGraph(current: RenderedGraph | null, target: DeploymentGraph | null): GraphPatch[] { const targetNodes = new Map(); @@ -133,3 +177,41 @@ export function diffGraph(current: RenderedGraph | null, target: DeploymentGraph return patches; } + +export function hasTopologyChange(current: RenderedGraph | null, target: DeploymentGraph | null): boolean { + if (!target) { + return (current?.nodes.length ?? 0) > 0; + } + + const targetNodes = new Map(target.nodes.map((node) => [node.id, toCanonicalNode(node)])); + const targetEdgeIds = new Set(target.edges.map((edge) => edgeId(edge.sourceId, edge.targetId))); + const currentNodes = current?.nodes ?? []; + const currentEdges = current?.edges ?? []; + + if (currentNodes.length !== targetNodes.size || currentEdges.length !== targetEdgeIds.size) { + return true; + } + + for (const node of currentNodes) { + const targetNode = targetNodes.get(node.id); + if (!targetNode || node.kind !== targetNode.kind || node.parentId !== targetNode.parentId) { + return true; + } + } + + return currentEdges.some((edge) => !targetEdgeIds.has(edge.id)); +} + +export function layoutGraph(current: RenderedGraph, target: DeploymentGraph | null): GraphPatch[] | undefined { + if (hasTopologyChange(current, target)) { + return undefined; + } + + if (!target) { + return []; + } + + const targetNodes = target.nodes.map(toCanonicalNode); + + return [...buildFakeLayout(targetNodes)].map(([nodeId, layout]) => ({ op: "setNodeLayout", nodeId, layout })); +} diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/features/devtools/fake-message-channel.ts b/src/vscode-bicep-ui/apps/visual-designer/src/features/devtools/fake-message-channel.ts index ffcf04df14e..4efc36ebd3f 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/features/devtools/fake-message-channel.ts +++ b/src/vscode-bicep-ui/apps/visual-designer/src/features/devtools/fake-message-channel.ts @@ -5,6 +5,8 @@ import type { WebviewNotificationCallback, WebviewNotificationMessage } from "@v import type { DeploymentGraph, DeploymentGraphPayload, + GetGraphLayoutRequest, + GetGraphLayoutResponse, GetGraphUpdateRequest, GetGraphUpdateResponse, } from "@/lib/messaging"; @@ -12,12 +14,13 @@ import type { import { DEPLOYMENT_GRAPH_NOTIFICATION, DOCUMENT_DID_CHANGE_NOTIFICATION, + GET_GRAPH_LAYOUT_REQUEST, GET_GRAPH_UPDATE_REQUEST, READY_NOTIFICATION, REVEAL_FILE_RANGE_NOTIFICATION, SHOW_PROBLEMS_PANEL_NOTIFICATION, } from "@/lib/messaging/messages"; -import { diffGraph } from "./fake-graph-differ"; +import { diffGraph, layoutGraph } from "./fake-graph-differ"; const FAKE_FILE_PATH = "file:///main.bicep"; @@ -816,6 +819,16 @@ export class FakeMessageChannel { return Promise.resolve({ patches } as GetGraphUpdateResponse as T); } + if (requestMessage.method === GET_GRAPH_LAYOUT_REQUEST) { + const { current } = requestMessage.params as GetGraphLayoutRequest; + const patches = layoutGraph(current, this.currentGraph); + const result: GetGraphLayoutResponse = patches + ? { status: "ok", patches } + : { status: "graphChanged", patches: [] }; + + return Promise.resolve(result as T); + } + return Promise.reject(new Error(`FakeMessageChannel does not support request: ${requestMessage.method}`)); } diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/features/layout/use-auto-layout.ts b/src/vscode-bicep-ui/apps/visual-designer/src/features/layout/use-auto-layout.ts index 3d29d184f59..59993bd70fa 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/features/layout/use-auto-layout.ts +++ b/src/vscode-bicep-ui/apps/visual-designer/src/features/layout/use-auto-layout.ts @@ -4,7 +4,7 @@ import { usePanZoomControl } from "@vscode-bicep-ui/components"; import { useAtomValue, useSetAtom } from "jotai"; import { useLayoutEffect } from "react"; -import { graphVersionAtom, layoutReadyAtom } from "@/lib/graph"; +import { graphVersionAtom, layoutReadyAtom, serverLayoutActiveAtom } from "@/lib/graph"; import { applyLayoutAtom, computeFitViewTransform } from "./elk-layout"; import { useComputeLayout } from "./use-compute-layout"; @@ -28,6 +28,7 @@ export function useAutoLayout() { const { transform } = usePanZoomControl(); const graphVersion = useAtomValue(graphVersionAtom); const isLayoutReady = useAtomValue(layoutReadyAtom); + const serverLayoutActive = useAtomValue(serverLayoutActiveAtom); // Run ELK layout after the DOM has been updated with the new graph. // useLayoutEffect fires synchronously after React commits DOM changes, @@ -37,7 +38,7 @@ export function useAutoLayout() { // previous layout is still in flight, the stale layout's completion // is ignored (via the `cancelled` flag set in the cleanup function). useLayoutEffect(() => { - if (graphVersion === 0) { + if (serverLayoutActive || graphVersion === 0) { return; } @@ -83,5 +84,5 @@ export function useAutoLayout() { return () => { cancelled = true; }; - }, [computeLayout, applyLayout, setLayoutReady, isLayoutReady, graphVersion, transform]); + }, [computeLayout, applyLayout, setLayoutReady, isLayoutReady, serverLayoutActive, graphVersion, transform]); } diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/features/layout/use-reset-layout.ts b/src/vscode-bicep-ui/apps/visual-designer/src/features/layout/use-reset-layout.ts index 5659222b363..e0e074c939e 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/features/layout/use-reset-layout.ts +++ b/src/vscode-bicep-ui/apps/visual-designer/src/features/layout/use-reset-layout.ts @@ -1,8 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { useSetAtom } from "jotai"; +import { useAtomValue, useSetAtom } from "jotai"; import { useCallback, useRef } from "react"; +import { serverLayoutActiveAtom } from "@/lib/graph"; import { applyLayoutAtom } from "./elk-layout"; import { useComputeLayout } from "./use-compute-layout"; @@ -12,19 +13,25 @@ import { useComputeLayout } from "./use-compute-layout"; * are deduplicated — if a layout is already in flight the callback * is a no-op. */ -export function useResetLayout() { +export function useResetLayout(requestServerLayout?: () => Promise) { const computeLayout = useComputeLayout(); const applyLayout = useSetAtom(applyLayoutAtom); + const serverLayoutActive = useAtomValue(serverLayoutActiveAtom); const layoutInFlight = useRef(false); return useCallback(async () => { if (layoutInFlight.current) return; layoutInFlight.current = true; try { + if (serverLayoutActive) { + await requestServerLayout?.(); + return; + } + const { layout } = await computeLayout(); await applyLayout({ result: layout }); } finally { layoutInFlight.current = false; } - }, [computeLayout, applyLayout]); + }, [computeLayout, applyLayout, requestServerLayout, serverLayoutActive]); } diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/atoms/graph.ts b/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/atoms/graph.ts index 739db7e9f35..5d46cc2930d 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/atoms/graph.ts +++ b/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/atoms/graph.ts @@ -14,6 +14,12 @@ import { nodesByIdAtom } from "./nodes"; */ export const graphVersionAtom = atom(0); +/** + * Whether the current visualizer session is using server-provided node positions. When true, + * the client-side ELK auto-layout is disabled and graph updates apply `setNodeLayout` patches directly. + */ +export const serverLayoutActiveAtom = atom(false); + /** * Whether the first ELK layout has completed and nodes are in * their final positions. The graph layer uses this to stay diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/components/AtomicNode.tsx b/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/components/AtomicNode.tsx index b6f04d80cbb..ebc2b738d53 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/components/AtomicNode.tsx +++ b/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/components/AtomicNode.tsx @@ -9,6 +9,7 @@ import { useWebviewMessageChannel } from "@vscode-bicep-ui/messaging"; import { useAtomValue, useStore } from "jotai"; import { frame } from "motion/react"; import { useEffect, useLayoutEffect, useRef } from "react"; +import { serverLayoutActiveAtom } from "@/lib/graph/atoms"; import { focusedNodeIdAtom, getNodeZIndex } from "@/lib/graph/atoms/nodes"; import { useBoxUpdate, useDragListener } from "@/lib/graph/hooks"; import { REVEAL_FILE_RANGE_NOTIFICATION } from "@/lib/messaging/messages"; @@ -56,10 +57,11 @@ export function AtomicNode({ id, boxAtom, dataAtom }: AtomicNodeState) { store.set(boxAtom, (box) => { // On first measurement the box is a zero-size point (min === max) - // placed at the spawn origin. Shift min so the node's center - // aligns with the origin instead of its top-left corner. + // placed at the spawn origin. Legacy ELK layout uses that origin as a center; + // server layout uses it as an already-computed top-left coordinate. + const serverLayoutActive = store.get(serverLayoutActiveAtom); const isInitial = box.min.x === box.max.x && box.min.y === box.max.y; - const min = isInitial ? { x: box.min.x - offsetWidth / 2, y: box.min.y - offsetHeight / 2 } : box.min; + const min = isInitial && !serverLayoutActive ? { x: box.min.x - offsetWidth / 2, y: box.min.y - offsetHeight / 2 } : box.min; return { min, @@ -75,11 +77,19 @@ export function AtomicNode({ id, boxAtom, dataAtom }: AtomicNodeState) { return; } + // Round to whole pixels so this matches the integer `offsetWidth`/`offsetHeight` + // used for the initial measurement above. `borderBoxSize` is device-pixel precise + // (e.g. 200.4), and letting that fractional value into the box would (a) visibly + // resize the enclosing module box by a fraction of a pixel and (b) feed a slightly + // different size into the server layout, making re-layout shift nodes by ~1px. + const width = Math.round(borderBoxSize.inlineSize); + const height = Math.round(borderBoxSize.blockSize); + store.set(boxAtom, (box) => ({ ...box, max: { - x: box.min.x + borderBoxSize.inlineSize, - y: box.min.y + borderBoxSize.blockSize, + x: box.min.x + width, + y: box.min.y + height, }, })); }); diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/hooks/use-fit-view.ts b/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/hooks/use-fit-view.ts index 4f9d3f8186f..d7c2786af8c 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/hooks/use-fit-view.ts +++ b/src/vscode-bicep-ui/apps/visual-designer/src/lib/graph/hooks/use-fit-view.ts @@ -5,15 +5,51 @@ import { useGetPanZoomDimensions, usePanZoomControl } from "@vscode-bicep-ui/com import { useAtomCallback } from "jotai/utils"; import { useCallback } from "react"; import { graphBoundsAtom } from "@/lib/graph/atoms"; +import type { Box } from "@/lib/utils/math/geometry"; import { getBoxCenter, getBoxHeight, getBoxWidth } from "@/lib/utils/math/geometry"; +/** + * Returns a callback that applies a pan-zoom transform to center the + * given bounding box in the viewport. Unlike {@link useFitView}, the + * bounds are supplied explicitly, so callers can fit to a layout's + * final positions before the nodes have animated there. + */ +export function useFitViewToBounds() { + const getPanZoomDimensions = useGetPanZoomDimensions(); + const { transform } = usePanZoomControl(); + + return useCallback( + (bounds: Box) => { + const graphWidth = getBoxWidth(bounds); + const graphHeight = getBoxHeight(bounds); + const { x: graphCenterX, y: graphCenterY } = getBoxCenter(bounds); + + const { width: viewportWidth, height: viewportHeight } = getPanZoomDimensions(); + + // Calculate scale to fit content in viewport with some padding + const padding = 100; + const scaleX = (viewportWidth - padding * 2) / graphWidth; + const scaleY = (viewportHeight - padding * 2) / graphHeight; + const scale = Math.min(scaleX, scaleY, 1); // Don't zoom in beyond 1:1 + + // Calculate translation to center the content + const viewportCenterX = viewportWidth / 2; + const viewportCenterY = viewportHeight / 2; + const translateX = viewportCenterX - graphCenterX * scale; + const translateY = viewportCenterY - graphCenterY * scale; + + transform(translateX, translateY, scale); + }, + [getPanZoomDimensions, transform], + ); +} + /** * Returns a callback that reads the graph bounding box * and applies a pan-zoom transform to center the graph in the viewport. */ export function useFitView() { - const getPanZoomDimensions = useGetPanZoomDimensions(); - const { transform } = usePanZoomControl(); + const fitViewToBounds = useFitViewToBounds(); return useAtomCallback( useCallback( @@ -24,27 +60,9 @@ export function useFitView() { return; } - const graphWidth = getBoxWidth(bounds); - const graphHeight = getBoxHeight(bounds); - const { x: graphCenterX, y: graphCenterY } = getBoxCenter(bounds); - - const { width: viewportWidth, height: viewportHeight } = getPanZoomDimensions(); - - // Calculate scale to fit content in viewport with some padding - const padding = 100; - const scaleX = (viewportWidth - padding * 2) / graphWidth; - const scaleY = (viewportHeight - padding * 2) / graphHeight; - const scale = Math.min(scaleX, scaleY, 1); // Don't zoom in beyond 1:1 - - // Calculate translation to center the content - const viewportCenterX = viewportWidth / 2; - const viewportCenterY = viewportHeight / 2; - const translateX = viewportCenterX - graphCenterX * scale; - const translateY = viewportCenterY - graphCenterY * scale; - - transform(translateX, translateY, scale); + fitViewToBounds(bounds); }, - [getPanZoomDimensions, transform], + [fitViewToBounds], ), ); } diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/__tests__/layout-invalidation.test.ts b/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/__tests__/layout-invalidation.test.ts new file mode 100644 index 00000000000..802d74a9771 --- /dev/null +++ b/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/__tests__/layout-invalidation.test.ts @@ -0,0 +1,145 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { GraphNode, GraphPatch, Range, RenderedGraph, RenderedGraphNode } from "../messages"; + +import { describe, expect, it } from "vitest"; +import { patchMayAffectLayout, renderedGraphsEqual } from "../layout-invalidation"; + +const ZERO_RANGE: Range = { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } }; + +function makeNode(overrides: Partial = {}): GraphNode { + return { + id: "n", + kind: "resource", + parentId: null, + type: "Microsoft.Storage/storageAccounts", + symbolName: "n", + isCollection: false, + hasChildren: false, + hasError: false, + filePath: "file:///main.bicep", + range: ZERO_RANGE, + ...overrides, + }; +} + +function graphOf(...nodes: GraphNode[]) { + return { nodes: new Map(nodes.map((node) => [node.id, node])) }; +} + +/** Mirror the server's stateless `updateNode`: every metadata field is always re-sent. */ +function fullUpdate(node: GraphNode, changes: Partial = {}): GraphPatch { + const merged = { ...node, ...changes }; + return { + op: "updateNode", + nodeId: node.id, + changes: { + type: merged.type, + isCollection: merged.isCollection, + hasChildren: merged.hasChildren, + hasError: merged.hasError, + filePath: merged.filePath, + range: merged.range, + }, + }; +} + +describe("patchMayAffectLayout", () => { + const node = makeNode({ id: "a" }); + const graph = graphOf(node); + + it("treats structural patches as layout-affecting", () => { + const structural: GraphPatch[] = [ + { op: "clearGraph" }, + { op: "addNode", node: makeNode({ id: "b" }) }, + { op: "removeNode", nodeId: "a" }, + { op: "addEdge", edge: { id: "a>b", sourceId: "a", targetId: "b" } }, + { op: "removeEdge", edgeId: "a>b" }, + ]; + + for (const patch of structural) { + expect(patchMayAffectLayout(graph, patch)).toBe(true); + } + }); + + it("treats setNodeLayout and setErrorCount as non-affecting", () => { + expect(patchMayAffectLayout(graph, { op: "setNodeLayout", nodeId: "a", layout: { x: 1, y: 2 } })).toBe(false); + expect(patchMayAffectLayout(graph, { op: "setErrorCount", errorCount: 3 })).toBe(false); + }); + + it("does not reflow when an updateNode only shifts the range (the blank-line-edit case)", () => { + const movedRange: Range = { start: { line: 9, character: 0 }, end: { line: 9, character: 1 } }; + expect(patchMayAffectLayout(graph, fullUpdate(node, { range: movedRange }))).toBe(false); + }); + + it("does not reflow when an updateNode only toggles hasError", () => { + expect(patchMayAffectLayout(graph, fullUpdate(node, { hasError: true }))).toBe(false); + }); + + it("does not reflow when an updateNode only changes filePath", () => { + expect(patchMayAffectLayout(graph, fullUpdate(node, { filePath: "file:///other.bicep" }))).toBe(false); + }); + + it("reflows when a size-affecting field actually changes", () => { + expect(patchMayAffectLayout(graph, fullUpdate(node, { type: "Microsoft.Web/sites" }))).toBe(true); + expect(patchMayAffectLayout(graph, fullUpdate(node, { isCollection: true }))).toBe(true); + expect(patchMayAffectLayout(graph, fullUpdate(node, { hasChildren: true }))).toBe(true); + }); + + it("does not reflow for an updateNode targeting an unknown node", () => { + expect(patchMayAffectLayout(graph, fullUpdate(makeNode({ id: "missing" })))).toBe(false); + }); +}); + +describe("renderedGraphsEqual", () => { + function rnode(overrides: Partial = {}): RenderedGraphNode { + return { id: "a", kind: "resource", parentId: null, width: 220, height: 80, ...overrides }; + } + + const base: RenderedGraph = { + nodes: [rnode({ id: "a" }), rnode({ id: "b" })], + edges: [{ id: "a>b", sourceId: "a", targetId: "b" }], + }; + + it("returns false when the previous input is null", () => { + expect(renderedGraphsEqual(null, base)).toBe(false); + }); + + it("returns true for the same graph regardless of node and edge order", () => { + const reordered: RenderedGraph = { + nodes: [rnode({ id: "b" }), rnode({ id: "a" })], + edges: [{ id: "a>b", sourceId: "a", targetId: "b" }], + }; + expect(renderedGraphsEqual(base, reordered)).toBe(true); + }); + + it("returns false when a node count differs", () => { + const extra: RenderedGraph = { nodes: [...base.nodes, rnode({ id: "c" })], edges: base.edges }; + expect(renderedGraphsEqual(base, extra)).toBe(false); + }); + + it("returns false when a measured size differs (the sub-pixel case)", () => { + const widened: RenderedGraph = { + nodes: [rnode({ id: "a", width: 221 }), rnode({ id: "b" })], + edges: base.edges, + }; + expect(renderedGraphsEqual(base, widened)).toBe(false); + }); + + it("returns false when containment (parentId) differs", () => { + const reparented: RenderedGraph = { + nodes: [rnode({ id: "a", parentId: "b" }), rnode({ id: "b" })], + edges: base.edges, + }; + expect(renderedGraphsEqual(base, reparented)).toBe(false); + }); + + it("returns false when the edge set differs", () => { + const rewired: RenderedGraph = { + nodes: base.nodes, + edges: [{ id: "b>a", sourceId: "b", targetId: "a" }], + }; + expect(renderedGraphsEqual(base, rewired)).toBe(false); + }); +}); diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/layout-invalidation.ts b/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/layout-invalidation.ts new file mode 100644 index 00000000000..3b01d3ee769 --- /dev/null +++ b/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/layout-invalidation.ts @@ -0,0 +1,92 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { GraphNode, GraphPatch, RenderedGraph } from "./messages"; + +/** + * The node metadata fields that influence a node's rendered size, and therefore the layout. + * + * "What affects layout" is decided in three places that must stay consistent: + * + * 1. {@link patchMayAffectLayout} here — the cheap client pre-filter that decides whether an + * applied `updateNode` patch is worth a re-measure. + * 2. {@link renderedGraphsEqual} here — the authoritative check that compares the freshly measured + * graph (structure + measured sizes) against the last graph that produced a layout. + * 3. The language server's `VisualGraphDiffer.HasTopologyChange` — which validates a measured + * layout request against the live compilation (node id/kind/parent and the edge set). + * + * If you add a field that changes a node's rendered size, add it here and confirm (2) and (3) still + * capture it; otherwise range-only edits may wrongly reflow, or a real change may be missed. + */ +const LAYOUT_AFFECTING_NODE_FIELDS = ["type", "isCollection", "hasChildren"] as const; + +type LayoutRelevantNode = Pick; + +interface LayoutRelevantGraph { + nodes: ReadonlyMap; +} + +/** + * Whether applying `patch` to the client graph may change the layout. Used as a cheap pre-filter + * before the (more expensive) render + measure + {@link renderedGraphsEqual} confirmation. + * + * Must be called BEFORE the patch is applied: for `updateNode` it compares the incoming values + * against the node's CURRENT values. The server is stateless and re-sends ALL metadata on every + * `updateNode`, so a field being present in `changes` does not mean it changed — e.g. deleting a + * blank line shifts every node's `range`, so every node receives an `updateNode`. Only an actual + * change to a size-affecting field counts. (`hasError`, `filePath`, and `range` never affect layout.) + */ +export function patchMayAffectLayout(graph: LayoutRelevantGraph, patch: GraphPatch): boolean { + switch (patch.op) { + case "clearGraph": + case "addNode": + case "removeNode": + case "addEdge": + case "removeEdge": + return true; + case "updateNode": { + const node = graph.nodes.get(patch.nodeId); + if (!node) { + return false; + } + const { changes } = patch; + return LAYOUT_AFFECTING_NODE_FIELDS.some( + (field) => changes[field] !== undefined && changes[field] !== node[field], + ); + } + case "setNodeLayout": + case "setErrorCount": + return false; + } +} + +/** + * Whether two measured graphs are equivalent layout inputs: the same node set (id, kind, parent, + * and measured width/height) and the same edge set. This is the authoritative second tier after + * {@link patchMayAffectLayout}; a layout request is sent only when this returns false. Node and + * edge order is irrelevant (compared by id). + */ +export function renderedGraphsEqual(left: RenderedGraph | null, right: RenderedGraph): boolean { + if (!left || left.nodes.length !== right.nodes.length || left.edges.length !== right.edges.length) { + return false; + } + + const leftNodes = new Map(left.nodes.map((node) => [node.id, node])); + + for (const rightNode of right.nodes) { + const leftNode = leftNodes.get(rightNode.id); + if ( + !leftNode || + leftNode.kind !== rightNode.kind || + leftNode.parentId !== rightNode.parentId || + leftNode.width !== rightNode.width || + leftNode.height !== rightNode.height + ) { + return false; + } + } + + const leftEdges = new Set(left.edges.map((edge) => `${edge.id}|${edge.sourceId}|${edge.targetId}`)); + + return right.edges.every((edge) => leftEdges.has(`${edge.id}|${edge.sourceId}|${edge.targetId}`)); +} diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/messages.ts b/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/messages.ts index 5fba2f29f56..1a53e474612 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/messages.ts +++ b/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/messages.ts @@ -91,6 +91,17 @@ export interface GetGraphUpdateResponse { patches: GraphPatch[]; } +export const GET_GRAPH_LAYOUT_REQUEST = "getGraphLayout"; + +export interface GetGraphLayoutRequest { + current: RenderedGraph; +} + +export interface GetGraphLayoutResponse { + status: "ok" | "graphChanged" | "layoutFailed"; + patches: GraphPatch[]; +} + export type GraphNodeKind = "resource" | "module"; /** The graph as currently rendered by the webview, sent with each update request for the server to diff against. */ diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/use-deployment-graph.ts b/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/use-deployment-graph.ts index faf019668d2..5a8c2268f3d 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/use-deployment-graph.ts +++ b/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/use-deployment-graph.ts @@ -1,10 +1,13 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +import type { PrimitiveAtom } from "jotai"; +import type { Box } from "@/lib/utils/math"; import type { Point } from "@/lib/utils/math/geometry"; -import type { DeploymentGraph } from "./messages"; +import type { DeploymentGraph, NodeLayout } from "./messages"; import { getDefaultStore, useSetAtom } from "jotai"; +import { animate, transform, type AnimationPlaybackControlsWithThen } from "motion"; import { useCallback, useRef } from "react"; import { errorCountAtom, hasNodesAtom } from "@/features/status"; import { @@ -12,15 +15,154 @@ import { addCompoundNodeAtom, addEdgeAtom, edgesAtom, + graphBoundsAtom, graphVersionAtom, layoutReadyAtom, nodesByIdAtom, removeNodesAtom, + serverLayoutActiveAtom, } from "@/lib/graph"; import { isDeploymentGraphEqual } from "@/lib/utils/deployment-graph-equality"; +import { translateBox } from "@/lib/utils/math"; const store = getDefaultStore(); +/** Duration (in seconds) of the spring animation when nodes move to new positions. */ +const ANIMATION_DURATION_S = 0.6; + +/** + * Animations still settling from the most recent server layout. They are + * cancelled before a new layout is applied so overlapping springs don't + * fight over the same boxes. + */ +let activeLayoutAnimations: AnimationPlaybackControlsWithThen[] = []; + +function waitForAnimationFrame(): Promise { + return new Promise((resolve) => requestAnimationFrame(() => resolve())); +} + +/** + * Spring a node's boxAtom from its current position to a target position. + * Returns the animation control so it can be cancelled if a newer layout + * arrives before it settles. + */ +function springNodeTo(boxAtom: PrimitiveAtom, targetX: number, targetY: number) { + const box = store.get(boxAtom); + const fromX = box.min.x; + const fromY = box.min.y; + + const opts = { clamp: false }; + const xTransform = transform([0, 100], [fromX, targetX], opts); + const yTransform = transform([0, 100], [fromY, targetY], opts); + + return animate(0, 100, { + type: "spring", + duration: ANIMATION_DURATION_S, + onUpdate: (latest) => { + const x = xTransform(latest); + const y = yTransform(latest); + store.set(boxAtom, (b) => translateBox(b, x - b.min.x, y - b.min.y)); + }, + }); +} + +/** + * Measure the bounding box the graph will occupy once the server layout is + * applied, including compound (module) boxes. + * + * A module box is derived from its children's union plus padding, so the union + * of atomic leaf positions alone is a few pixels tighter than the real graph + * bounds. To get the true target bounds we briefly move the atomic boxes to + * their server positions, read the derived {@link graphBoundsAtom} (which + * recomputes compound boxes synchronously from their children), then restore + * the original positions so the follow-up spring animation still starts from + * where the nodes currently are. + * + * Reading the same atom the Fit View button uses guarantees Reset Layout and + * Fit View settle on an identical viewport instead of drifting by a few pixels. + */ +export function measureServerLayoutBounds(nodeLayouts: Map): Box | null { + if (nodeLayouts.size === 0) { + return null; + } + + const nodes = store.get(nodesByIdAtom); + const snapshot = new Map(); + + for (const [nodeId, layout] of nodeLayouts) { + const node = nodes[nodeId]; + + if (!node || node.kind !== "atomic") { + continue; + } + + const box = store.get(node.boxAtom); + snapshot.set(nodeId, box); + store.set(node.boxAtom, translateBox(box, layout.x - box.min.x, layout.y - box.min.y)); + } + + const bounds = store.get(graphBoundsAtom); + + // Restore the pre-measurement positions. These writes are synchronous and the + // DOM is only painted on the next animation frame, so no movement is visible. + for (const [nodeId, box] of snapshot) { + const node = nodes[nodeId]; + + if (node && node.kind === "atomic") { + store.set(node.boxAtom, box); + } + } + + return bounds; +} + +/** + * Apply a server-computed layout to the current graph: reveal the graph + * layer if it was hidden, then spring every atomic node from its current + * position to its server-assigned position. Compound boxes follow their + * children automatically. + * + * Mirrors the ELK auto-layout reveal/animate sequence so server-driven + * re-layouts and node additions/removals animate instead of snapping. + */ +export async function applyServerLayout(nodeLayouts: Map): Promise { + // When the graph layer is hidden (initial load or a major topology + // replacement), wait one painted frame so the fitted viewport and the + // freshly-spawned node positions are committed, then reveal. Incremental + // edits leave the graph visible so nodes animate in place. + if (!store.get(layoutReadyAtom)) { + await waitForAnimationFrame(); + store.set(layoutReadyAtom, true); + } + + if (nodeLayouts.size === 0) { + // No positions to apply (e.g. unchanged sizes or a failed layout). Leave + // any in-flight springs running so they settle on their targets; we only + // needed to make sure the graph was revealed. + return; + } + + // Cancel any in-flight springs from the previous layout so overlapping + // springs don't fight over the same boxes. New springs read each box's + // current (interpolated) position, so movement continues smoothly. + for (const animation of activeLayoutAnimations) { + animation.stop(); + } + activeLayoutAnimations = []; + + const nodes = store.get(nodesByIdAtom); + + for (const [nodeId, layout] of nodeLayouts) { + const node = nodes[nodeId]; + + if (!node || node.kind !== "atomic") { + continue; + } + + activeLayoutAnimations.push(springNodeTo(node.boxAtom, layout.x, layout.y)); + } +} + /** * Snapshot the current position (box.min) of every node so we can * restore positions for nodes that survive a graph update, giving @@ -44,6 +186,16 @@ function snapshotNodePositions(): Map { return positions; } +interface ApplyDeploymentGraphOptions { + /** + * When true, the graph is being applied in server-layout mode: the + * topology is updated in place but node positions and the visibility + * gate are driven separately via {@link applyServerLayout} (no ELK + * version bump). + */ + serverLayout?: boolean; +} + export function useApplyDeploymentGraph(getViewportCenter: () => Point) { const setEdgesAtom = useSetAtom(edgesAtom); const addAtomicNode = useSetAtom(addAtomicNodeAtom); @@ -55,7 +207,12 @@ export function useApplyDeploymentGraph(getViewportCenter: () => Point) { const previousGraphRef = useRef(null); return useCallback( - (graph: DeploymentGraph | null) => { + (graph: DeploymentGraph | null, options: ApplyDeploymentGraphOptions = {}) => { + const isServerLayoutActive = options.serverLayout === true; + const wasServerLayoutActive = store.get(serverLayoutActiveAtom); + + store.set(serverLayoutActiveAtom, isServerLayoutActive); + // Update status bar atoms store.set(errorCountAtom, graph?.errorCount ?? 0); store.set(hasNodesAtom, (graph?.nodes.length ?? 0) > 0); @@ -76,6 +233,13 @@ export function useApplyDeploymentGraph(getViewportCenter: () => Point) { })); } } + + // In server-layout mode, positions and visibility are applied + // separately via applyServerLayout, so nothing else to do here. + // When switching back to ELK, bump the version to re-lay out. + if (!isServerLayoutActive && wasServerLayoutActive) { + setGraphVersion((v) => v + 1); + } } previousGraphRef.current = graph; return; @@ -290,9 +454,17 @@ export function useApplyDeploymentGraph(getViewportCenter: () => Point) { } } - // Phase 6: Bump graph version so subscribers (e.g. useLayoutEffect - // in App) can trigger ELK layout after the DOM reflects the new graph. - setGraphVersion((v) => v + 1); + if (isServerLayoutActive) { + // Server-layout mode: the topology is now in place. Node positions + // and the visibility reveal are applied separately via + // applyServerLayout once the server returns the computed layout. + // The visibility gate set above (hidden on major replacement, kept + // visible on incremental edits) is preserved until then. + } else { + // Phase 6: Bump graph version so subscribers (e.g. useLayoutEffect + // in App) can trigger ELK layout after the DOM reflects the new graph. + setGraphVersion((v) => v + 1); + } }, [ setEdgesAtom, diff --git a/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/use-graph-update.ts b/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/use-graph-update.ts index 5e68779771c..9477905f1bf 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/use-graph-update.ts +++ b/src/vscode-bicep-ui/apps/visual-designer/src/lib/messaging/use-graph-update.ts @@ -1,14 +1,18 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +import type { Box } from "@/lib/utils/math"; import type { Point } from "@/lib/utils/math/geometry"; import type { DeploymentGraph, + GetGraphLayoutRequest, + GetGraphLayoutResponse, GetGraphUpdateRequest, GetGraphUpdateResponse, GraphEdge, GraphNode, GraphPatch, + NodeLayout, RenderedGraph, } from "./messages"; @@ -16,8 +20,9 @@ import { useWebviewMessageChannel } from "@vscode-bicep-ui/messaging"; import { getDefaultStore } from "jotai"; import { useCallback, useRef } from "react"; import { nodesByIdAtom } from "@/lib/graph"; -import { GET_GRAPH_UPDATE_REQUEST } from "./messages"; -import { useApplyDeploymentGraph } from "./use-deployment-graph"; +import { patchMayAffectLayout, renderedGraphsEqual } from "./layout-invalidation"; +import { GET_GRAPH_LAYOUT_REQUEST, GET_GRAPH_UPDATE_REQUEST } from "./messages"; +import { applyServerLayout, measureServerLayoutBounds, useApplyDeploymentGraph } from "./use-deployment-graph"; const store = getDefaultStore(); @@ -36,7 +41,7 @@ function createClientGraph(): ClientGraph { return { nodes: new Map(), edges: new Map(), errorCount: 0 }; } -function applyPatch(graph: ClientGraph, patch: GraphPatch): void { +function applyPatch(graph: ClientGraph, nodeLayouts: Map, patch: GraphPatch): void { switch (patch.op) { case "clearGraph": graph.nodes.clear(); @@ -70,8 +75,7 @@ function applyPatch(graph: ClientGraph, patch: GraphPatch): void { graph.edges.delete(patch.edgeId); return; case "setNodeLayout": - // Server-side layout is not produced yet (Phase 5). ELK still positions nodes on the - // client, so layout patches are intentionally ignored here. + nodeLayouts.set(patch.nodeId, patch.layout); return; case "setErrorCount": graph.errorCount = patch.errorCount; @@ -130,6 +134,22 @@ function buildRenderedGraph(graph: ClientGraph): RenderedGraph { }; } +function waitForAnimationFrame(): Promise { + return new Promise((resolve) => requestAnimationFrame(() => resolve())); +} + +function collectNodeLayouts(patches: GraphPatch[]): Map { + const nodeLayouts = new Map(); + + for (const patch of patches) { + if (patch.op === "setNodeLayout") { + nodeLayouts.set(patch.nodeId, patch.layout); + } + } + + return nodeLayouts; +} + /** * Drives the notify-then-request loop for server-driven graph updates. * @@ -139,14 +159,84 @@ function buildRenderedGraph(graph: ClientGraph): RenderedGraph { * single follow-up request. Because every response is a complete delta against the graph that was * submitted, applying only the latest response is always correct without version tokens. */ -export function useGraphUpdate(getViewportCenter: () => Point): () => Promise { +export interface GraphUpdateActions { + requestGraphUpdate: () => Promise; + /** + * Re-run the server layout for the current graph and apply it, bypassing the + * "sizes unchanged since last layout" short-circuit so it re-lays out (and + * animates) even after the user has only dragged nodes around. Backs the Reset + * Layout button. Shares the single in-flight slot with {@link requestGraphUpdate}, + * so it never races a concurrent document-change update. + */ + resetLayout: () => Promise; +} + +export function useGraphUpdate( + getViewportCenter: () => Point, + fitViewToBounds: (bounds: Box) => void, +): GraphUpdateActions { const applyGraph = useApplyDeploymentGraph(getViewportCenter); const messageChannel = useWebviewMessageChannel(); const clientGraphRef = useRef(createClientGraph()); + const lastLayoutInputRef = useRef(null); const inFlightRef = useRef(false); const dirtyRef = useRef(false); + const forceLayoutRef = useRef(false); + + const requestGraphLayout = useCallback( + async (force = false) => { + const graph = clientGraphRef.current; - return useCallback(async () => { + if (graph.nodes.size === 0) { + lastLayoutInputRef.current = null; + return; + } + + await waitForAnimationFrame(); + + const measuredGraph = buildRenderedGraph(graph); + + if (!force && renderedGraphsEqual(lastLayoutInputRef.current, measuredGraph)) { + // Sizes are unchanged since the last layout, so positions still hold. + // Just make sure the graph is revealed in case it was hidden. + await applyServerLayout(new Map()); + return; + } + + const layoutRequest: GetGraphLayoutRequest = { current: measuredGraph }; + const layoutResponse = await messageChannel.sendRequest({ + method: GET_GRAPH_LAYOUT_REQUEST, + params: layoutRequest, + }); + + if (layoutResponse.status === "graphChanged") { + dirtyRef.current = true; + return; + } + + if (layoutResponse.status === "layoutFailed") { + // No usable layout — reveal the graph as-is so it isn't stuck hidden. + await applyServerLayout(new Map()); + return; + } + + const nodeLayouts = collectNodeLayouts(layoutResponse.patches); + lastLayoutInputRef.current = measuredGraph; + + // Fit the viewport to the layout's final bounds before the nodes animate + // there (mirrors the ELK auto-layout reveal/animate sequence). The bounds + // include compound boxes so this matches the Fit View button exactly. + const bounds = measureServerLayoutBounds(nodeLayouts); + if (bounds) { + fitViewToBounds(bounds); + } + + await applyServerLayout(nodeLayouts); + }, + [fitViewToBounds, messageChannel], + ); + + const requestGraphUpdate = useCallback(async () => { if (inFlightRef.current) { // A request is already outstanding; mark dirty so it issues one more round when it returns. dirtyRef.current = true; @@ -157,6 +247,15 @@ export function useGraphUpdate(getViewportCenter: () => Point): () => Promise Point): () => Promise(); + let layoutMayBeStale = false; + for (const patch of response.patches) { - applyPatch(graph, patch); + layoutMayBeStale ||= patchMayAffectLayout(graph, patch); + applyPatch(graph, nodeLayouts, patch); + } + + const shouldMeasureLayout = layoutMayBeStale && graph.nodes.size > 0; + + if (graph.nodes.size === 0) { + lastLayoutInputRef.current = null; } - applyGraph(toDeploymentGraph(graph)); - } while (dirtyRef.current); + // Apply the new topology. Visibility is preserved for incremental + // edits (so nodes animate in place) and gated for major changes; + // positions arrive in the layout phase below. + applyGraph(toDeploymentGraph(graph), { serverLayout: true }); + + if (shouldMeasureLayout) { + await requestGraphLayout(); + } + } while (dirtyRef.current || forceLayoutRef.current); } finally { inFlightRef.current = false; } - }, [applyGraph, messageChannel]); + }, [applyGraph, requestGraphLayout, messageChannel]); + + const resetLayout = useCallback(async () => { + forceLayoutRef.current = true; + + if (inFlightRef.current) { + // A request is already outstanding; the in-flight loop drains forceLayoutRef before it + // releases the lock, so the reset runs there instead of racing it. + return; + } + + await requestGraphUpdate(); + }, [requestGraphUpdate]); + + return { requestGraphUpdate, resetLayout }; } diff --git a/src/vscode-bicep-ui/apps/visual-designer/tsconfig.app.json b/src/vscode-bicep-ui/apps/visual-designer/tsconfig.app.json index a0cfcf8735d..f10738f836a 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/tsconfig.app.json +++ b/src/vscode-bicep-ui/apps/visual-designer/tsconfig.app.json @@ -1,6 +1,7 @@ { "extends": ["../../tsconfig.base.json"], "include": ["src"], + "exclude": ["src/**/*.test.ts", "src/**/*.test.tsx", "src/**/__tests__/**"], "compilerOptions": { "composite": true, "noUncheckedIndexedAccess": true, diff --git a/src/vscode-bicep-ui/apps/visual-designer/visual-graph-protocol.md b/src/vscode-bicep-ui/apps/visual-designer/visual-graph-protocol.md new file mode 100644 index 00000000000..75342bb7672 --- /dev/null +++ b/src/vscode-bicep-ui/apps/visual-designer/visual-graph-protocol.md @@ -0,0 +1,267 @@ +# Visual Graph Protocol + +This document describes the server-driven visual graph protocol used by the Bicep visual designer when `bicep.visualizer.serverLayout.enabled` is on. + +The protocol is intentionally split into two phases: + +1. Reconcile graph topology and metadata. +2. Render and measure nodes on the client, then request layout using actual node sizes. + +This split exists because the language server cannot know the final rendered dimensions of React node cards before the webview renders them. + +## Participants + +- **Language server** builds the canonical graph from the live Bicep compilation, diffs topology and metadata, validates measured layout requests, and runs MSAGL. +- **VS Code extension host** forwards webview requests to language-server requests and forwards responses back. It does not compute topology or layout. +- **React webview** owns rendering, node measurement, pan/zoom, fit-view, and applying graph/layout patches. + +## Message Flow + +```mermaid +sequenceDiagram + participant LS as Language server + participant Ext as VS Code extension + participant UI as React webview + + Note over LS,Ext: compilation or diagnostics update + Ext-->>UI: documentDidChange + + alt request already in flight + UI->>UI: mark dirty + else idle + UI->>Ext: getGraphUpdate(current topology) + Ext->>LS: textDocument/visualGraphUpdate(current topology) + LS->>LS: rebuild canonical graph and diff topology/metadata + LS-->>Ext: topology/metadata patches + Ext-->>UI: topology/metadata patches + UI->>UI: apply patches and detect layout-affecting changes + + opt layout may be stale + UI->>UI: render graph and measure node boxes + UI->>UI: compare measured graph with last layout input + alt measured layout input changed + UI->>Ext: getGraphLayout(measured graph) + Ext->>LS: textDocument/visualGraphLayout(measured graph) + LS->>LS: rebuild canonical graph and validate measured topology + alt topology still matches + LS->>LS: run MSAGL with measured node sizes + LS-->>Ext: ok + setNodeLayout patches + Ext-->>UI: ok + setNodeLayout patches + UI->>UI: apply layout patches and fit view + else topology changed + LS-->>Ext: graphChanged + Ext-->>UI: graphChanged + UI->>UI: mark dirty and restart graph update + end + else measured layout input unchanged + UI->>UI: keep existing positions + end + end + + opt dirty + UI->>UI: request graph update again + end + end +``` + +## Graph Update + +The graph update request reconciles topology and metadata only. It does not run layout. + +```ts +interface GetGraphUpdateRequest { + current: RenderedGraph | null; +} + +interface GetGraphUpdateResponse { + patches: GraphPatch[]; +} +``` + +The extension forwards this as: + +```ts +interface VisualGraphUpdateParams { + textDocument: { uri: string }; + current: RenderedGraph | null; +} + +interface VisualGraphUpdateResult { + patches: GraphPatch[]; +} +``` + +### Update Sequence + +```mermaid +sequenceDiagram + participant UI as React webview + participant Ext as VS Code extension + participant LS as Language server + + UI->>Ext: getGraphUpdate(current) + Ext->>LS: textDocument/visualGraphUpdate(current) + LS->>LS: build canonical graph + LS->>LS: diff current topology/metadata against canonical graph + LS-->>Ext: GraphPatch[] + Ext-->>UI: GraphPatch[] + UI->>UI: apply patches to client graph mirror + UI->>UI: record whether patches may affect layout +``` + +## Client Layout Invalidation + +The client decides whether layout may be stale while applying patches. This avoids making the server guess whether metadata changes affect rendered dimensions. + +Layout-affecting patches: + +- `clearGraph` +- `addNode` +- `removeNode` +- `addEdge` +- `removeEdge` +- `updateNode` when `type`, `isCollection`, or `hasChildren` changes + +Non-layout-affecting patches: + +- `updateNode` when only `hasError`, `filePath`, or `range` changes +- `setErrorCount` +- `setNodeLayout` + +Notably, `hasError` does not trigger layout. + +Because the server is stateless, every `updateNode` is an idempotent refresh that carries *all* node metadata fields, not only the ones that changed. The client therefore compares each incoming field against the value it currently holds and treats the patch as layout-affecting only when a layout-relevant field (`type`, `isCollection`, or `hasChildren`) actually changed value. Checking for field *presence* alone would reflow on every edit — for example, deleting a blank line shifts every node's `range`, so every `updateNode` would carry a (new) `range` and, if presence were the test, falsely look layout-affecting. + +If a patch may affect layout, the client renders the updated graph, measures actual node boxes, builds a measured `RenderedGraph`, and compares it with the last measured graph that produced a layout. The client sends a layout request only when measured topology, sizes, or layout options changed. + +```mermaid +flowchart TD + A[Apply graph update patches] --> B{Any patch may affect layout?} + B -- No --> C[Keep current positions] + B -- Yes --> D[Render updated graph] + D --> E[Measure actual node sizes] + E --> F{Measured graph equals last layout input?} + F -- Yes --> C + F -- No --> G[Send measured layout request] +``` + +## Measured Layout + +The layout request is sent only after the graph has rendered and node dimensions have been measured. + +```ts +interface GetGraphLayoutRequest { + current: RenderedGraph; +} + +interface GetGraphLayoutResponse { + status: "ok" | "graphChanged" | "layoutFailed"; + patches: GraphPatch[]; +} +``` + +The extension forwards this as: + +```ts +interface VisualGraphLayoutParams { + textDocument: { uri: string }; + current: RenderedGraph; + options?: VisualGraphLayoutOptions; +} + +interface VisualGraphLayoutResult { + status: "ok" | "graphChanged" | "layoutFailed"; + patches: GraphPatch[]; +} +``` + +Successful layout responses contain only `setNodeLayout` patches. + +### Layout Sequence + +```mermaid +sequenceDiagram + participant UI as React webview + participant Ext as VS Code extension + participant LS as Language server + + UI->>Ext: getGraphLayout(measured graph) + Ext->>LS: textDocument/visualGraphLayout(measured graph) + LS->>LS: rebuild canonical graph from live compilation + LS->>LS: compare measured topology with canonical topology + + alt topology matches + LS->>LS: run MSAGL with measured node sizes + LS-->>Ext: ok + setNodeLayout patches + Ext-->>UI: ok + setNodeLayout patches + UI->>UI: apply positions and fit view + else topology changed + LS-->>Ext: graphChanged + Ext-->>UI: graphChanged + UI->>UI: mark dirty and request graph update + else layout failed recoverably + LS-->>Ext: layoutFailed + Ext-->>UI: layoutFailed + UI->>UI: keep existing positions + end +``` + +## Rendered Graph + +`RenderedGraph` carries topology plus measured node sizes. It intentionally does not send current positions back to the server. + +```ts +interface RenderedGraph { + nodes: RenderedGraphNode[]; + edges: RenderedGraphEdge[]; +} + +interface RenderedGraphNode { + id: string; + kind: "resource" | "module"; + parentId: string | null; + width: number; + height: number; +} + +interface RenderedGraphEdge { + id: string; + sourceId: string; + targetId: string; +} +``` + +## Patch Shape + +```ts +type GraphPatch = + | { op: "clearGraph" } + | { op: "addNode"; node: GraphNode } + | { op: "removeNode"; nodeId: string } + | { op: "updateNode"; nodeId: string; changes: GraphNodeChanges } + | { op: "addEdge"; edge: GraphEdge } + | { op: "removeEdge"; edgeId: string } + | { op: "setNodeLayout"; nodeId: string; layout: NodeLayout } + | { op: "setErrorCount"; errorCount: number }; +``` + +## Concurrency Rules + +Each visualizer keeps one in-flight visual graph request at a time. A visual graph request is either a graph update request or a layout request. + +```mermaid +stateDiagram-v2 + [*] --> Idle + Idle --> Updating: documentDidChange + Updating --> Measuring: patches may affect layout + Updating --> Idle: no layout needed + Measuring --> Layouting: measured graph changed + Measuring --> Idle: measured graph unchanged + Layouting --> Idle: ok or layoutFailed + Layouting --> Updating: graphChanged or dirty + Updating --> Updating: dirty after response +``` + +If `documentDidChange` arrives while a request is in flight, the client sets a dirty flag. When the current request finishes, the client sends a fresh graph update if dirty is set. + +The server remains stateless per request. It validates each measured layout request against the current live compilation instead of tracking graph revisions. diff --git a/src/vscode-bicep-ui/apps/visual-designer/vite.config.ts b/src/vscode-bicep-ui/apps/visual-designer/vite.config.ts index 73f554bf0d1..4c6dadba26f 100644 --- a/src/vscode-bicep-ui/apps/visual-designer/vite.config.ts +++ b/src/vscode-bicep-ui/apps/visual-designer/vite.config.ts @@ -6,7 +6,7 @@ import type { Plugin } from "vite"; import fs from "fs"; import path from "path"; import react from "@vitejs/plugin-react"; -import { defineConfig } from "vite"; +import { defineConfig } from "vitest/config"; /** * Inject a fake `acquireVsCodeApi` with sample graph data only @@ -93,4 +93,9 @@ export default defineConfig({ }, }, }, + test: { + // Unit tests live next to the source under `src`. Playwright e2e specs in `e2e/` + // are run by Playwright, not Vitest, so keep them out of test discovery. + include: ["src/**/*.test.{ts,tsx}"], + }, }); diff --git a/src/vscode-bicep-ui/package-lock.json b/src/vscode-bicep-ui/package-lock.json index 153cf4d1d03..703ca943b61 100644 --- a/src/vscode-bicep-ui/package-lock.json +++ b/src/vscode-bicep-ui/package-lock.json @@ -101,7 +101,8 @@ "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^5.1.4", "@vscode-elements/webview-playground": "^1.1.3", - "vite": "^7.3.2" + "vite": "^7.3.2", + "vitest": "^3.2.4" } }, "apps/visual-designer/node_modules/@types/node": { diff --git a/src/vscode-bicep/src/language/protocol.ts b/src/vscode-bicep/src/language/protocol.ts index 82a9c4cf8e8..96bb5316c11 100644 --- a/src/vscode-bicep/src/language/protocol.ts +++ b/src/vscode-bicep/src/language/protocol.ts @@ -84,6 +84,24 @@ export const visualGraphUpdateRequestType = new ProtocolRequestType< void >("textDocument/visualGraphUpdate"); +export interface VisualGraphLayoutParams { + textDocument: TextDocumentIdentifier; + current: VisualGraphRendered; +} + +export interface VisualGraphLayoutResult { + status: "ok" | "graphChanged" | "layoutFailed"; + patches: unknown[]; +} + +export const visualGraphLayoutRequestType = new ProtocolRequestType< + VisualGraphLayoutParams, + VisualGraphLayoutResult, + never, + void, + void +>("textDocument/visualGraphLayout"); + export interface GetDeploymentDataRequest { textDocument: TextDocumentIdentifier; } diff --git a/src/vscode-bicep/src/visualizer/view.ts b/src/vscode-bicep/src/visualizer/view.ts index 9c5a5a14910..5f6216009f2 100644 --- a/src/vscode-bicep/src/visualizer/view.ts +++ b/src/vscode-bicep/src/visualizer/view.ts @@ -7,6 +7,8 @@ import vscode from "vscode"; import { LanguageClient } from "vscode-languageclient/node"; import { deploymentGraphRequestType, + visualGraphLayoutRequestType, + VisualGraphLayoutResult, VisualGraphRendered, visualGraphUpdateRequestType, VisualGraphUpdateResult, @@ -194,6 +196,41 @@ export class BicepVisualizerView extends Disposable { } } + private async handleGetGraphLayout(id: string, params: unknown): Promise { + const current = (params as { current?: VisualGraphRendered })?.current; + let result: VisualGraphLayoutResult = { status: "layoutFailed", patches: [] }; + + if (!current) { + await this.webviewPanel.webview.postMessage({ id, result }); + return; + } + + try { + const document = await vscode.workspace.openTextDocument(this.documentUri); + + if (this.isDisposed) { + return; + } + + result = await this.languageClient.sendRequest(visualGraphLayoutRequestType, { + textDocument: this.languageClient.code2ProtocolConverter.asTextDocumentIdentifier(document), + current, + }); + } catch (error) { + getLogger().error(`Visual graph layout request failed: ${parseError(error).message}`); + } + + if (this.isDisposed) { + return; + } + + try { + await this.webviewPanel.webview.postMessage({ id, result }); + } catch (error) { + getLogger().debug((error as Error).message ?? error); + } + } + private handleDidReceiveMessage(message: unknown): void { if (!message || typeof message !== "object") { return; @@ -230,6 +267,10 @@ export class BicepVisualizerView extends Disposable { case "getGraphUpdate": void this.handleGetGraphUpdate(request.id, request.params); return; + + case "getGraphLayout": + void this.handleGetGraphLayout(request.id, request.params); + return; } getLogger().warn(`Unhandled request method: ${request.method}`);