test: custom-node E2E regression suite#13389
Open
CodeJuggernaut wants to merge 66 commits into
Open
Conversation
Data-driven Playwright harness verifying custom-node packs load and render under both LiteGraph 1.0 and Vue Nodes 2.0 against a real ComfyUI backend. Pure classifier/validator/manifest logic is unit-tested; the regression spec renders each pack's nodes in both renderers (Vue via data-node-id DOM) and a self-check runs a workflow to confirm execution-error capture. Proven against Impact Pack + VideoHelperSuite. Makes ComfyPage.createUser idempotent so the suite runs against a persistent backend (Desktop server user storage).
Normalize the executing event tap: its CustomEvent detail is a bare node-id string, so the previous object-spread left the executed-set permanently empty. The self-check now asserts a non-empty executed-set to keep that path live. T0 now clears the graph per renderer pass, asserts exact node counts, and verifies each added pack node's own data-node-id mounts in the Vue pass (default-workflow nodes can no longer satisfy the assertions). Console capture starts before the renderer toggle. Added an object_info sanity floor so a depleted getNodeDefs fails loudly instead of skipping everything. Run/io tiers gain test.setTimeout, requiresModels gating, and an empty-workflow guard. Interrupted runs get pure-spec coverage; the shared console collector moves to fixtures/utils.
Impact runs ImpactInt and ImpactFloat into PreviewAny as a group; VHS decodes the existing plain_video.mp4 asset through VHS_LoadVideoPath into VHS_VideoInfo. The executed-set check asserts each expected node individually executed, so group workflows still verify per-node execution. Requires a cache-disabled backend (--cache-none) with the video staged in its input dir; documented on the manifest workflow field.
Every browser tier now asserts the app's user-facing error surfaces (error overlay, error dialog, node render errors, error toasts) are absent at test start and after each pass, so a run is green only if a human watching the screen sees zero errors. The harness self-check asserts the overlay IS visible after a forced execution error, keeping the selectors provably live. Sessions boot with a blank graph (Comfy.TutorialCompleted=false) because the bundled default template references models absent on a scoped backend; the tutorial path's auto-opened template browser is dismissed per test. Settings now reach the session on single-user server-storage backends by routing devtools set_settings to the default user, and the errors tab stays enabled so error indicators are never suppressed in this suite. The smoke test loads a core-only model-free workflow instead of the SD1.5 default asset.
T2a gated on the ComfyUI-test-framework 'Assert Executed' nodes, which are not published for any backend yet, so the tier could only ever skip. A test that cannot run anywhere is reporting noise; restore it from history when the assertion pack lands. Suite is now 16 passed, zero skips, zero failures.
test:custom-nodes runs the whole suite headless (the gate); :watch opens a headed slow-motion run of the browser tiers; :debug steps through them in the Playwright Inspector. All target the local dev server on :5173 and use the committed system-Chrome config (no bundled-chromium download). Pass -g to :watch / :debug to run a single test, e.g. -g 'VideoHelperSuite.*T1'.
…uite One script per pack tier (impact-render/impact-run/vhs-render/vhs-run) plus the self-check, all opening the Playwright Inspector so anyone can step through what the robot does. README covers prerequisites, every script, a worked example, the zero-visible-errors contract, and how to add a pack.
🌐 Website E2ETip All tests passed.
|
🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ✅ 1708 passed, 0 failed · 2 flaky📊 Browser Reports
📦 Bundle: 7.78 MB gzip 🟢 -30 BDetailsSummary
Category Glance App Entry Points — 47.8 kB (baseline 47.8 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.27 MB (baseline 1.27 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 98 kB (baseline 98 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed / 3 unchanged Panels & Settings — 547 kB (baseline 547 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 16 unchanged User & Accounts — 29.1 kB (baseline 29.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed / 3 unchanged Editors & Dialogs — 117 kB (baseline 117 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 4 added / 4 removed / 1 unchanged UI Components — 57.5 kB (baseline 57.5 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed / 8 unchanged Data & Services — 270 kB (baseline 270 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 13 added / 13 removed / 3 unchanged Utilities & Hooks — 3.39 MB (baseline 3.39 MB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 16 added / 16 removed / 18 unchanged Vendor & Third-Party — 15.3 MB (baseline 15.3 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 11.7 MB (baseline 11.7 MB) • ⚪ 0 BBundles that do not match a named category
Status: 67 added / 67 removed / 98 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-07-09T02:27:34.138Z",
"gitSha": "f02830fe00f0d2b0de2687343f356545e92fe58b",
"branch": "nathaniel/custom-node-e2e-suite",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2036.231000000015,
"styleRecalcs": 9,
"styleRecalcDurationMs": 5.612000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 315.16299999999995,
"heapDeltaBytes": 3041064,
"heapUsedBytes": 68067764,
"domNodes": 18,
"jsHeapTotalBytes": 22507520,
"scriptDurationMs": 13.613,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-idle",
"durationMs": 2024.1770000000088,
"styleRecalcs": 10,
"styleRecalcDurationMs": 8.354000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 351.142,
"heapDeltaBytes": -7327980,
"heapUsedBytes": 62304620,
"domNodes": -276,
"jsHeapTotalBytes": 19550208,
"scriptDurationMs": 15.767000000000003,
"eventListeners": -195,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1726.173999999986,
"styleRecalcs": 72,
"styleRecalcDurationMs": 30.956999999999997,
"layouts": 12,
"layoutDurationMs": 2.754,
"taskDurationMs": 600.916,
"heapDeltaBytes": -12638356,
"heapUsedBytes": 56832500,
"domNodes": -241,
"jsHeapTotalBytes": 19812352,
"scriptDurationMs": 82.494,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1736.1099999999965,
"styleRecalcs": 72,
"styleRecalcDurationMs": 29.48,
"layouts": 12,
"layoutDurationMs": 3.1639999999999997,
"taskDurationMs": 642.365,
"heapDeltaBytes": -3689756,
"heapUsedBytes": 48466968,
"domNodes": -270,
"jsHeapTotalBytes": 16117760,
"scriptDurationMs": 92.977,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1744.0860000000384,
"styleRecalcs": 30,
"styleRecalcDurationMs": 12.721000000000002,
"layouts": 6,
"layoutDurationMs": 0.5310000000000001,
"taskDurationMs": 295.943,
"heapDeltaBytes": -2991488,
"heapUsedBytes": 49447860,
"domNodes": -239,
"jsHeapTotalBytes": 9302016,
"scriptDurationMs": 16.174,
"eventListeners": -186,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1708.235000000002,
"styleRecalcs": 32,
"styleRecalcDurationMs": 15.7,
"layouts": 6,
"layoutDurationMs": 0.626,
"taskDurationMs": 293.582,
"heapDeltaBytes": 2321300,
"heapUsedBytes": 61498348,
"domNodes": 78,
"jsHeapTotalBytes": 24641536,
"scriptDurationMs": 22.384999999999998,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "dom-widget-clipping",
"durationMs": 430.01700000002074,
"styleRecalcs": 12,
"styleRecalcDurationMs": 6.383000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 263.77799999999996,
"heapDeltaBytes": 7047772,
"heapUsedBytes": 66215248,
"domNodes": 20,
"jsHeapTotalBytes": 19136512,
"scriptDurationMs": 42.789,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "dom-widget-clipping",
"durationMs": 486.29199999999173,
"styleRecalcs": 12,
"styleRecalcDurationMs": 10.052999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 305.519,
"heapDeltaBytes": -22911972,
"heapUsedBytes": 46727348,
"domNodes": -273,
"jsHeapTotalBytes": 6180864,
"scriptDurationMs": 42.097,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "large-graph-idle",
"durationMs": 2036.0959999999864,
"styleRecalcs": 10,
"styleRecalcDurationMs": 6.761000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 427.61499999999995,
"heapDeltaBytes": -25766312,
"heapUsedBytes": 58163948,
"domNodes": -268,
"jsHeapTotalBytes": 3485696,
"scriptDurationMs": 66.475,
"eventListeners": -195,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 1991.5050000000178,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.0809999999999995,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 410.325,
"heapDeltaBytes": 7692972,
"heapUsedBytes": 61204436,
"domNodes": -265,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 78.911,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2091.038000000026,
"styleRecalcs": 69,
"styleRecalcDurationMs": 16.241,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 878.405,
"heapDeltaBytes": -5730176,
"heapUsedBytes": 79646240,
"domNodes": -271,
"jsHeapTotalBytes": 1855488,
"scriptDurationMs": 311.751,
"eventListeners": -195,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2052.186000000006,
"styleRecalcs": 67,
"styleRecalcDurationMs": 16.151000000000007,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 854.2660000000002,
"heapDeltaBytes": 27601252,
"heapUsedBytes": 80754748,
"domNodes": -275,
"jsHeapTotalBytes": 204800,
"scriptDurationMs": 298.631,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3025.156999999979,
"styleRecalcs": 64,
"styleRecalcDurationMs": 16.091,
"layouts": 60,
"layoutDurationMs": 7.313999999999999,
"taskDurationMs": 1005.0669999999999,
"heapDeltaBytes": 12208872,
"heapUsedBytes": 66664944,
"domNodes": -277,
"jsHeapTotalBytes": 2621440,
"scriptDurationMs": 390.777,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3085.2689999999825,
"styleRecalcs": 68,
"styleRecalcDurationMs": 21.369,
"layouts": 60,
"layoutDurationMs": 7.783000000000001,
"taskDurationMs": 1205.0140000000001,
"heapDeltaBytes": -4037480,
"heapUsedBytes": 70123164,
"domNodes": 18,
"jsHeapTotalBytes": 11153408,
"scriptDurationMs": 450.316,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2020.1220000000149,
"styleRecalcs": 9,
"styleRecalcDurationMs": 5.974000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 429.739,
"heapDeltaBytes": -8454152,
"heapUsedBytes": 60336280,
"domNodes": -263,
"jsHeapTotalBytes": -1056768,
"scriptDurationMs": 70.619,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 1997.8590000000054,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.2639999999999985,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 509.63599999999997,
"heapDeltaBytes": 10507660,
"heapUsedBytes": 71839472,
"domNodes": 16,
"jsHeapTotalBytes": 5767168,
"scriptDurationMs": 83.72099999999999,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 511.1220000000003,
"styleRecalcs": 47,
"styleRecalcDurationMs": 10.944,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 329.05400000000003,
"heapDeltaBytes": -4041928,
"heapUsedBytes": 48001268,
"domNodes": -272,
"jsHeapTotalBytes": 389120,
"scriptDurationMs": 99.312,
"eventListeners": -193,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999727
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 525.6739999999809,
"styleRecalcs": 47,
"styleRecalcDurationMs": 10.000000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 340.49800000000005,
"heapDeltaBytes": -22484044,
"heapUsedBytes": 47223524,
"domNodes": -273,
"jsHeapTotalBytes": 6443008,
"scriptDurationMs": 98.785,
"eventListeners": -193,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "subgraph-idle",
"durationMs": 2021.3110000000256,
"styleRecalcs": 12,
"styleRecalcDurationMs": 7.485000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 306.19399999999996,
"heapDeltaBytes": 18962396,
"heapUsedBytes": 71469412,
"domNodes": -288,
"jsHeapTotalBytes": 15593472,
"scriptDurationMs": 10.766000000000002,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2020.4919999999902,
"styleRecalcs": 11,
"styleRecalcDurationMs": 8.267,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 375.003,
"heapDeltaBytes": -8240240,
"heapUsedBytes": 61539084,
"domNodes": -275,
"jsHeapTotalBytes": 19812352,
"scriptDurationMs": 15.709000000000001,
"eventListeners": -195,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1706.388000000004,
"styleRecalcs": 77,
"styleRecalcDurationMs": 29.792,
"layouts": 16,
"layoutDurationMs": 3.4479999999999995,
"taskDurationMs": 514.5269999999999,
"heapDeltaBytes": 15952748,
"heapUsedBytes": 74412376,
"domNodes": 66,
"jsHeapTotalBytes": 25427968,
"scriptDurationMs": 68.246,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1687.702999999999,
"styleRecalcs": 80,
"styleRecalcDurationMs": 30.93,
"layouts": 16,
"layoutDurationMs": 3.382,
"taskDurationMs": 548.509,
"heapDeltaBytes": -10488796,
"heapUsedBytes": 48415052,
"domNodes": 66,
"jsHeapTotalBytes": 26214400,
"scriptDurationMs": 77.067,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-transition-enter",
"durationMs": 883.2219999999893,
"styleRecalcs": 20,
"styleRecalcDurationMs": 25.941000000000006,
"layouts": 16,
"layoutDurationMs": 11.935999999999998,
"taskDurationMs": 646.6750000000001,
"heapDeltaBytes": 29901104,
"heapUsedBytes": 101568076,
"domNodes": 13673,
"jsHeapTotalBytes": 16777216,
"scriptDurationMs": 24.98999999999999,
"eventListeners": 2531,
"totalBlockingTimeMs": 101,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8144.889999999976,
"styleRecalcs": 252,
"styleRecalcDurationMs": 53.34199999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 2970.834,
"heapDeltaBytes": -15248276,
"heapUsedBytes": 68698752,
"domNodes": -260,
"jsHeapTotalBytes": 7622656,
"scriptDurationMs": 957.6510000000001,
"eventListeners": -179,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333338,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8141.12700000004,
"styleRecalcs": 251,
"styleRecalcDurationMs": 57.407,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3373.15,
"heapDeltaBytes": 21433144,
"heapUsedBytes": 93236732,
"domNodes": 20,
"jsHeapTotalBytes": 19456000,
"scriptDurationMs": 1091.532,
"eventListeners": 20,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 11064.068000000021,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 11039.738,
"heapDeltaBytes": -43633364,
"heapUsedBytes": 172756252,
"domNodes": -8314,
"jsHeapTotalBytes": 21069824,
"scriptDurationMs": 437.604,
"eventListeners": -16372,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 11298.16500000004,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 11288.889000000001,
"heapDeltaBytes": -40446724,
"heapUsedBytes": 166351244,
"domNodes": -3301,
"jsHeapTotalBytes": 9961472,
"scriptDurationMs": 452.475,
"eventListeners": -16368,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.669999999999952,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-pan",
"durationMs": 13453.634999999962,
"styleRecalcs": 67,
"styleRecalcDurationMs": 17.60299999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13434.118999999999,
"heapDeltaBytes": -26898728,
"heapUsedBytes": 179029084,
"domNodes": -3300,
"jsHeapTotalBytes": 6729728,
"scriptDurationMs": 707.097,
"eventListeners": -16376,
"totalBlockingTimeMs": 2,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-pan",
"durationMs": 14019.350000000031,
"styleRecalcs": 65,
"styleRecalcDurationMs": 17.830000000000013,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13996.855,
"heapDeltaBytes": -34657680,
"heapUsedBytes": 165893456,
"domNodes": -3298,
"jsHeapTotalBytes": 16748544,
"scriptDurationMs": 681.75,
"eventListeners": -16374,
"totalBlockingTimeMs": 25,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 467.04399999998714,
"styleRecalcs": 18,
"styleRecalcDurationMs": 21.113,
"layouts": 4,
"layoutDurationMs": 0.9990000000000001,
"taskDurationMs": 179.15300000000002,
"heapDeltaBytes": -22015560,
"heapUsedBytes": 48709072,
"domNodes": -161,
"jsHeapTotalBytes": 4345856,
"scriptDurationMs": 14.997000000000002,
"eventListeners": -130,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.799999999999727
},
{
"name": "workflow-execution",
"durationMs": 477.94999999996435,
"styleRecalcs": 17,
"styleRecalcDurationMs": 23.531000000000002,
"layouts": 4,
"layoutDurationMs": 1.0299999999999998,
"taskDurationMs": 198.14499999999998,
"heapDeltaBytes": -22297716,
"heapUsedBytes": 48466616,
"domNodes": -177,
"jsHeapTotalBytes": 4870144,
"scriptDurationMs": 18.683999999999997,
"eventListeners": -134,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66666666666665,
"p95FrameDurationMs": 16.700000000000273
}
]
} |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a manifest-driven custom-node browser regression suite, with new fixtures, validation helpers, browser specs, workflow assets, scripts, CI, and docs. It also updates ChangesCustom-node regression suite
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CodeJuggernaut
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
browser_tests/README.md (1)
22-22: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAvoid hardcoding the suite total.
16 passedwill drift as soon as the manifest grows, which is the documented extension path for this suite. Describe success in terms of all manifest packs passing instead.♻️ Suggested wording
-| `pnpm test:custom-nodes` | whole suite headless - the pass/fail gate (expect `16 passed`, zero skips) | +| `pnpm test:custom-nodes` | whole suite headless - the pass/fail gate (all manifest packs pass, zero unexpected skips) |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@browser_tests/README.md` at line 22, The browser_tests README currently hardcodes the suite result as 16 passed, which will become stale as the manifest grows. Update the success wording in the README to describe the outcome generically in terms of all manifest packs passing, and adjust the related browser_tests documentation text so it no longer depends on a fixed total.
♻️ Duplicate comments (1)
browser_tests/tests/customNodes/spikeDesktop.spec.ts (1)
28-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate of
customNode.regression.spec.tsbeforeEach.Same concern as flagged in
customNode.regression.spec.ts(lines 42-47): this block is copy-pasted verbatim. Consolidating into a shared helper avoids the two specs diverging silently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@browser_tests/tests/customNodes/spikeDesktop.spec.ts` around lines 28 - 33, The test setup in test.beforeEach is duplicated verbatim from customNode.regression.spec.ts, so consolidate the repeated template-workflows-content hide logic into a shared helper and call it from both specs. Move the shared steps (getting the getByTestId('template-workflows-content'), waiting for visible, pressing Escape, and waiting for hidden) into one reusable function or fixture, then update spikeDesktop.spec.ts and the regression spec to use that helper so they stay in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@browser_tests/fixtures/customNode/manifest.ts`:
- Line 4: The manifest lookup in the customNode manifest module still relies on
the process working directory, so update the path handling around MANIFEST_PATH
and the readFileSync(resolve(...)) usage to resolve relative to import.meta.url
instead. Adjust the manifest loading logic in manifest.ts so it builds an
absolute path from this module’s location, keeping the existing MANIFEST_PATH
constant but no longer depending on a repo-root relative string.
In `@browser_tests/fixtures/utils/errorSurfaces.ts`:
- Around line 9-10: The test selectors in errorSurfaces use raw string literals
for test ids, which should be centralized to avoid drift. Update the
errorOverlay and errorDialog locators to reference the shared TestIds constants
used elsewhere in ComfyPage.ts instead of hardcoded strings, keeping the
selector definitions aligned with the centralized test-id source.
In `@browser_tests/tests/customNodes/customNode.regression.spec.ts`:
- Around line 29-38: The test.use initialSettings block is duplicated across
customNode.regression.spec.ts and spikeDesktop.spec.ts, so centralize the shared
settings into a common constant such as customNodeSuiteSettings and reuse it in
both specs. Keep the existing values for Comfy.TutorialCompleted, Comfy.userId,
and Comfy.RightSidePanel.ShowErrorsTab, and update the relevant test.use calls
to reference the shared symbol so both suites stay in sync.
- Around line 42-47: The `beforeEach` in `customNode.regression.spec.ts`
duplicates the template-dismissal steps already used in `spikeDesktop.spec.ts`.
Extract this logic into a shared helper such as
`dismissTemplatesDialog(comfyPage)` in the same utilities area as
`errorSurfaces` and `collectConsoleErrors`, then replace both inline
`beforeEach` blocks with the helper call to keep the behavior centralized and
prevent drift.
In `@browser_tests/tests/customNodes/README.md`:
- Around line 63-68: The add-pack checklist in the customNodes README is missing
manifest fields that loadManifest() actually validates, so the documented
example would fail before tests run. Update the instructions around the
customNodeManifest entry and the run/load steps to include the required schema
fields repo, pin, workflow, requiresGpu, and requiresModels, and keep the
guidance aligned with the existing fixtures and assets referenced by
browser_tests/fixtures/data/customNodeManifest.json and
browser_tests/assets/customNodes/.
In `@browser_tests/tests/customNodes/spikeDesktop.spec.ts`:
- Around line 1-63: The test file name and its purpose are inconsistent: the
current spike-oriented name does not match the maintained smoke/regression suite
described by the `test.describe('smoke: core workflow')` block and the
`smokeWorkflow` setup. Rename the spec file to a stable, descriptive name such
as `coreSmoke.spec.ts` (or equivalent) so the filename matches the long-lived
test intent and is easy to discover alongside the `collectConsoleErrors` and
`errorSurfaces` checks.
---
Outside diff comments:
In `@browser_tests/README.md`:
- Line 22: The browser_tests README currently hardcodes the suite result as 16
passed, which will become stale as the manifest grows. Update the success
wording in the README to describe the outcome generically in terms of all
manifest packs passing, and adjust the related browser_tests documentation text
so it no longer depends on a fixed total.
---
Duplicate comments:
In `@browser_tests/tests/customNodes/spikeDesktop.spec.ts`:
- Around line 28-33: The test setup in test.beforeEach is duplicated verbatim
from customNode.regression.spec.ts, so consolidate the repeated
template-workflows-content hide logic into a shared helper and call it from both
specs. Move the shared steps (getting the
getByTestId('template-workflows-content'), waiting for visible, pressing Escape,
and waiting for hidden) into one reusable function or fixture, then update
spikeDesktop.spec.ts and the regression spec to use that helper so they stay in
sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ae0a1a1a-deae-4829-aa7b-479f132dcd81
📒 Files selected for processing (20)
browser_tests/README.mdbrowser_tests/assets/customNodes/core_smoke.jsonbrowser_tests/assets/customNodes/impact_primitives_run.jsonbrowser_tests/assets/customNodes/vhs_video_pipeline_run.jsonbrowser_tests/fixtures/ComfyPage.tsbrowser_tests/fixtures/customNode/ComfyTarget.tsbrowser_tests/fixtures/customNode/manifest.tsbrowser_tests/fixtures/customNode/objectInfoValidator.tsbrowser_tests/fixtures/customNode/runResult.tsbrowser_tests/fixtures/data/customNodeManifest.jsonbrowser_tests/fixtures/utils/consoleErrorCollector.tsbrowser_tests/fixtures/utils/errorSurfaces.tsbrowser_tests/tests/customNodes/README.mdbrowser_tests/tests/customNodes/customNode.regression.spec.tsbrowser_tests/tests/customNodes/manifest.pure.spec.tsbrowser_tests/tests/customNodes/objectInfoValidator.pure.spec.tsbrowser_tests/tests/customNodes/runResult.pure.spec.tsbrowser_tests/tests/customNodes/spikeDesktop.spec.tspackage.jsonplaywright.chrome.config.ts
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #13389 +/- ##
==========================================
- Coverage 78.17% 78.13% -0.04%
==========================================
Files 1648 1651 +3
Lines 95561 96845 +1284
Branches 33460 32627 -833
==========================================
+ Hits 74704 75670 +966
- Misses 20182 20464 +282
- Partials 675 711 +36
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
CodeJuggernaut
force-pushed
the
nathaniel/custom-node-e2e-suite
branch
from
f8d2ee6 to
addcb60
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
browser_tests/fixtures/customNode/manifest.ts (1)
25-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate
requiresGpuhere as well.The regression spec already branches on
entry.requiresGpu, butassertEntry()does not require that field. If it is missing or misspelled in the manifest, GPU-only packs can be treated as CPU-runnable and the run tier will misclassify them.🔧 Minimal fix
- ['pack', 'workflow', 'expectedNodes', 'tiers', 'requiresModels'] as const + ['pack', 'workflow', 'expectedNodes', 'tiers', 'requiresModels', 'requiresGpu'] as const🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@browser_tests/fixtures/customNode/manifest.ts` around lines 25 - 34, `assertEntry()` in `manifest.ts` is missing validation for `requiresGpu`, so manifest entries can omit or misspell it and still pass. Update the missing-field checks in `assertEntry(entry, index)` to require `requiresGpu` alongside the other mandatory properties, using the same null/undefined validation pattern and error reporting already used for `pack`, `workflow`, `expectedNodes`, `tiers`, and `requiresModels`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@browser_tests/tests/customNodes/connectivity.spec.ts`:
- Around line 292-294: The `outDot.dragTo` call in `connectivity.spec.ts` is
tripping the `no-force-option` lint rule because it uses `{ force: true }`; keep
the forced drag since the z-999 overlay blocks pointer events, but add a local
lint suppression on that specific call using the existing overlay justification
so the `connectivity.spec` test remains valid and CI passes.
---
Outside diff comments:
In `@browser_tests/fixtures/customNode/manifest.ts`:
- Around line 25-34: `assertEntry()` in `manifest.ts` is missing validation for
`requiresGpu`, so manifest entries can omit or misspell it and still pass.
Update the missing-field checks in `assertEntry(entry, index)` to require
`requiresGpu` alongside the other mandatory properties, using the same
null/undefined validation pattern and error reporting already used for `pack`,
`workflow`, `expectedNodes`, `tiers`, and `requiresModels`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: faeeeab0-0c5a-45ba-9239-21ef7d2f47b2
📒 Files selected for processing (7)
browser_tests/fixtures/customNode/manifest.tsbrowser_tests/fixtures/customNode/typePairing.tsbrowser_tests/fixtures/data/customNodeManifest.jsonbrowser_tests/tests/customNodes/README.mdbrowser_tests/tests/customNodes/connectivity.spec.tsbrowser_tests/tests/customNodes/typePairing.pure.spec.tspackage.json
CodeJuggernaut
force-pushed
the
nathaniel/custom-node-e2e-suite
branch
from
bbb543c to
7a92da1
Compare
A type-pairing generator indexes /object_info producers and consumers and plans one representative typed edge per slot, excluding wildcard slots (isValidConnection short-circuits on * before the real type compare, so a wildcard link proves reachability, not interop). The breadth sweep connects every planned edge through the real validator in-page and requires each link to survive serialize/configure and appear in graphToPrompt output; verified up front that graphToPrompt emits links even when other required inputs dangle. A curated subset is dragged slot-dot to slot-dot under both renderers, addressed by data-slot-key so shared labels cannot misfire. Orphan types are reported, never failed; connect vetoes must match a committed allow-list. Manifest packs opt in via a connectivity tier that needs no extra assets.
CodeJuggernaut
force-pushed
the
nathaniel/custom-node-e2e-suite
branch
from
7a92da1 to
8b81a4f
Compare
A permanent self-check feeds the shared pair executor a type-incompatible pair and a fabricated slot name and requires CONNECT_REJECTED and SLOT_CONTRACT_MISMATCH back, so a green sweep can never come from a classifier that lost the ability to fail. The breadth test asserts every connectivity-tier pack contributes pairs, guarding pack attribution. The drag tier's widget-primitive exclusion is removed: widget-backed inputs render real slot dots under Vue Nodes (verified empirically), so every pack now gets an in-pack drag in both renderers, asserted present.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
browser_tests/fixtures/customNode/manifest.ts (1)
25-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTighten manifest shape checks.
assertEntryonly rejects nullish values, so malformedtiers/expectedNodesvalues can still pass and later skew the connectivity filter and length-based checks. Validate the actual array types here (and keeptimeoutMsfinite/positive) so bad manifests fail fast.Based on source_other:
browser_tests/tests/customNodes/connectivity.spec.tsandbrowser_tests/tests/customNodes/customNode.regression.spec.tsconsume these fields as arrays.🛠️ Proposed fix
function assertEntry(entry: CustomNodeManifestEntry, index: number): void { - const missing: string[] = ( - ['pack', 'workflow', 'expectedNodes', 'tiers', 'requiresModels'] as const - ).filter((key) => entry[key] == null) - if (typeof entry.timeoutMs !== 'number') missing.push('timeoutMs') + const missing: string[] = [] + if (typeof entry.pack !== 'string' || entry.pack.length === 0) missing.push('pack') + if (typeof entry.workflow !== 'string' || entry.workflow.length === 0) missing.push('workflow') + if (!Array.isArray(entry.expectedNodes)) missing.push('expectedNodes') + if (!Array.isArray(entry.tiers)) missing.push('tiers') + if (!Array.isArray(entry.requiresModels)) missing.push('requiresModels') + if (!Number.isFinite(entry.timeoutMs) || entry.timeoutMs <= 0) missing.push('timeoutMs')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@browser_tests/fixtures/customNode/manifest.ts` around lines 25 - 34, The manifest validation in assertEntry only checks for nullish values, so malformed tiers and expectedNodes can still pass through and break later array-based logic. Update assertEntry to verify those fields are actual arrays before accepting the entry, and also tighten timeoutMs validation in the same function to require a finite positive number. Keep the checks centralized in assertEntry so bad custom node manifests fail fast before connectivity.spec and customNode.regression.spec consume the data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@browser_tests/tests/customNodes/connectivity.spec.ts`:
- Line 94: The breadth sweep in connectivity.spec.ts starts a console error
collector via collectConsoleErrors(comfyPage.page), but the collected errors are
never asserted before stopping it. Update the breadth-sweep test around the
collectConsoleErrors usage and the final stop/teardown block to check
consoleErrors.errors the same way the fidelity test does, so any console errors
during the connect/serialize/prompt loop fail the test.
- Around line 59-61: The concrete() helper is duplicating the wildcard check
instead of reusing the shared predicate. Export isWildcard from typePairing.ts
and update concrete() in connectivity.spec.ts to use !isWildcard(slot.type) so
the wildcard logic stays centralized and cannot drift apart.
- Around line 217-230: The “in-pack” selection in the connectivity test is using
the full node pool, so it can return a compatible pair from any pack instead of
the current one. Update the logic around connectivityEntries to build the
candidate set from only the nodes in entry.pack before calling planPairs, rather
than filtering after the fact. Use the planPairs result and the
pair.producer.pack/pair.consumer.pack check only as a sanity guard, and keep
dragEdges populated from same-pack matches.
---
Outside diff comments:
In `@browser_tests/fixtures/customNode/manifest.ts`:
- Around line 25-34: The manifest validation in assertEntry only checks for
nullish values, so malformed tiers and expectedNodes can still pass through and
break later array-based logic. Update assertEntry to verify those fields are
actual arrays before accepting the entry, and also tighten timeoutMs validation
in the same function to require a finite positive number. Keep the checks
centralized in assertEntry so bad custom node manifests fail fast before
connectivity.spec and customNode.regression.spec consume the data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fc2ff995-7c7c-4e41-b687-b2109380e87f
📒 Files selected for processing (7)
browser_tests/fixtures/customNode/manifest.tsbrowser_tests/fixtures/customNode/typePairing.tsbrowser_tests/fixtures/data/customNodeManifest.jsonbrowser_tests/tests/customNodes/README.mdbrowser_tests/tests/customNodes/connectivity.spec.tsbrowser_tests/tests/customNodes/typePairing.pure.spec.tspackage.json
Resolve the manifest path from import.meta.url so tests are cwd-independent, and validate requiresGpu at manifest load. Reuse the centralized TestIds for the error overlay, error dialog, and templates dialog selectors. Extract the shared suite settings and templates-dialog dismissal into fixtures/utils/customNodeSuite so the three specs cannot drift. Rename spikeDesktop.spec.ts to coreSmoke.spec.ts to match its maintained purpose, document the full manifest schema in the README, and describe the gate outcome without a hardcoded test count.
…checks The breadth sweep now fails on any console error captured during the connect/serialize/prompt loop, matching the fidelity test. The wildcard predicate is exported from typePairing and reused instead of re-derived. assertEntry validates real shapes (non-empty pack/expectedNodes/tiers, arrays, boolean requiresGpu, finite positive timeoutMs); workflow stays allowed as an empty string until a pack gains a run-tier fixture.
knip flags exported types with no external consumers; CustomNodeTier, ObjectInfoNode, NormalizedSlot, and SlotRef are referenced only within their own modules.
…t fidelity, widget round-trip, combo pairing Executes ~340 more nodes and hardens every tier's assertions: - CHAINABLE verdict: required sockets with a model-free producer (EmptyImage, EmptyLatentImage, SolidMask, Primitive*, EmptyAudio) are synthesized and wired automatically; NEEDS_WIRES now means only truly unproducible types (MODEL, SEGS, CONDITIONING...) - auto-run asserts data flow: every PreviewAny sink must emit a ui payload (NO_OUTPUT class); OUTPUT_NODE targets stay event-covered - Vue mount asserts DOM widget/slot counts (missing fails, extras and in-row control_after_generate tolerated) - save/reload is now two passes: pristine (reload must never shrink a node or change a value - the "widgets disappear" bug class) and set-and-stick (every plain widget holds a programmatic non-default write and it survives reload where topology is stable) - connectivity pairs COMBO slots on exact option-vocabulary match (+~120 pairs); mismatched vocabularies stay excluded by design - harness invariants: node ids never reused within a page (the widgetValueStore keys state by node id and survives graph.clear(), so a reused id inherits stale widget values - core bug, reported separately), and run events are filtered by prompt id + graph node id membership so late websocket events or flap-retry double-queues can never pin one node's failure on the next - new mechanism ledgers: WIDGET_SET_ALLOWLIST, ROUNDTRIP_VALUE_ALLOWLIST, MOUNT_WIDGET_ALLOWLIST, all stale-guarded; AUTO_RUN_EXCLUDE gains the observed offenders (rembg pip-install-at-execute, empty-find infinite loop, from_pretrained downloads, minutes-long per-pixel loops) - manifest baselines reconciled against three observation runs; stale entries removed, real failures (missing optional deps, degenerate synthesized inputs, CUDA-only recorders) baselined Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…renderers; reconcile Linux CI flips Mount fidelity now has a renderer-independent bar: under BOTH the LiteGraph and Vue passes, every created instance must materialize everything its def declares - each non-socketless input exists as a widget or a socket (autogrow templates count via their dot-qualified expansion slots, e.g. variables.a/variables.b), and every declared output exists. The Vue pass keeps its extra layer: the DOM must render at least the instance's widget and slot counts. Verified against all 823 nodes under both renderers; the only def-shape special case found was the core autogrow container semantics. Also reconciles the first Linux CI run of the chain-builder tier: - environment flips move to AUTO_RUN_EXCLUDE with mechanisms and leave the baseline: Image Analyze, Text Parse A1111 Embeddings (fail macOS, clean Linux), Image Crop Face (clean macOS, AttributeError Linux), ImageReceiver (av decode error macOS, clean Linux) - run-to-run flip-floppers excluded: ImpactRemoteInt, ImpactSchedulerAdapter, ImpactQueueTriggerCountdown (queue-hook JS transient refusals), LoadText|pysssss (state-dependent file combo) Local verification: allNodes 21/21 twice consecutively, plus lint, format:check, knip, and both typechecks clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-side queue throws; scope decisions written in-code Coverage-gap audit after the mount-fidelity miss (assertions silently narrower than their claim). Fixes: - save/reload now runs under BOTH renderers with staged evaluates and frame yields, so Vue component mount/configure effects actually flush before each serialize - the one renderer-dependent value path a LiteGraph-only pass could not see. Console errors are now collected during the tier too (configure-time pack JS noise was uncovered). - queuePrompt is wrapped in-page: pack JS that THROWS mid-graphToPrompt (VHS applyToGraph crashed CI's whole VHS tier) now classifies as VALIDATION_FAIL carrying the exception text, so the offender self-identifies instead of aborting the tier. VHS_SelectLatest excluded with that mechanism: its applyToGraph assumes downstream inputs have widgets and hard-crashes when its output feeds a pure socket while the input dir has matching files (upstream-report candidate). - pack-owned-value nodes (ROUNDTRIP_VALUE_ALLOWLIST) no longer receive set-and-stick probe writes - writing `_cn` markers into editor JSON widgets just made pack JS choke on our own probes. - deliberate scopes are now stated where the assertion lives: auto-run runs single-renderer because execution is a backend contract and values flow through the same store in both renderers; it deliberately skips the zero-visible-errors check because it provokes expected failures; the connectivity breadth sweep is renderer-independent with the curated drag test covering both renderers; combo vocabulary matching is deliberately order-sensitive (option order defines the default). Local verification: full customNodes suite 67/67 under CI parity, plus lint, format:check, knip, and both typechecks clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pair combos with unknown options A wired combo input bypasses its own widget, so menu order and the options[0] default are not part of the wire contract - membership is (backend validation checks value-in-options). Vocabulary fingerprints are now order-insensitive (sorted, element-wise canonicalized). In the current corpus this changes zero pairs (measured: no same-set, different-order combos exist across the 7 packs); the rule is now correct for packs where they do. Auditing that change surfaced a real hole: the frontend's transformed defs present some combos as the literal string COMBO with options in the opts object. The old fingerprint hashed all of those identically, silently cross-pairing dropdowns with no vocabulary evidence - exactly the checkpoint-into-scheduler class the combo rule exists to exclude. Normalization now pulls V2-form options, and a combo with no known option list is excluded from pairing instead of blind-matched. Plan moves 5,058 -> 5,030 pairs; the 28 removed were vocabulary-blind. Also from CI: MiDaS Mask Image excluded (torch.hub download inside execute hung the Linux runner; runs clean only where the hub cache is warm) and ImageTransformKJ ledgered (pack JS initializes its fill-options JSON widget on configure). Local verification: full customNodes suite 68/68 twice, lint, format, knip, and both typechecks clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ceInput beats every form, census-derived fixtures Systematic audit for siblings of the two combo bug classes (ungrounded contract, shape blindness), driven by a live shape census of the exact getNodeDefs object the suite consumes: - classifyInput now handles the V2 schema form (string 'COMBO' with options in the opts object; 495 such inputs exist in the transformed defs): options present = widget, empty or remote/lazy = NEEDS_MODELS. Real effect measured: 8 KJNodes nodes were silently misclassified NEEDS_WIRES and never executed - 5 now run clean, 3 correctly land in NEEDS_MODELS (remote combos). - forceInput now beats every input form, list-form combos included (a census-found form the old branch order classified as widget; today's 4 instances are optional or non-manifest, so this is protection, not a behavior change). - pure-spec fixtures for both parsers now include every census form, copied from real census examples (V2 options, V2 empty, V2 remote, forceInput-on-combo, cross-form vocabulary pairing) so fixtures can no longer self-confirm the parser's assumptions. - ADDING_CUSTOM_NODES.md gains the evidence rules: independent-oracle grounding for semantic claims, shape-census-driven parsing with exclude-with-record on unknown shapes, and verify-against-the-source- the-code-consumes. defaultInput checked against frontend source: deprecated and ignored (nodeDefStore warning only) - deliberately not handled. Local verification: full customNodes suite 72/72 under CI parity; lint, format:check, knip, and both typechecks clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…he incident behind every invariant The suite had run/onboarding docs but nothing describing the SYSTEM: what the pieces are, how node definitions flow through the planners and classifiers, how the execution harness attributes outcomes, and why each non-obvious rule exists. This adds the missing third doc with four Mermaid box-line diagrams (system overview, def data flow, run pipeline, CI pipeline), the tier-by-renderer coverage matrix, the full ledger table with the two-way baseline semantics, and the hard-won invariants each tied to the incident that forced it (widgetValueStore id bleed, event cross-attribution, pack JS queue-hook crashes, Vue effect timing, queue-jam tripwire). Scope contract is stated up front: compatibility and regression gate, not a behavior certifier. Every path, symbol, and number cross-checked against the tree before commit. README and ADDING_CUSTOM_NODES now cross-reference it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s, gotchas reference Architecture documentation for the custom-node regression suite, written as design views rather than an implementation dump: - eight responsibility-level views: system context, building blocks, the node-definition pipeline, the execution flow, the persistence check, event attribution, the evidence model, and the CI deployment view; every diagram box names a responsibility or concept, arrows carry meaning, and decision points read in plain English - a one-minute What/Why/How opening with the three explicit non-goals (output semantics, frontend-virtual nodes, hour-scale soak) and a clearly labeled scale snapshot so instance numbers never read as properties of the design - a 12-row design-decisions table with honest trade-offs (why a real browser at all, why the backend serves the built frontend, one worker, disabled execution cache, pinned pack versions, one-row extensibility, per-tier renderer policy, mechanism-carrying exceptions, the two-way baseline, batch+bisect, and the scope line), plus the curated-workflow fixture named as the deliberate extension seam - a 14-item gotchas reference, each entry in symptom / root cause / defense / which-team-concern-it-answers form, with named nodes kept only as worked examples of their class - one implementation map section where architecture names meet code symbols, covering every building block including the orchestrator and the evidence ledgers - the workflow's rotted sharding comment fixed (suite duration and the real shard trigger) Grounded on the C4 model's published guidance, reviewed by an independent architect pass (two view-coherence gaps found and fixed) after three earlier Opus review passes on content accuracy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reader feedback on the rendered views: D2's service-to-tier arrows crossed the whole diagram with ambiguous fan-ins (three unattributed "pass/fail + exceptions" curves), and D3's corpus box mixed the two-dialects annotation into a flow node right where three arrows fan out, reading as if the dialects explained the fan-out. Fixes, using the rules that make the CI view work: one direction of flow per diagram, no many-to-many edges (the service-to-tier matrix is now a table, which is what a matrix is), and annotations live in prose rather than inside flow boxes. D2 is now a straight manifest -> orchestrator -> tiers -> evidence -> verdict pipeline with a three-row shared-services table; D3 moves the dialect fact into the normalize step, labels the fan-out "derives", and adds one sentence mapping each derived plan to its consuming tier. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…m D1-D4 Reader feedback on the rendered views: manual line breaks inside boxes force Mermaid to render narrow, tall boxes with heavy wrapping, so the diagrams cost too much scrolling. Mermaid sizes a box to its longest line, so the fix is one or two long lines per box with elaboration in the prose below the diagram. Applied to D1 (context), D2 (pipeline), D3 (definition pipeline), and D4 (execution flow); D7 and D8 stay as approved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e batching box
Headings were double-numbered ("2. D1 - system context") with an
internal diagram-numbering scheme that means nothing to a reader.
Sections are already numbered: headings now just name the view, and
every cross-reference points at a section. Also replaced "queue cost
is amortized" with plain English: one submission carries many nodes
instead of paying the round-trip per node.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The frontend box was a dead end: the suite drove it but nothing flowed back, so the verdicts arrow to the team looked sourceless. Added the return edge (observations back: what mounted, what persisted, what executed, every error) and reworded the team edge so verdicts are visibly the synthesis of those observations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… width The building-blocks pipeline rendered as a tall narrow strip for two mermaid reasons: labels auto-wrap at the ~200px default regardless of line length, and a subgraph's declared direction is ignored once it has external edges, so the tier row silently stacked vertically. The view is now a left-to-right pipeline with the tier group in the middle, and the wrap-width directive makes boxes wide instead of tall here and in the definition-pipeline and execution-flow views. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ctured per review - context diagram flows one way: driver -> frontend -> verdict synthesis -> team - execution flow: classification fans out to its three verdicts; runnable paths converge on batching, blocked routes straight to reconciliation - persistence check: sequence diagram replaced with a linear pipeline (one actor issuing commands is a procedure, not a message exchange) - building blocks: tiers fan 2x2 inside the horizontal pipeline - tripwire step + small recovers? diamond instead of one giant diamond Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…; name the map's ordering principle Dagre's crossing minimizer ignores edge declaration order, so the fix is node declaration order. Also states explicitly that the map is ordered by zoom, not page order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ed stack claims Implements the cold insider review's full finding set plus pipeline verification fixes: - title gains 'regression', matching every sibling doc - wiring tier renderer coverage told truthfully (breadth sweep one, curated drags both); decision 7 enumerates all renderer surfaces - section 5 gains the manifest tiers vocabulary bridge (run and connectivity gate; load and io currently gate nothing) - renderers named once (LiteGraph / Vue Nodes 2.0); opener deduplicated against section 1; implementation map matches the real manifest schema - section 13 names Playwright, bundled Chromium, GitHub Actions, with a caveated runtime ballpark; gotcha receipts carry only verifiable claims - sections 10 and 13 diagrams conform to the doc's diagram grammar - ADDING_CUSTOM_NODES tiers gloss and manifest.ts workflow comment aligned with the same gating reality Reviewed via ninja pipeline: 4-hat CORE panel (2 passes), senior QA gate (2 rounds), gated review (Primary, Double Checker, Ultimate Skeptic). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iminating self-checks - positive prompt-id attribution: capture the /prompt response id as the primary event filter (seen-set + graph membership stay as depth); a new attribution self-check injects a foreign-prompt terminal error mid-run and proves it cannot fail the run - console collection now includes pageerror (uncaught exceptions and rejections), with a collector self-check as positive control; surfaces a real Custom-Scripts betterCombos typeof-null bug, ledgered with mechanism - connectivity allowlists are two-way stale-guarded: every entry must be observed failing in its recorded way, all stale keys reported per run - manifest pins are required full 40-hex SHAs (CUSTOM_NODES_ALLOW_UNPINNED=1 admits only empty pins, reserved for the planned pack-HEAD canary); pack must be a plain path segment; contract pinned by pure specs - CI installs each pack under custom_nodes/<pack> with charset and pin gates before cloning (attribution keys on the install dirname) - allNodes renderer loops honor rendererPassesFor (vueNodesCompatible) - curated T1 asserts every display sink emitted a ui payload; console sinks documented as excluded (no ui payload by design) - the always()-wrapper suggestion was rejected on sibling evidence: ci-tests-unit.yaml gates its required check with a changes job and job-level if, and no repo workflow uses a wrapper Reviewed via ninja pipeline: 4-hat CORE panel (2 passes), senior QA gate (2 rounds, discrimination proven by falsification), gated review (Primary, Double Checker, Ultimate Skeptic - 15-entry evidence ledger, all PROVEN). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dosubot
Bot
added
size:XXL
…uth pass from round-2 review - Extract the pack console-error allowlist into a shared fixture (consoleErrorLedger.ts); the curated T1 run now collects console and page errors across load+run and asserts them through the ledger. The filter is pinned by a discriminating pure spec (pattern match, cross-pack ownership, unknown-pack fail-open). - T1 asserts every expectedNodes type is actually present in the curated workflow before running it, killing the vacuous-green path where a drifted fixture shrank the executed-set check to an empty id list. - typePairing records unrecognizable slot specs (unknownSlots on the node, unknownShapes on the plan) instead of silently dropping them; connectivity logs the list; pure tests pin the input/output drop paths and the socketless boundary. - Add test:custom-nodes:ci, the gate-equivalent run against the backend-served built frontend; README re-scopes test:custom-nodes as the dev-server loop that is NOT the gate, and scopes the zero-visible-errors invariant to the tiers that hold it. - ARCHITECTURE truth pass: event attribution leads with the positive prompt-id capture; section 10 grades ledger guards in three strengths; the decentralized parser story (declaredShape, classifyInput, normalizer) is stated consistently in section 4, gotcha G5, and the legend; ADDING_CUSTOM_NODES points the console ledger at its new home. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… onboarding mirrors the scoped invariant Round-3 review follow-ups: the definition-pipeline diagram no longer implies a centralized normalizer (live census -> wiring slot normalizer / execution classifier / mount declared-shape parser, matching section 4), and ADDING_CUSTOM_NODES scopes the zero-visible-errors claim to the tiers that assert it, same wording as the README. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…exercise found
- Wiring drop resolution: the curated drag test only targeted first-slot
inputs, so a slot hit-test regression that falls back to the first
compatible input went undetected. Add a second-slot anchor
(EmptyImage.IMAGE -> ImageBatch.image2) that only links if the drop
resolves the exact slot; proven by breaking getNodeInputOnPos.
- Curated-run failure naming: a backend validation rejection answers
/prompt with node_errors but app.queuePrompt swallows it, so a
VALIDATION_FAIL reported {}. Capture and flatten the node_errors
(summarizePromptError, typed off apiSchema PromptResponse) into the
result's clientError and surface it in the T1 message, so a red names
the node and input. Exported with a pure unit test since the happy path
never runs it.
- Console-error window: document (README + ARCHITECTURE section 10) that
the ledger collects per-tier, so boot-time pack console noise before the
first tier action is out of scope by design, backstopped by the startup
zero-visible-errors check.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rrelate to real regressions) The correlation matrix + throwaway-PR plan that proves the suite catches every failure mode ARCHITECTURE.md claims: one deliberate break per surface, each citing the real historical regression it recreates (Linear Custom Node Bugs issues + FE PR #12279) and the exact CI red it produces. Every "exact red" is captured from a real falsification run, not a prediction. Renames the earlier "kill-test" work to the falsification pass. States the honest local-full-run idempotency caveat (CI shards per-pack; a single-backend serial run is not the oracle). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…erson reference) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n-noise filter, slow-node budgets) The suite runs all 7 packs' execution tiers against one shared backend locally (CI shards one backend per pack). Serial execution created three distinct cross-test contaminations that failed a different set of packs each run; each is now fixed at its mechanism: - Foreign execution noise: mount/persistence/wiring/T0/core-smoke tiers queue no prompts, yet caught a prior tier's async execution error (PromptExecutionError, a 400 on /api/prompt). isForeignExecutionNoise filters execution-domain console lines from the non-executing tiers only; the executing tiers still assert them. Same "not this test's evidence" principle as event attribution (ARCHITECTURE section 9). - Queue contention: the auto-run queue-busy guard hard-failed when a prior pack's slow CPU execution was still draining. drainUntilIdle waits it out (interrupt + clear + poll, throw-on-error so a failed read counts as busy); only a genuinely wedged backend fails. runBatch's post-timeout drain grows from 5s to 90s for the same reason. - Slow-under-load misread as a regression: the single-node disambiguation re-run gets 60s instead of the batch's 20s. A real hang still exceeds it. Also excludes the CLIPSeg model loaders (essentials, WAS) - model-download nodes, same non-interruptible class as the listed BLIP/SAM/MiDaS loaders. Reviewed by four-hat CORE (ship it); the new predicate is unit-pinned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… to idle Every test gets a fresh page but all tests share one backend, locally and on CI alike (the CI job is deliberately unsharded). A test ending while its prompt still executed left that work running, and the next test's fresh page connected mid-execution and inherited its async errors or its busy queue. Drain the backend to idle in an afterEach in all four backend-running specs, while the finishing test's own page is still open so late events land there: no test can affect the next. The drain helper moves to the shared fixture util (drainBackendToIdle, byte-identical body); the auto-run tier's queue guard and runBatch post-timeout drain rewire to it with their explicit budgets. The hooks use a 10s budget: a no-op when already idle, and a backend still busy past it is wedged, which the auto-run tier's 150s guard surfaces with the restart diagnostic. DETECTION_PROOF.md's caveat is rewritten to match, and a false claim that CI shards one backend per pack is corrected in every location (code comments and doc): the CI job runs the whole suite against one fresh backend on an unloaded runner, which is why executions stay inside their budgets there. Empirical: a full-suite run with the hook eliminated the cross-test bleed class entirely (zero mount/save-reload/core-smoke console or overlay failures, previously 3-5 per run). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nation audit Every quoted red message re-verified against its original run log (10/10 match). Seven citation corrections from the audit: the iTools missing- button tickets are Nodes 2.0 regressions and move to the v2 mount row (the v1 row is now honestly class-only); the SAM3 hidden-values tickets are removed (extras-exposed class, which the mount tier tolerates by design, so citing them overclaimed coverage); the persistence row now cites the verified defaultInput migration regression (widgets reverting to socket-only on reload) that open PR #12279 fixes, instead of a live widget-interaction ticket; the links-type and serialization rows drop borrowed tickets and state their class plainly; the drag row's tickets are labeled nearest-symptom family; the two expansion bullets now cite the committed pure-spec catches instead of an unproven live-sweep catch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pack-mode breaks are delivered via manifest pin swaps (CI clones packs at pins, so in-repo pack-file edits cannot reach it); corpus-derived red messages promise the tier and failure class, not byte-identical offender text; remaining citations and mechanism descriptions tightened to what the code and captured runs actually show. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… and all-live commit design
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
2 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.
Summary
Playwright suite proving community custom-node packs register, render under both renderers (LiteGraph canvas and Vue Nodes 2.0), wire type-correct connections, and execute real workflows, with every browser test asserting the app shows zero visible errors. Manifest-driven: 7 packs covered (Impact, VideoHelperSuite, rgthree, essentials, KJNodes, Custom-Scripts, WAS), and adding another is one JSON row plus one small workflow file. A gating CI job installs every manifest pack from scratch (pinned to verified commits) and fails on any test failure, any pack-install error, or any skipped test.
Beyond the curated per-pack tiers, the suite covers EVERY node the packs register - 823 nodes total, discovered live from the backend with zero configuration: each mounts in both renderers (the Vue mount also asserts the DOM renders the instance's widget and slot counts), survives save/reload twice (a pristine pass where reload must never shrink a node or change a value - the "widgets disappear after reload" bug class - and a set-and-stick pass where every plain widget must hold a programmatic non-default write), has its concrete slots wired through the typed-connection sweep (5,058 pairs; COMBO slots pair when their option vocabularies match exactly), and executes for real on the backend (446 nodes run clean): self-sufficient nodes run on their widget defaults, and nodes whose required sockets have model-free producers (EmptyImage, EmptyLatentImage, SolidMask, Primitive*, EmptyAudio) get those producers synthesized and wired automatically. Executed nodes must observably produce - the PreviewAny sink wired to each node must emit a ui payload. Nodes that cannot run are classified and reconciled against a committed per-pack baseline that is asserted both ways, so it can neither hide regressions nor rot. Every escape hatch is a reviewed ledger entry carrying its mechanism (details in
ADDING_CUSTOM_NODES.md).The all-nodes sweep also surfaced real bugs, worked around in the harness and queued for upstream reports: the frontend widgetValueStore keeps widget state keyed by node id across
graph.clear(), so a recreated node that reuses an id inherits a deleted node's same-named widget values (the suite never reuses ids); WASText Find and Replace Inputinfinite-loops the server on an empty find string; WASImage Rembgrunspip installinside execute; VHSper_batchserializes to null after reload; KJNodesImageGridtoBatchships a default that violates its own minimum.Changes
browser_tests/tests/customNodes/suite +browser_tests/fixtures/customNode/harness (in-page event tap, pure run classifier, object_info validator, manifest loader, type-pairing generator). Tiers per pack: T0 load/render (per-nodedata-node-idassertion under Vue Nodes), connectivity (one representative typed edge per slot planned from/object_info, connected through the realisValidConnectionveto, survives serialize/configure/graphToPrompt, plus curated real slot drags under both renderers), T1 run (per-node execution verified via theexecutingevent stream). Zero-visible-errors invariant across error overlay/dialog/node errors/toasts, with a self-check that forces an execution error and asserts the overlay appears (positive control).vueNodesCompatiblemanifest field: a pack proven unable to mount under Vue Nodes 2.0 runs its LiteGraph assertions only. Never atest.skip, so the zero-skip CI gate stays honest. No current pack needs it (all 7 mount under Vue Nodes empirically); the mechanism is unit-tested and its evidence rule is documented..github/workflows/ci-tests-custom-nodes.yaml: gating job that clones and pip-installs every manifest pack (CPU torch constraints), boots the backend, runs the suite, and forbids skipped tests.browser_tests/tests/customNodes/ADDING_CUSTOM_NODES.md: the authoritative step-by-step process for onboarding a new pack (verify real node keys, author the workflow, manifest field reference, local verify, CI expectations), followable by humans and agents.ComfyPage.createUseris now idempotent on400 Duplicate username.so the fixture works against single-user server-storage backends.playwright.chrome.config.ts: opt-in system-Chrome config (trace off; the trace recorder crashes pages under the branded Chrome channel).New Scripts Added
High level (full command table with every variant lives in the committed suite README,
browser_tests/tests/customNodes/README.md):pnpm test:custom-nodesruns the whole suite headless. This is the pass/fail gate: every tier for every pack, zero skips.-gpattern works against the generic scripts for ad-hoc filtering.Prerequisites (backend flags, dev server) and a worked example are in the same README; the process for adding new packs and their scripts is in
ADDING_CUSTOM_NODES.md.Review Focus
runResult.ts: only non-nullexecutingevents count as executed (cache-safe); the run tier therefore requires a--cache-nonebackend, documented in the README and manifest field docs.custom-nodes-e2e). Path gating lives in achangesjob, not trigger-levelpaths:, so the required check never wedges Pending on unrelated PRs. Fork PRs skip it (the manifest install loop is a code-execution surface) and keep coverage via the main e2e shards.ComfyPage.createUser(blast radius on existing suites is nil: the 200 path is unchanged).litegraphUtils.tsNodeSlotReference.getPosition: slot positions now add the canvas element's page offset so mouse targeting survives pack JS that injects page chrome (rgthree's progress bar shifts the canvas 16px). Exact no-op when the canvas sits at (0,0), which is every stock environment - verified by the full suite passing against both the dev server and a dist-serving backend.Local verification: 67 passed / 0 skipped / 0 failed against ComfyUI v0.26.2 (CPU,
--multi-user --cache-none) with all 7 manifest packs installed at their pinned commits; lint, format, knip, and both typechecks clean under the CI-mirror gate.