fix: derive subgraph preview promotion for any interior node type - #14551
Closed
claude[bot] wants to merge 8 commits into
Closed
fix: derive subgraph preview promotion for any interior node type#14551claude[bot] wants to merge 8 commits into
claude[bot] wants to merge 8 commits into
Conversation
autoExposeKnownPreviewNodes only auto-exposes previews for interior node types hardcoded in CANVAS_IMAGE_PREVIEW_NODE_TYPES. Node types not on that list (e.g. SamplerCustomAdvanced) fall back to a one-shot requestAnimationFrame check that runs before the node has produced any output and is never retried, so a real preview arriving later over the websocket never reaches the outer SubgraphNode. Adds a unit test exercising autoExposeKnownPreviewNodes directly and an e2e test simulating a mid-execution websocket preview for a SamplerCustomAdvanced node nested in a subgraph. Both currently fail.
🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ✅ 1783 passed, 0 failed · 2 flaky📊 Browser Reports
📦 Bundle: 8.24 MB gzip 🔴 +545 BDetailsSummary
Category Glance App Entry Points — 3.64 kB (baseline 3.64 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.34 MB (baseline 1.34 MB) • 🔴 +1.51 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed / 1 unchanged Views & Navigation — 111 kB (baseline 111 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 12 added / 12 removed / 4 unchanged Panels & Settings — 551 kB (baseline 551 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 15 unchanged User & Accounts — 28.7 kB (baseline 28.7 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed / 3 unchanged Editors & Dialogs — 124 kB (baseline 124 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 6 added / 6 removed / 1 unchanged UI Components — 70 kB (baseline 70 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed / 9 unchanged Data & Services — 3.46 MB (baseline 3.46 MB) • 🔴 +1.07 kBStores, services, APIs, and repositories
Status: 14 added / 14 removed / 3 unchanged Utilities & Hooks — 386 kB (baseline 386 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 16 added / 16 removed / 20 unchanged Vendor & Third-Party — 15.7 MB (baseline 15.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 12.8 MB (baseline 12.8 MB) • ⚪ 0 BBundles that do not match a named category
Status: 70 added / 70 removed / 211 unchanged ⚡ Performance Report
✅ No regressions detected. All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-08-04T03:12:52.371Z",
"gitSha": "2bfa3f0a87ab25570efcf3a2a3bf3ea2d3c90f0c",
"branch": "fix/subgraph-preview-allowlist-gap",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2021.1309999999685,
"styleRecalcs": 9,
"styleRecalcDurationMs": 5.281999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 453.36199999999997,
"heapDeltaBytes": 5381736,
"heapUsedBytes": 69290936,
"domNodes": 18,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 14.692000000000002,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-idle",
"durationMs": 2014.0240000000063,
"styleRecalcs": 10,
"styleRecalcDurationMs": 6.179,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 386.20500000000004,
"heapDeltaBytes": 5592124,
"heapUsedBytes": 69697864,
"domNodes": 20,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 13.383000000000001,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1751.2869999999907,
"styleRecalcs": 72,
"styleRecalcDurationMs": 24.945,
"layouts": 12,
"layoutDurationMs": 2.8930000000000002,
"taskDurationMs": 667.5920000000001,
"heapDeltaBytes": -654360,
"heapUsedBytes": 63249000,
"domNodes": 54,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 88.27399999999999,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1779.9759999999765,
"styleRecalcs": 75,
"styleRecalcDurationMs": 26.991,
"layouts": 12,
"layoutDurationMs": 2.8600000000000003,
"taskDurationMs": 719.308,
"heapDeltaBytes": -18635724,
"heapUsedBytes": 45409116,
"domNodes": 24,
"jsHeapTotalBytes": 23576576,
"scriptDurationMs": 89.426,
"eventListeners": -148,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1684.0610000000424,
"styleRecalcs": 30,
"styleRecalcDurationMs": 11.988999999999997,
"layouts": 6,
"layoutDurationMs": 0.582,
"taskDurationMs": 299.70199999999994,
"heapDeltaBytes": 8213436,
"heapUsedBytes": 72241392,
"domNodes": 79,
"jsHeapTotalBytes": 24641536,
"scriptDurationMs": 14.371000000000002,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1710.3399999999738,
"styleRecalcs": 31,
"styleRecalcDurationMs": 12.972000000000001,
"layouts": 6,
"layoutDurationMs": 0.52,
"taskDurationMs": 317.791,
"heapDeltaBytes": 8673864,
"heapUsedBytes": 72738380,
"domNodes": 78,
"jsHeapTotalBytes": 24379392,
"scriptDurationMs": 15.888,
"eventListeners": 21,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 501.9790000000057,
"styleRecalcs": 14,
"styleRecalcDurationMs": 6.005,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 286.216,
"heapDeltaBytes": -11004992,
"heapUsedBytes": 52999808,
"domNodes": 24,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 38.528,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.669999999999998,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 480.5029999999988,
"styleRecalcs": 12,
"styleRecalcDurationMs": 5.396,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 283.335,
"heapDeltaBytes": -12436256,
"heapUsedBytes": 51402060,
"domNodes": 20,
"jsHeapTotalBytes": 27000832,
"scriptDurationMs": 39.111999999999995,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.669999999999998,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "large-graph-idle",
"durationMs": 2010.5049999999665,
"styleRecalcs": 11,
"styleRecalcDurationMs": 5.667,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 568.8910000000001,
"heapDeltaBytes": 7057412,
"heapUsedBytes": 68563200,
"domNodes": -278,
"jsHeapTotalBytes": 3256320,
"scriptDurationMs": 68.84899999999999,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2011.9290000000092,
"styleRecalcs": 9,
"styleRecalcDurationMs": 5.165999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 556.514,
"heapDeltaBytes": 3821260,
"heapUsedBytes": 64242956,
"domNodes": -280,
"jsHeapTotalBytes": 3780608,
"scriptDurationMs": 70,
"eventListeners": -179,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-pan",
"durationMs": 2104.2269999999803,
"styleRecalcs": 69,
"styleRecalcDurationMs": 10.499,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 975.4110000000001,
"heapDeltaBytes": 3616752,
"heapUsedBytes": 64258200,
"domNodes": -281,
"jsHeapTotalBytes": 3448832,
"scriptDurationMs": 303.57099999999997,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2088.97300000001,
"styleRecalcs": 67,
"styleRecalcDurationMs": 10.390999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1037.0879999999997,
"heapDeltaBytes": 3282628,
"heapUsedBytes": 64441020,
"domNodes": -282,
"jsHeapTotalBytes": 3710976,
"scriptDurationMs": 328.287,
"eventListeners": -147,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-zoom",
"durationMs": 3105.1109999999653,
"styleRecalcs": 65,
"styleRecalcDurationMs": 11.575999999999999,
"layouts": 60,
"layoutDurationMs": 6.292999999999999,
"taskDurationMs": 1149.8919999999998,
"heapDeltaBytes": -10667332,
"heapUsedBytes": 53905708,
"domNodes": 14,
"jsHeapTotalBytes": 6926336,
"scriptDurationMs": 361.28,
"eventListeners": -151,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3134.13300000002,
"styleRecalcs": 66,
"styleRecalcDurationMs": 14.510000000000002,
"layouts": 60,
"layoutDurationMs": 7.882,
"taskDurationMs": 1254.81,
"heapDeltaBytes": -8448692,
"heapUsedBytes": 55681404,
"domNodes": -287,
"jsHeapTotalBytes": 6926336,
"scriptDurationMs": 394.743,
"eventListeners": -181,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2027.6450000000068,
"styleRecalcs": 9,
"styleRecalcDurationMs": 5.813000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 553.3209999999999,
"heapDeltaBytes": 7386252,
"heapUsedBytes": 70307984,
"domNodes": -281,
"jsHeapTotalBytes": 3256320,
"scriptDurationMs": 67.593,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "minimap-idle",
"durationMs": 2007.7820000000202,
"styleRecalcs": 9,
"styleRecalcDurationMs": 5.3790000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 557.706,
"heapDeltaBytes": 7197684,
"heapUsedBytes": 67956172,
"domNodes": -287,
"jsHeapTotalBytes": 2994176,
"scriptDurationMs": 70.187,
"eventListeners": -149,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 520.0999999999567,
"styleRecalcs": 48,
"styleRecalcDurationMs": 7.542,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 303.485,
"heapDeltaBytes": -10800104,
"heapUsedBytes": 53344492,
"domNodes": 22,
"jsHeapTotalBytes": 26476544,
"scriptDurationMs": 84.553,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 505.8210000000827,
"styleRecalcs": 48,
"styleRecalcDurationMs": 7.9049999999999985,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 317.353,
"heapDeltaBytes": -10284556,
"heapUsedBytes": 53667476,
"domNodes": 22,
"jsHeapTotalBytes": 25427968,
"scriptDurationMs": 88.424,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999727
},
{
"name": "subgraph-idle",
"durationMs": 2021.3830000000144,
"styleRecalcs": 10,
"styleRecalcDurationMs": 5.767000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 401.95300000000003,
"heapDeltaBytes": 5214740,
"heapUsedBytes": 69104056,
"domNodes": 20,
"jsHeapTotalBytes": 25690112,
"scriptDurationMs": 11.941,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 2011.9359999999915,
"styleRecalcs": 10,
"styleRecalcDurationMs": 5.773000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 381.579,
"heapDeltaBytes": 5084612,
"heapUsedBytes": 69204316,
"domNodes": 20,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 11.874999999999998,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1696.111999999971,
"styleRecalcs": 76,
"styleRecalcDurationMs": 26.576,
"layouts": 16,
"layoutDurationMs": 3.2300000000000004,
"taskDurationMs": 600.815,
"heapDeltaBytes": -4173884,
"heapUsedBytes": 59767492,
"domNodes": 62,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 62.68,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1680.7799999999133,
"styleRecalcs": 76,
"styleRecalcDurationMs": 29.471999999999998,
"layouts": 16,
"layoutDurationMs": 3.441,
"taskDurationMs": 621.559,
"heapDeltaBytes": -4177416,
"heapUsedBytes": 59780252,
"domNodes": 65,
"jsHeapTotalBytes": 25427968,
"scriptDurationMs": 66.389,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-transition-enter",
"durationMs": 965.780000000052,
"styleRecalcs": 19,
"styleRecalcDurationMs": 22.703,
"layouts": 15,
"layoutDurationMs": 8.639000000000001,
"taskDurationMs": 716.858,
"heapDeltaBytes": 8377080,
"heapUsedBytes": 100188936,
"domNodes": 13673,
"jsHeapTotalBytes": 11796480,
"scriptDurationMs": 21.134,
"eventListeners": 2369,
"totalBlockingTimeMs": 100,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8136.45299999996,
"styleRecalcs": 250,
"styleRecalcDurationMs": 26.809000000000005,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3309.378,
"heapDeltaBytes": 10545332,
"heapUsedBytes": 70145300,
"domNodes": -278,
"jsHeapTotalBytes": 5545984,
"scriptDurationMs": 929.458,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8094.715000000065,
"styleRecalcs": 249,
"styleRecalcDurationMs": 31.496000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3461.8729999999996,
"heapDeltaBytes": 10146404,
"heapUsedBytes": 70319188,
"domNodes": -280,
"jsHeapTotalBytes": 5545984,
"scriptDurationMs": 985.185,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 11506.342000000019,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 11486.862000000001,
"heapDeltaBytes": -40938828,
"heapUsedBytes": 166910992,
"domNodes": -8312,
"jsHeapTotalBytes": -8855552,
"scriptDurationMs": 426.941,
"eventListeners": -16391,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 12439.706000000002,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12425.296,
"heapDeltaBytes": -55116192,
"heapUsedBytes": 151970448,
"domNodes": -8312,
"jsHeapTotalBytes": -9904128,
"scriptDurationMs": 443.17900000000003,
"eventListeners": -16389,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 13907.670999999993,
"styleRecalcs": 68,
"styleRecalcDurationMs": 12.408999999999976,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13874.641,
"heapDeltaBytes": -58254792,
"heapUsedBytes": 164223008,
"domNodes": -8312,
"jsHeapTotalBytes": -13049856,
"scriptDurationMs": 678.245,
"eventListeners": -16389,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.223333333333237,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 15457.535000000007,
"styleRecalcs": 77,
"styleRecalcDurationMs": 13.799000000000007,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15433.942000000003,
"heapDeltaBytes": -34592124,
"heapUsedBytes": 177025532,
"domNodes": -8312,
"jsHeapTotalBytes": -10235904,
"scriptDurationMs": 749.9409999999999,
"eventListeners": -16387,
"totalBlockingTimeMs": 41,
"frameDurationMs": 17.77333333333336,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 441.73299999999927,
"styleRecalcs": 14,
"styleRecalcDurationMs": 14.108999999999998,
"layouts": 2,
"layoutDurationMs": 0.3939999999999999,
"taskDurationMs": 92.109,
"heapDeltaBytes": 5332788,
"heapUsedBytes": 68770540,
"domNodes": 136,
"jsHeapTotalBytes": 5242880,
"scriptDurationMs": 7.656,
"eventListeners": 97,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 433.8510000000042,
"styleRecalcs": 14,
"styleRecalcDurationMs": 13.230000000000002,
"layouts": 3,
"layoutDurationMs": 0.476,
"taskDurationMs": 84.57199999999999,
"heapDeltaBytes": 5098584,
"heapUsedBytes": 68171108,
"domNodes": 125,
"jsHeapTotalBytes": 5242880,
"scriptDurationMs": 7.322,
"eventListeners": 99,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
}
]
} |
dosubot
Bot
added
the
size:S
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #14551 +/- ##
==========================================
+ Coverage 78.14% 78.16% +0.02%
==========================================
Files 1768 1785 +17
Lines 105794 107755 +1961
Branches 32899 33961 +1062
==========================================
+ Hits 82669 84225 +1556
- Misses 22659 23045 +386
- Partials 466 485 +19
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 76 files with indirect coverage changes 🚀 New features to boost your workflow:
|
…fixture
The regression fixture gave the interior SamplerCustomAdvanced node the same
id (1) as the outer SubgraphNode. LiteGraph's deduplicateSubgraphNodeIds
treats the loading workflow's own root node ids as reserved, so it silently
remapped the interior node's id to 2 on load. The test's composite execution
id ("1:1") therefore targeted a node locator that no longer existed, so the
websocket-driven preview never reached the watcher registered in
autoExposeKnownPreviewNodes for the real (remapped) locator, and the
assertion timed out. Giving the interior node a non-colliding id (2) avoids
the remap entirely and keeps the test's assumed execution id in sync with
the id LiteGraph actually assigns.
Drop the CANVAS_IMAGE_PREVIEW_NODE_TYPES allowlist and the nodeOutputStore watcher from autoExposeKnownPreviewNodes. Every interior node without a real preview widget now gets a virtual preview exposure registered unconditionally; whether it actually renders on the host SubgraphNode is already fully derived from live store state by usePromotedPreviews and the canvas draw-loop preview poller, so no per-type gating or one-shot watch is needed. The allowlist itself stays in canvasImagePreviewTypes.ts, used only by getPromotableWidgets for the manual widget-promotion menu.
claude
Bot
changed the title
…ures autoExposeKnownPreviewNodes goes back to allowlist-gated eager registration (matching main, minus the dead RAF fallback) so it no longer writes exposure entries for arbitrary node types into the persisted previewExposures list. The actual fix moves into usePromotedPreviews: alongside explicitly exposed sources, it now also derives a preview for any interior node that isn't already exposed, isn't a nested SubgraphNode, and has no real preview widget, purely by reading live nodeOutputStore state. Nothing is persisted for this path, so it renders once real preview data arrives without affecting exposure counts or serialized state.
dosubot
Bot
added
size:M
…ident-94) Adds an e2e test and a unit test that reproduce a distinct subgraph live-preview bug: LGraphNode.vue unconditionally hides the LivePreview element on any SubgraphNode via a `!lgraphNode?.isSubgraphNode()` guard (added in #12197 / 0157b47), so a mid-execution sampler preview never reaches the outer, non-entered SubgraphNode. This is separate from the allowlist gap fixed in this PR: that fix promotes previews through usePromotedPreviews/NodeContent, a rendering path that is not gated by isSubgraphNode(). It does not touch or bypass the LivePreview guard, so it does not fix this scenario - verified empirically: this unit test still fails on top of this branch's fix with the same "Unable to find an accessible element with the role img" error as on main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
I looked into whether this PR's fix also covers the regression tracked in incident-94 (a mid-execution sampler preview inside a subgraph not appearing on the outer, non-entered It's a distinct bug, on a separate rendering path, so I've added the regression tests here as commit e252352 rather than duplicating effort in a second PR:
Added:
Closing #14548 (which had these as draft tests) in favor of this PR now carrying them. Requested by Christian Byrne · Slack thread Generated by Claude Code |
3 tasks
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
Review from a multi-agent pass over this diff, with a verification gate and two adversarial refutation passes on top. Findings that did not survive verification were dropped, including a claim that deleting the requestAnimationFrame(updatePreviews) fallback regressed the classic canvas: git blame plus a mechanism trace show that callback could never fire (it is handed only to useNodeImage(...).showPreview(), whose onLoaded runs after the synchronous section already returned), so removing it is safe.
Two items that do not anchor to a specific line:
issue: the PR description documents commit a1d0a45b43, which d247aa33f6 reverted. autoExposeKnownPreviewNodes does still consult the allowlist (promotionUtils.ts:595), nothing is registered or persisted, and the promotionUtils test the body cites asserts the opposite of what the body says it asserts (the committed test asserts toEqual([])). The Changes list names 4 of the 8 changed files and omits usePromotedPreviews.ts, where the fix actually lives. Worth stating the Vue-renderer-only scope explicitly too, and adding Fixes #14597, Fixes #9859, Refs Comfy-Org/ComfyUI#15264, none of which are currently linked.
note: #14569 adds an equivalent ambient rollup (useAmbientSubgraphPreviews) over the same computed. Its version keeps usePromotedPreviews a pure exposure projection and already carries the input-preview and hideOutputImages guards this PR lacks, but its e2e fixture is two KSamplers, so it has no regression coverage for the node type this was reported against. Landing #14569's composable and porting this PR's sampler_custom_advanced_in_subgraph.json and subgraphPreviewAllowlistGap.spec.ts onto it would avoid shipping two rollups over the same state. This also looks like it needs needs-backport: the unfixed code is live on core/1.46 through core/1.49 and cloud/1.46 through cloud/1.49, and 1.46 predates appendNodeExecutionId, so that branch needs a hand-written variant rather than a cherry-pick.
| const exposedSourceIds = new Set( | ||
| exposures.map((exposure) => exposure.sourceNodeId) | ||
| ) | ||
| const unexposed = node.subgraph.nodes.filter( |
Contributor
There was a problem hiding this comment.
issue: the implicit branch keys off hasAnySource, which accepts committed nodeOutputs and not only live preview blobs. useImageUploadWidget.ts:128 writes folder: 'input' outputs inside a requestAnimationFrame on workflow load, so opening any saved workflow whose subgraph contains a LoadImage now paints a thumbnail on the host before the user runs anything. There is no exposure behind it, so SubgraphEditor.getActivePreviewRows cannot enumerate it and there is no way to dismiss it. ImagePreview.vue carries min-h-55, and useVueNodeResizeTracking syncs the resulting growth back into layoutStore bounds, so this also changes persisted node geometry on already-saved workflows.
Gating on nodePreviewImages only would match the stated intent ("a mid-execution sampler preview") and drop this case out. isInputPreviewOutput already exists at nodeOutputStore.ts:157, and #14569 applies both that guard and hideOutputImages.
| (interiorNode) => | ||
| !exposedSourceIds.has(interiorNode.id) && | ||
| !interiorNode.isSubgraphNode() && | ||
| !interiorNode.widgets?.some(isPreviewPseudoWidget) |
Contributor
There was a problem hiding this comment.
issue: this filter is self-defeating. showCanvasImagePreview (useNodeCanvasImagePreview.ts:20) pushes a $$canvas-image-preview widget onto node.widgets once node.imgs is populated, and SubgraphEditor.vue:156-164 calls updatePreviews on every interior node inside its interiorWidgets computed. Once that widget exists the node leaves unexposed, has no exposure to fall back on, and the promoted preview is gone for the rest of the session.
Reproduced against the real composable and fixtures: preview present, widget added, next store write yields []. The trigger needs a second selection change after the image loads rather than merely opening the panel, but the effect is permanent once it happens (removeCanvasImagePreview only runs for animated outputs).
exposedSourceIds already handles the double-render case this filter was presumably meant to prevent.
| ] | ||
| }) | ||
|
|
||
| const exposedSourceIds = new Set( |
Contributor
There was a problem hiding this comment.
issue: removing a preview exposure no longer sticks. SubgraphEditor.vue:292 calls removeExposure, the node then falls into unexposed, and the same preview is re-derived on the next recompute. It also bypasses the persisted cleared state: autoExposeKnownPreviewNodes deliberately honours properties.previewExposures === [] (there is an existing test for "user cleared"), but the implicit branch never reads that property, so a cleared host re-populates after save and reload.
ADR 0009 says preview exposures are "removed only by explicit user action or by destruction/unpacking of the owning host".
Worth noting the canvas context-menu demote path is not affected: demoteWidget only fires for a real widget, and such a node is excluded by the isPreviewPseudoWidget filter above, so removal does stick there. It is specifically the side-panel removal of a virtual exposure that becomes a no-op.
|
|
||
| if (hasPreviewWidget()) continue | ||
|
|
||
| if (supportsVirtualCanvasImagePreview(node)) { |
Contributor
There was a problem hiding this comment.
issue: the fix does not reach the classic canvas renderer. getPseudoWidgetPreviewTargets (litegraphService.ts:193) resolves its targets from previewExposureStore alone, and this allowlist gate is unchanged, so a SamplerCustomAdvanced still gets no exposure and still shows nothing there.
Comfy.VueNodes.Enabled is defaultValue: false with defaultsByInstallVersion: { '1.41.0': isCloud || isDesktop } (coreSettings.ts:1207), so OSS local installs are on the unfixed path, and LGraphCanvas.drawNode returns before onDrawBackground in Vue mode, meaning the two renderers are mutually exclusive rather than layered. Both new e2e specs are tagged @vue-nodes.
Either cover the classic path or scope it out explicitly in the description and file the follow-up, since the body currently claims the legacy renderer polls the same store state.
| }) | ||
| }) | ||
|
|
||
| describe('autoExposeKnownPreviewNodes — unlisted node types', () => { |
Contributor
There was a problem hiding this comment.
suggestion (non-blocking): this test passes with the entire promotionUtils.ts change reverted to origin/main, verified by copying it into a detached main worktree and running it there. On main, unlisted types went down the requestAnimationFrame branch, which never flushes under vitest, so no exposure was persisted on main either. It does catch over-exposure (forcing every node down the eager branch makes it fail), so it is worth keeping, but it does not cover the behaviour this PR changed in this file.
Separately, updatePreviewsMock at line 49 and its two not.toHaveBeenCalled() assertions are now vacuous since promotionUtils.ts no longer imports useLitegraphService, and the name "skips deferred updatePreviews when a preview widget already exists" no longer describes anything that exists.
| wstest( | ||
| 'Promotes a live sampler preview from inside a subgraph to the outer SubgraphNode', | ||
| async ({ comfyPage, getWebSocket }) => { | ||
| test.fail() |
Contributor
There was a problem hiding this comment.
note: keeping the expected-fail markers is fine, but they could use a breadcrumb. They pin the !lgraphNode?.isSubgraphNode() guard at LGraphNode.vue:172, not the allowlist gap this PR fixes: the interior node here is a KSampler, which is allowlisted and already promoted on main, and the assertion targets LivePreview's alt text rather than the NodeContent/ImagePreview path this PR feeds.
Confirmed by patching that line and running the suite: the vitest it.fails counterpart in LGraphNode.test.ts then hard-fails with a bare Error: Expect test to fail, naming no incident, file, or next step. A comment naming incident-94 and the exact line each marker waits on would save whoever fixes the guard a confusing red build.
| { | ||
| sourceNodeId: interiorNode.id, | ||
| sourceWidgetName: CANVAS_IMAGE_PREVIEW_WIDGET, | ||
| type: getPreviewMediaType(interiorNode), |
Contributor
There was a problem hiding this comment.
suggestion (non-blocking): getPreviewMediaType reads node.previewMediaType, which is only assigned inside updatePreviews -> showPreview (useNodeImage.ts:103,132). For an interior node in a subgraph the user has never entered, that never runs in Vue mode, so it is undefined and falls through to 'image', and NodeContent routes 'image' to ImagePreview, producing <img src="....webm"> for a video interior node.
On the explicit path these were coupled (the video pseudo-widget and previewMediaType = 'video' are set in the same call), so this only becomes reachable now that arbitrary interior nodes can promote. Deriving the type from the output payload via isVideoOutput (litegraphUtil.ts) would be sturdier than the mutable node field.
- Track explicitly-removed implicit preview sources in previewExposureStore so demoting a virtual/unexposed preview actually sticks instead of being re-derived on the next recompute. - Gate usePromotedPreviews' implicit branch on live nodePreviewImages only, not committed nodeOutputs, so a restored LoadImage thumbnail from a saved workflow doesn't leak through as an undismissable host preview. - Remove the isPreviewPseudoWidget filter from the implicit branch: it excluded nodes purely for having gained a $$canvas-image-preview pseudo-widget (which happens to any node with live images once SubgraphEditor's interiorWidgets computed runs updatePreviews), permanently killing the very previews this PR was meant to surface. - Derive preview media type from the actual output payload via isVideoOutput instead of the mutable node.previewMediaType field, which is never set for an interior node in a subgraph the user hasn't visually entered. - Clean up promotionUtils.test.ts: remove the dead useLitegraphService mock and its vacuous assertions now that promotionUtils.ts doesn't import it, and rewrite the test that depended on it. - Add breadcrumb comments on the two incident-94 expected-fail markers pointing at the actual (different, still-open) guard bug and the LGraphNode.vue:172 line to fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAFGAHSLhxcazgjp3rzH5X
dosubot
Bot
added
size:L
Contributor
|
Closing in favour of #14569, which fixes the same user-visible bug with a better-guarded mechanism. Rationale, from the review above: both PRs add an ambient rollup of interior-node previews over the same computed. #14569 keeps Three artifacts here are not redundant and should be ported onto that branch rather than lost:
The |
claude Bot
pushed a commit
that referenced
this pull request
Aug 5, 2026
#14551 was closed in favor of this PR's ambient-rollup mechanism, but it carried three artifacts not otherwise covered here: - browser_tests/assets/nodes/sampler_custom_advanced_in_subgraph.json + browser_tests/tests/subgraph/subgraphPreviewAllowlistGap.spec.ts: this PR's own e2e fixture uses two KSamplers, both allowlisted, so it would pass even if a node-type gate were reintroduced into the ambient path. This fixture pins a non-allowlisted node type (SamplerCustomAdvanced) instead. - Two incident-94 expected-fail regression tests (LGraphNode.test.ts, imagePreview.spec.ts) pinning the still-open `!lgraphNode?.isSubgraphNode()` guard at LGraphNode.vue:172, which neither PR touches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Requested by Christian Byrne · Slack thread
Summary
Fixes a regression where a mid-execution sampler preview inside a subgraph only bubbled up to the outer
SubgraphNodefor a hardcoded set of node types (KSampler,KSamplerAdvanced,PreviewImage,SaveImage,GLSLShader). Any other sampler variant — e.g.SamplerCustomAdvanced, or a third-party custom sampler — never showed a live preview on the collapsed subgraph.Renderer scope: this fix applies only to the Vue Nodes ("Nodes 2.0") renderer. It does not change the classic canvas renderer's preview behavior at all — see "Fix" below.
Root cause
PR #12197 (Subgraph Link Only Promotion, ADR 0009) replaced the old universal live-preview bubbling with an exposure allowlist:
autoExposeKnownPreviewNodes(src/core/graph/subgraph/promotionUtils.ts) only registered a persisted preview exposure for interior node types inCANVAS_IMAGE_PREVIEW_NODE_TYPES(src/composables/node/canvasImagePreviewTypes.ts). Every other node type never got an exposure at all, so a real preview arriving later over the websocket had nothing to promote through.Fix
autoExposeKnownPreviewNodesis unchanged frommain— it still only eagerly persists an exposure for allowlisted node types. (An earlier version of this fix made it register an exposure for every interior node unconditionally; that wrote spurious entries intoproperties.previewExposuresregardless of whether a node ever produced a preview, and was reverted.)Instead, the actual fix lives in
usePromotedPreviews(src/composables/node/usePromotedPreviews.ts), which now derives previews for non-exposed interior nodes directly from live store state on every read, without persisting anything:previewExposureStoreentry.SubgraphNodeand isn't already exposed gets a derived preview oncenodeOutputStore.nodePreviewImageshas live progress-preview data for it — no allowlist, no persisted state, so it renders once real preview data arrives for any node type. Gated to live preview blobs only (not committednodeOutputs), so a restoredLoadImagethumbnail from a saved workflow doesn't leak through as an undismissable preview on load. If the user explicitly removes a preview (real or virtual), it stays removed and won't be re-derived on the next recompute.Only the Vue node renderer reads this composable. The classic canvas renderer (
LGraphCanvas.drawNode/getPseudoWidgetPreviewTargetsinlitegraphService.ts) is untouched by this PR and continues to resolve preview targets purely frompreviewExposureStore. Nodes 2.0 and the classic canvas are mutually exclusive renderers (Comfy.VueNodes.Enabled), so classic canvas does not benefit from this fix — that's a deliberate scope boundary, not a gap.Changes
src/composables/node/usePromotedPreviews.ts: adds the derived/implicit preview branch described above (this is where the actual fix lives); derives media type (image/video/audio) from the output payload instead of the mutablenode.previewMediaTypefield, which is never set for an interior node the user hasn't visually entered.src/composables/node/usePromotedPreviews.test.ts: regression coverage for the above — an explicitly-removed preview doesn't reappear, a committed input-preview output isn't implicitly promoted, gaining a canvas-image-preview pseudo-widget doesn't kill the implicit preview, and video type is derived correctly withoutpreviewMediaType.src/stores/previewExposureStore.ts: tracks sources explicitly demoted while they had no exposure entry to remove (a virtual/implicit preview), so removal actually sticks instead of being re-derived on the next recompute.src/stores/previewExposureStore.test.ts: regression coverage for the newisSourceExplicitlyRemovedtracking — a source is flagged only afterremoveExposuredemotes it, the flag clears once the source is re-exposed viaaddExposure, andclearGraphresets it for the whole graph.src/stores/nodeOutputStore.ts: exposes the existingisInputPreviewOutputhelper for reuse.src/core/graph/subgraph/promotionUtils.ts:autoExposeKnownPreviewNodesreverted to the allowlist-gated behavior onmain(minus the deadrequestAnimationFramefallback).src/core/graph/subgraph/promotionUtils.test.ts: keeps the existing unlisted-node-type test, which assertsautoExposeKnownPreviewNodesdoes not eagerly persist an exposure for an unlisted type (SamplerCustomAdvanced) — useful for catching a future over-exposure regression, but it also passes unmodified againstpromotionUtils.tsonmain, so it isn't itself coverage for this PR's fix (that'susePromotedPreviews.test.ts, above). Also removes a deaduseLitegraphServicemock and its now-vacuousnot.toHaveBeenCalled()assertions (promotionUtils.tsno longer importsuseLitegraphService), and rewrites the test that depended on it to assert the real preview-widget exposure behavior instead.browser_tests/tests/subgraph/subgraphPreviewAllowlistGap.spec.ts+browser_tests/assets/nodes/sampler_custom_advanced_in_subgraph.json: e2e regression test simulating a mid-execution websocket preview for aSamplerCustomAdvancednode nested in a subgraph, asserting the preview renders on the outerSubgraphNode.browser_tests/tests/vueNodes/interactions/node/imagePreview.spec.ts+src/renderer/extensions/vueNodes/components/LGraphNode.test.ts: expected-fail regression tests for a separate, still-open bug found while investigating this incident — the!lgraphNode?.isSubgraphNode()guard atLGraphNode.vue:172— eachtest.fail()/it.fails()marker is now commented with the guard line to fix and a reminder to remove the marker (not just delete the test) once it's addressed.Breaking: None.
Fixes #14597
Refs Comfy-Org/ComfyUI#15264
(Not linking #9859: that report is explicitly about the classic canvas renderer with Nodes 2.0 already working for the reporter, which is out of scope here — see "Renderer scope" above.)