Uh oh!
There was an error while loading. Please reload this page.
fix(web): anchor the dashboard viewport across graph re-layout - #377
Merged
Jason Robert (jrob5756) merged 2 commits intoAug 7, 2026
Merged
Conversation
Expanding an inline subworkflow made the whole graph appear to jump. `layoutTopLevel` normalizes each rebuild's bounding box to origin, so growing one container shifts minX/minY and therefore every node, while the camera is never touched — the `fitView` prop is initial-render only. The world slid under a fixed viewport, which reads as "the view reset". Compensate the camera instead of changing the layout. After each rebuild, find a node present in both layouts, compute its canvas-absolute top-left in each (walking parentId chains, since inline expansion re-parents nodes and React Flow stores nested positions relative to the parent), and pan by the negated delta at unchanged zoom. The toggled container is preferred as the anchor so the chevron stays under the cursor; bulk toggles and live topology growth fall back to the node nearest the pane center. Verified in a browser: the toggled node moves 0.00px, and expand/collapse round-trips to the byte-identical viewport. `WorkflowGraph` gains a `ReactFlowProvider` wrapper because the rebuild effect lives outside `<ReactFlow>` and could not otherwise reach `setViewport`. The export surface is unchanged. Closes#375 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Jason Robert (jrob5756)force-pushed
the
feature/375-anchor-viewport-on-expand
branch
from
August 7, 2026 14:45
6c3d0cd to
b073d34CompareBlocking issues found by review: - An instant `setViewport` on rebuild took d3-zoom's non-transition path, which calls `interrupt()`, so a rebuild landing inside an animated `fitView` cancelled it mid-flight and left its promise permanently unsettled. Deep-link centering was the real casualty: it polls up to 40 frames to land on a node, and a running workflow's topology churn fires exactly the rebuilds that would undo it. `lib/camera-authority.ts` makes the ownership explicit — every animated fit claims the camera for its duration and the rebuild effect yields while a claim is live. - `NaN === 0` is false, so a non-finite delta slipped past the zero-delta check into d3, producing an invalid CSS transform (graph renders at identity) that then fed back through `getViewport()` into every later rebuild. Guarded explicitly, mirroring `graph-layout.ts`. Tests: the two `nextAnchorHint` "drops the hint" cases never passed a non-null hint, so removing the context-switch and multi-toggle drops outright survived the suite. A mutation sweep found 11 non-equivalent survivors in all — zoom divisor, Euclidean ranking, pane-height guard, next-only hint match, `anchoredViewport`'s own prev-layout resolution. All 11 are now killed, and a new fixture pins a node nested inside an already-expanded container, which is the only case that exercises the parent-chain fold and the Y axis against real layout output. Comments: the module claimed inline expansion *re-parents* nodes, but `for_each` pills do not exist while collapsed and arrive already parented — the test one line below asserted exactly that. The real invariant is that a nested node keeps a byte-identical relative `position` while its container moves. Also corrected: a docstring that contradicted `nextAnchorHint` on sticky reuse, a cycle-degradation note describing behaviour the code does not have, and a known-limitation paragraph that omitted that the residual accumulates. Also drops the unnecessary index signature on `AnchorNode.data` (which turns the integration fixtures into a standing compile-time check that React Flow's `Node` stays assignable), types the hint ref as `AnchorHintResult` so a future field cannot be silently dropped, merges `resolveAnchorFrom` into `resolveAnchorId`, and records the design in AGENTS.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Jason Robert (jrob5756)
marked this pull request as ready for review
August 7, 2026 17:38
Uh oh!
There was an error while loading. Please reload this page.
Jason Robert (jrob5756)
deleted the
feature/375-anchor-viewport-on-expand
branch
August 7, 2026 17:39
Jason Robert (jrob5756) pushed a commit
that referenced
this pull request
Aug 7, 2026
Resolves conflicts in two generated artifacts by rebuilding from the merged source rather than picking a side: - src/conductor/web/static/** (hashed bundle + index.html) - src/conductor/web/frontend/tsconfig.tsbuildinfo All hand-written source auto-merged. #377 added real frontend source (graph-anchor, camera-authority, use-deep-link) whose 47 tests pass alongside this branch's 79. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes#375
The bug
layoutTopLevelnormalizes each rebuild's bounding box to origin:When an expanded container grows,
minX/minYshift, so every node in thegraph gets a new position — including nodes nowhere near the one that was
toggled. Meanwhile the camera is never touched: the
fitViewprop isinitial-render only (I confirmed in the React Flow source that
fitViewQueuedis diff-guarded and cleared after the first successful fit), and
FitViewOnContextSwitchfires only onviewPathKeychange.So the entire world slides under a fixed camera. Visually that reads as "the
view reset", and after an expand/collapse round trip you're looking at the right
layout from the wrong place.
The fix
Leave the layout pass alone; compensate the camera instead. After each rebuild:
parentIdchains — React Flow stores a nested node'spositionrelative toits parent, and inline expansion re-parents nodes, so raw positions aren't
comparable across a rebuild.
context switch), where the existing
fitViewpaths still own the camera.Anchor preference:
chevron stays under the cursor. Both key namespaces sharing
expandedContextsare checked (
childContextKeyfor subworkflows/iterations,groupExpansionKeyforfor_eachgroups), on both sides of the rebuild —a group only carries
groupExpansionKeyonce it's expandable.Expand-all toggles many keys at once, so it takes the second branch. There's no
new exclusivity rule and no
fitViewon expansion — expand-all and N-at-oncekeep working exactly as before.
The same compensation steadies the graph when a running workflow's topology
grows (a
for_eachfanning out, a subworkflow's DAG arriving), which today alsoslides the graph under a fixed camera.
Structural note
WorkflowGraphgains aReactFlowProviderwrapper. There was previously noprovider anywhere in the app — every
useReactFlow()call site was a child of<ReactFlow>and relied on its implicit provider. The rebuild effect livesoutside
<ReactFlow>and so couldn't reachsetViewport. Provider and<ReactFlow>mount together (ResizableLayoutswaps the whole graph out forDialogOverlay), so the hoisted store has the same lifetime as before and theinitial
fitViewstill runs once per mount. The export surface is unchanged.Changes
web/frontend/src/lib/graph-anchor.ts— pure:toAbsolutePositions,resolveAnchorId,anchoredViewport,nextAnchorHint. No React, no@xyflow/reactruntime import, same shape aslib/reconnect.ts.web/frontend/src/lib/graph-anchor.test.ts— 35 tests.web/frontend/src/components/graph/WorkflowGraph.tsx— provider split plusthe anchoring effect. No change to
graph-layout.ts, the store, nodecomponents, or the event pipeline.
CHANGELOG.md, rebuiltweb/static/.Verification
tsc -bclean; 98/98 frontend tests pass (35 new).Browser-tested with Playwright against a live dashboard (1600x1000) running a
provider-free workflow with three subworkflow steps:
subAsubAsubA, thensubB, thensubCF, the Controls buttons (zoom in/out/fit), double-click drill-down andbreadcrumb-back all still work
Known limitation
Expanding three subworkflows individually and then clicking collapse-all
leaves ~19 px of horizontal pan, accumulating per repeat of that specific mixed
cycle. I traced it rather than guessing: three individual expands pin three
different containers, and no node in the collapsed layout carries the summed
delta needed to invert them, so it's structural rather than an anchor-selection
bug. Pinning one canonical node throughout would remove it, at the cost of the
property this exists for. Fit-view clears it, and it takes roughly 35 cycles
before content starts clipping. Documented in the module docstring.
Out of scope
nodesDraggableis on but therebuild replaces the node array wholesale, so a dragged node snaps back to its
dagre position. Separate fix (position memoization keyed by node id).
jumping.