Keep workflow Flow canvas nodes stable across renders (#1573)

[Paul Lizer (paullizer)]

Summary

The Flow canvas, shared by the V2 workflow editor and the read-only Flow view, rebuilt every React Flow node on renders that don't change the diagram. In the editor that meant the saving state, a failed save and typing outside the diagram; in the read-only view, every runtime poll that returned the same run. React Flow drops the measured handle bounds of a node object it hasn't seen, so every control edge left the page until React Flow measured the nodes again. A measurement that landed between two rebuilds could leave the edges missing for good, which is the test_save_failures_retain_flow_fields_and_never_replace_the_original_cas_revision[400|409|503] flake.

The canvas inputs are now stable across renders. All changes are in WorkflowFlowCanvas.tsx plus a small pure helper, so WorkflowEditorDialog, WorkflowFlowAuthoring and WorkflowFlowView are untouched and both surfaces are covered:

• Stable handlers. Nodes call the latest onSelect, onCollapse, onFocus and onNavigate through stable wrappers backed by a ref that a layout effect updates after every commit. A new callback identity alone never rebuilds a node, and a click or key still reaches the current editor state.
• Node reuse. reuseUnchangedFlowNodes (lib/workflowFlowNodeReuse.ts) keeps the previous object for every node whose fields are unchanged, comparing plain-object fields such as data, style and position one level down. When no node changed it returns the previous array, so React Flow has nothing to adopt again.
• Measured size carried into real rebuilds. The canvas records the dimensions React Flow reports through onNodesChange and passes them back as measured on rebuilt nodes. React Flow then keeps the handle bounds, so real changes (typing in a task, a new compiler status, collapse) no longer remove edges either. measured is ignored when comparing nodes, and sizes of removed nodes are pruned.
• Selection re-measures its node. The selected border is 2px instead of 1px, which moves the handles without resizing the node. Each node calls useUpdateNodeInternals when its chosen flag flips; the old rebuild used to force that re-measure by accident.
• Stable <ReactFlow> props. onInit, onError and fitViewOptions no longer change identity on every render.

No other behavior changes. Selection, collapse, keyboard navigation, focus requests, dragged positions and change tracking use the same callbacks and state as before.

Root cause

• Editor. The canvas nodes memo depended on onSelect, onCollapse, onFocus and onNavigate. WorkflowEditorDialog passes onSelect as an inline arrow, and WorkflowFlowAuthoring passes collapse, a plain function, as onCollapse. The canvas's own onNavigate depends on onCollapse. Every editor render therefore produced a new object for every node.
• Read-only view. selectNode and toggleCollapse are already useCallback. However, statuses depends on the polled runtime object, so every poll built a new map with the same values, and the canvas rebuilt every node.
• Why edges vanish. In @xyflow/react 12.11.6, adoptUserNodes keeps the internal node only for the identical user object. For a new object without measured, parseHandles returns undefined handle bounds, and EdgeWrapper renders nothing without them. Edges come back after the ResizeObserver measures the node again.
• Why they can stay missing. useNodeObserver re-observes a node only when isInitialized changes. The flaky sequence:
  1. The second fill renders the editor synchronously and rebuilds every node. That drops the handle bounds, so React Flow re-observes each node.
  2. The compiler-preview effect's setLoaded(null) renders WorkflowFlowAuthoring again at default priority with a new collapse, so every node is rebuilt a second time. That render commits, and its passive setNodes is still pending.
  3. The ResizeObserver notification from step 1 lands and restores the handle bounds. Its store update renders synchronously, so React first flushes the pending passive setNodes, which drops the handle bounds again.
  4. isInitialized went false→false, so there is no re-observe. The edges stay missing, and expect_compiled times out at test_v2_workflow_flow_authoring.py:282.
• Audit of the other canvas props (WorkflowFlowAuthoring.tsx L286–291 and WorkflowFlowView.tsx L382–387): compiled ?? model.structure, statuses (memoized on the draft's targets and status text), bindings, positions and focusRequest change only on real changes, and noExecutions is a module constant. inspect feeds only the Configure button, never node data.

Validation

Runner for the Python and Playwright tests: the pinned venv (aa1bf1e0…\venv: pytest, pytest-playwright, Chromium) on this Windows machine, against the offline harness (PLAYWRIGHT_SERVICE_URL=''), one spec file at a time. "Base" is the V2 bundle built at c33ad4d16 (index-B2zJGAwc.js), pinned with SIMPLECHAT_UI_EXPECTED_JS/CSS. "Fix" is this branch before the V2 merge (index-iSQsBt1R.js). "Merged" is after merging V2 2f8dab475 (index-xkm43UeJ.js).

Build and logic (Node on PATH, venv python)

• npm run typecheck and npm run build (application/v2_ui): pass, both before and after the V2 merge.
• node --test functional_tests/test_v2_workflow_flow_node_reuse_logic.mjs: 10/10.
• functional_tests/test_v2_workflow_change_tracking.py, which bundles and runs test_v2_workflow_change_tracking_logic.ts: 4/4.
• python functional_tests/test_docs_app_surface_coverage.py: 7/7. python functional_tests/test_docs_site_quality.py: 6/6.
• Known base docs failures, unchanged: test_docs_link_integrity.py 2/5 (none of the broken links are in this PR's files) and test_docs_release_notes_integrity.py 0/1. The generated release-notes pages are stale for 143 releases; v0.261.205 adds one entry to that list. Regenerating them is out of scope here.

New regression spec ui_tests/test_v2_workflow_flow_canvas_stability.py

• Base: 5 failed, 1 passed. Each failure is the churn assertion:
  • 24 control edges removed and redrawn while a save was pending and failing (400, 409, 503)
  • 72 during focus and selection changes
  • 24 across two runtime polls in the read-only view
  • Each time the edge count dropped to 0. The rapid test passed on the base.
• Fix: 6/6. Merged: 6/6.

Flake reproduction (session-only stress plugin; 20 repeats per test; Chromium 4× CPU throttle through CDP Emulation.setCPUThrottlingRate)

Configuration	Base	Fix
Save-failure test [400/409/503], 4× throttle + race amplifier, 60 runs	60 failed, all at the control-edge wait (:282 via :843)	0 failed
Save-failure test [400/409/503], 4× throttle only, 60 runs	3 failed (400 runs 2, 12 and 17), all at :282 via :843	0 failed
Rapid selection test, 4× throttle + race amplifier, 20 runs	2 failed, both at :282 while opening the Flow	0 failed

Plain 4× throttling reproduces the original flake at the same line as the full-suite failures. The race amplifier makes that failure deterministic on the base.

Existing specs (venv pytest + Chromium)

Spec	Fix	Merged
test_v2_workflow_flow_authoring.py	30/30 and 30/30 (two runs)	30/30
test_v2_workflow_flow_inspection.py (read-only Flow view)	53/53	53/53
test_v2_workflow_authoring_history.py	28/28	not re-run
test_v2_workflow_change_tracking.py	24/24	not re-run
test_v2_workflow_editor.py	9/9	not re-run
test_v2_workflow_calendar_schedules.py	14/14	not re-run
test_v2_workflow_control_flow.py	10/10	not re-run
test_v2_workflow_m365_runtime.py	8/8	not re-run

The V2 merge brought in only #1575 (chat ThoughtsList.tsx and activityLanes.ts), so after it I re-ran the three Flow specs.

The renumber to v0.261.205 (78a2ba465) changes only version strings (9 lines in 5 files). After it I re-ran:

• the stability spec: 6/6
• the node test: 10/10
• the docs tests: 7/7 and 6/6
• the guardrails, below

Local guardrails (run_v2_guardrails.py, venv python), all on the same 7 files:

• base c33ad4d16 before the merge
• base 2f8dab475 after the merge
• base 2f8dab475 again on the final head 78a2ba465

Every run:

• broken-access-control: PASS
• xss-sinks: PASS
• swagger-routes: PASS (no routes changed)
• python-syntax: PASS (476 files compile)
• malicious-pr-review: PASS, no blockers. It flagged 8 "Important" lines in the new spec; they are keyword matches on Playwright get_by_role(...) locators and one Python pass.
• v2-ts-sinks: PASS

Decisions made autonomously

• Fixed in the canvas instead of in each caller (as in the approved plan). Instead of useCallback in WorkflowEditorDialog and WorkflowFlowAuthoring, the canvas calls the latest handlers through stable wrappers, so no caller can reintroduce the churn. This covers the editor and the read-only view, and it stays out of WorkflowEditorDialog.tsx, which Phase 4 edits.

• Node reuse was needed. Stable handlers don't cover the read-only view: each runtime poll builds an equal but new statuses map. They also don't cover focus and selection changes, which recomputed every node when only one or two changed. Reusing equal nodes in the canvas covers both without adding content-keyed memos to each caller.

• Rebuilt nodes carry measured. A real draft edit, such as the flaky test's second fill, changes every record and must rebuild every node. Carrying the last reported size keeps React Flow's handle bounds through those rebuilds, instead of relying on the re-measure landing in time.

  • This needs the selection re-measure (useUpdateNodeInternals), because the rebuild no longer forces one.
  • Residual risk: a node that has never been measured has no size to carry. The fix doc lists this under Known limitations, and none of the 140 fixed stress runs showed it.

• Selection and focus changes are covered too, although the kickoff put real diagram changes out of scope. Reuse and measured keep the arrows drawn through selection changes at no extra cost, so the focus/selection test asserts that no control edge is removed. A geometry check asserts that every control edge starts and ends on its node's handles.

• Typing trigger. "Typing outside the diagram" uses the decision field builder with page.keyboard. It re-renders the dialog without changing the draft, and the test asserts that typing doesn't revalidate.

• The rapid test fits the view first. Besides the node observers, the amplifier also delays React Flow's pane-size observer. Under it, the initial fit ran at full size with only the root block's button clickable, on the base and the fix alike. That is an amplifier artifact, not V2 Flow canvas rebuilds every node on unrelated re-renders, so edges drop and a Playwright test flakes #1573, so the test clicks Fit Flow before clicking nodes. The rapid test passes on the base when its setup succeeds; it guards the fix's reuse and re-measure rather than reproducing the bug.

• Stress tooling isn't committed. The session-only pytest plugin has three switches:

  • FLOW_REPEAT repeats a test.
  • FLOW_CPU_THROTTLE throttles through CDP.
  • FLOW_RO_RACE installs the race amplifier. It's an init script that holds each ResizeObserver notification until a microtask after the next React commit, or at most one more frame: a rare but legal delivery order.

  The fix doc describes it. I can commit it if you want it in the repo.

• Version 0.261.205. The version follows the order PRs go ready, and the coordinator numbered this one first, so it went from .207 to .205 in 78a2ba465.

  • V2 2f8dab475 (Remove redundant orchestration progress card from V2 chat #1575, v0.261.204) is merged in. I resolved the config.py and release_notes.md conflicts by hand with rerere disabled.
  • The v0.261.205 section sits above v0.261.204.

• Guardrail base. After merging V2, the guardrails used 2f8dab475 as the base so the three-dot diff stays on this PR's 7 files; before the merge they used c33ad4d16, per the kickoff command.

• Push. git push origin returned 403, so the branch is on the fork remote and this is a cross-repo PR.

Fixes #1573
Found during #1543