fix: make ComfyApp root-graph readiness reactive - #14976
Open
mattmillerai wants to merge 8 commits into
Open
Conversation
Back the root graph with a shallowRef so watchers, computeds and useEventListener sources that read app.graph / app.rootGraph / app.isGraphReady re-evaluate when setup() assigns the graph. Stores constructed before ComfyApp.setup() runs (FirstRunTour pulls in appModeStore on every cold start) previously bound to a plain field with no reactive dependency, so a listener registered against app.rootGraph?.events never rebound once the graph existed.
mattmillerai
marked this pull request as ready for review
August 9, 2026 19:10
dosubot
Bot
added
the
size:S
🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ✅ 1816 passed, 0 failed · 2 flaky📊 Browser Reports
📦 Bundle: 8.85 MB gzip 🟢 -1 BDetailsSummary
Category Glance App Entry Points — 3.71 kB (baseline 3.71 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.37 MB (baseline 1.37 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 2 added / 2 removed / 1 unchanged Views & Navigation — 124 kB (baseline 124 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 13 added / 13 removed / 4 unchanged Panels & Settings — 565 kB (baseline 565 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed / 16 unchanged User & Accounts — 27.7 kB (baseline 27.7 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed / 5 unchanged Editors & Dialogs — 125 kB (baseline 125 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 7 added / 7 removed / 1 unchanged UI Components — 67.1 kB (baseline 67.1 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed / 8 unchanged Data & Services — 3.52 MB (baseline 3.52 MB) • 🔴 +95 BStores, services, APIs, and repositories
Status: 14 added / 14 removed / 3 unchanged Utilities & Hooks — 550 kB (baseline 550 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 18 added / 18 removed / 20 unchanged Vendor & Third-Party — 16.8 MB (baseline 16.8 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 18 unchanged Other — 14.2 MB (baseline 14.2 MB) • ⚪ 0 BBundles that do not match a named category
Status: 68 added / 68 removed / 217 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-08-16T06:48:41.138Z",
"gitSha": "81cb9fc0027caa410a3c2d70403209dd04e228c8",
"branch": "matt/fe-6860-root-graph-shallow-ref",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2031.4480000000117,
"styleRecalcs": 7,
"styleRecalcDurationMs": 6.678,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 575.373,
"heapDeltaBytes": 5019224,
"heapUsedBytes": 69731040,
"domNodes": 14,
"jsHeapTotalBytes": 24641536,
"scriptDurationMs": 9.088,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-idle",
"durationMs": 2036.2400000000207,
"styleRecalcs": 8,
"styleRecalcDurationMs": 8.441,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 605.46,
"heapDeltaBytes": 5019456,
"heapUsedBytes": 69679708,
"domNodes": 16,
"jsHeapTotalBytes": 24641536,
"scriptDurationMs": 12.386000000000001,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1872.9700000000093,
"styleRecalcs": 74,
"styleRecalcDurationMs": 39.053,
"layouts": 12,
"layoutDurationMs": 3.652,
"taskDurationMs": 902.1189999999999,
"heapDeltaBytes": -17040652,
"heapUsedBytes": 47651752,
"domNodes": -282,
"jsHeapTotalBytes": 23040000,
"scriptDurationMs": 118.221,
"eventListeners": -158,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1837.0409999999993,
"styleRecalcs": 73,
"styleRecalcDurationMs": 38.313,
"layouts": 12,
"layoutDurationMs": 3.6560000000000006,
"taskDurationMs": 894.3420000000001,
"heapDeltaBytes": -237384,
"heapUsedBytes": 64567328,
"domNodes": 55,
"jsHeapTotalBytes": 25427968,
"scriptDurationMs": 120.28499999999998,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1772.086999999999,
"styleRecalcs": 32,
"styleRecalcDurationMs": 18.409000000000002,
"layouts": 6,
"layoutDurationMs": 0.6739999999999999,
"taskDurationMs": 388.66200000000003,
"heapDeltaBytes": 7954800,
"heapUsedBytes": 72637864,
"domNodes": 78,
"jsHeapTotalBytes": 24379392,
"scriptDurationMs": 10.458999999999998,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1754.560999999967,
"styleRecalcs": 31,
"styleRecalcDurationMs": 19.167000000000005,
"layouts": 6,
"layoutDurationMs": 0.593,
"taskDurationMs": 427.05800000000005,
"heapDeltaBytes": 7957420,
"heapUsedBytes": 72710128,
"domNodes": 77,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 13.3,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 555.7459999999992,
"styleRecalcs": 11,
"styleRecalcDurationMs": 7.084999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 367.142,
"heapDeltaBytes": -11564540,
"heapUsedBytes": 53170400,
"domNodes": 18,
"jsHeapTotalBytes": 25427968,
"scriptDurationMs": 54.70099999999999,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 603.5960000000387,
"styleRecalcs": 10,
"styleRecalcDurationMs": 7.096,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 394.747,
"heapDeltaBytes": -11202144,
"heapUsedBytes": 53629740,
"domNodes": 16,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 60.058,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2037.2959999999694,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.939000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 582.152,
"heapDeltaBytes": 12712676,
"heapUsedBytes": 72169644,
"domNodes": -282,
"jsHeapTotalBytes": 2195456,
"scriptDurationMs": 15.341000000000005,
"eventListeners": -154,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-idle",
"durationMs": 2042.4460000000408,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.340000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 710.804,
"heapDeltaBytes": 11390388,
"heapUsedBytes": 71096284,
"domNodes": -282,
"jsHeapTotalBytes": 3244032,
"scriptDurationMs": 18.522999999999996,
"eventListeners": -184,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2136.1700000000496,
"styleRecalcs": 68,
"styleRecalcDurationMs": 12.995999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1171.972,
"heapDeltaBytes": 10302704,
"heapUsedBytes": 71537232,
"domNodes": -286,
"jsHeapTotalBytes": 3436544,
"scriptDurationMs": 325.00800000000004,
"eventListeners": -152,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2231.706000000031,
"styleRecalcs": 68,
"styleRecalcDurationMs": 14.187000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1261.344,
"heapDeltaBytes": -797640,
"heapUsedBytes": 60458348,
"domNodes": -287,
"jsHeapTotalBytes": 3698688,
"scriptDurationMs": 339.35,
"eventListeners": -184,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-zoom",
"durationMs": 3198.5599999999863,
"styleRecalcs": 65,
"styleRecalcDurationMs": 13.984,
"layouts": 60,
"layoutDurationMs": 7.1579999999999995,
"taskDurationMs": 1376.8960000000002,
"heapDeltaBytes": -4326632,
"heapUsedBytes": 58313848,
"domNodes": -288,
"jsHeapTotalBytes": 4292608,
"scriptDurationMs": 385.533,
"eventListeners": -158,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3217.3119999999926,
"styleRecalcs": 64,
"styleRecalcDurationMs": 14.328999999999999,
"layouts": 60,
"layoutDurationMs": 7.431,
"taskDurationMs": 1382.307,
"heapDeltaBytes": 16863576,
"heapUsedBytes": 79126936,
"domNodes": 10,
"jsHeapTotalBytes": 4718592,
"scriptDurationMs": 388.863,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2029.2460000000005,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.932999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 610.353,
"heapDeltaBytes": 13112052,
"heapUsedBytes": 73736624,
"domNodes": -285,
"jsHeapTotalBytes": 3244032,
"scriptDurationMs": 14.703000000000001,
"eventListeners": -154,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2014.8629999999912,
"styleRecalcs": 7,
"styleRecalcDurationMs": 6.733000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 716.9540000000001,
"heapDeltaBytes": 8559436,
"heapUsedBytes": 69525032,
"domNodes": -286,
"jsHeapTotalBytes": 3506176,
"scriptDurationMs": 18.375000000000004,
"eventListeners": -184,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 611.0059999999748,
"styleRecalcs": 46,
"styleRecalcDurationMs": 10.262000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 417.479,
"heapDeltaBytes": -10399084,
"heapUsedBytes": 54456516,
"domNodes": 18,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 122.05799999999999,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 644.504999999981,
"styleRecalcs": 47,
"styleRecalcDurationMs": 11.934000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 430.07599999999996,
"heapDeltaBytes": -10726752,
"heapUsedBytes": 54294640,
"domNodes": 20,
"jsHeapTotalBytes": 25690112,
"scriptDurationMs": 121.75599999999999,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-idle",
"durationMs": 2006.21000000001,
"styleRecalcs": 10,
"styleRecalcDurationMs": 8.784000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 437.01500000000004,
"heapDeltaBytes": 5209212,
"heapUsedBytes": 69993684,
"domNodes": 20,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 6.155999999999998,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "subgraph-idle",
"durationMs": 2004.8949999999195,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.929999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 542.546,
"heapDeltaBytes": 4871068,
"heapUsedBytes": 69650248,
"domNodes": 18,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 9.86,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1730.6459999999788,
"styleRecalcs": 76,
"styleRecalcDurationMs": 36.90599999999999,
"layouts": 16,
"layoutDurationMs": 4.186,
"taskDurationMs": 740.95,
"heapDeltaBytes": -3849656,
"heapUsedBytes": 60962104,
"domNodes": 64,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 80.839,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1722.846000000004,
"styleRecalcs": 77,
"styleRecalcDurationMs": 40.349000000000004,
"layouts": 16,
"layoutDurationMs": 4.692,
"taskDurationMs": 837.4639999999999,
"heapDeltaBytes": -4068396,
"heapUsedBytes": 60704292,
"domNodes": 64,
"jsHeapTotalBytes": 25427968,
"scriptDurationMs": 92.80300000000001,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-transition-enter",
"durationMs": 1431.8619999999669,
"styleRecalcs": 20,
"styleRecalcDurationMs": 32.457,
"layouts": 15,
"layoutDurationMs": 13.544999999999998,
"taskDurationMs": 1057.2910000000002,
"heapDeltaBytes": -446480,
"heapUsedBytes": 87347040,
"domNodes": 13673,
"jsHeapTotalBytes": 9699328,
"scriptDurationMs": 20.211999999999993,
"eventListeners": 2375,
"totalBlockingTimeMs": 142,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 8194.389000000001,
"styleRecalcs": 251,
"styleRecalcDurationMs": 37.13999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4133.659,
"heapDeltaBytes": 14800568,
"heapUsedBytes": 76262968,
"domNodes": -281,
"jsHeapTotalBytes": 4747264,
"scriptDurationMs": 1023.2719999999999,
"eventListeners": -138,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 9180.79399999999,
"styleRecalcs": 249,
"styleRecalcDurationMs": 38.111,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4719.7300000000005,
"heapDeltaBytes": 13384668,
"heapUsedBytes": 74101812,
"domNodes": -282,
"jsHeapTotalBytes": 4222976,
"scriptDurationMs": 1210.6729999999998,
"eventListeners": -134,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 17062.471000000016,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16472.562,
"heapDeltaBytes": -39947428,
"heapUsedBytes": 170607660,
"domNodes": -8313,
"jsHeapTotalBytes": -10166272,
"scriptDurationMs": 116.89199999999998,
"eventListeners": -16394,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.780000000000047,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 17610.35299999992,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16626.979,
"heapDeltaBytes": -40052644,
"heapUsedBytes": 170094716,
"domNodes": -8313,
"jsHeapTotalBytes": -6496256,
"scriptDurationMs": 125.233,
"eventListeners": -16394,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 20570.769999999982,
"styleRecalcs": 177,
"styleRecalcDurationMs": 18.303999999999988,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 19936.860000000004,
"heapDeltaBytes": -33455740,
"heapUsedBytes": 163954676,
"domNodes": -8313,
"jsHeapTotalBytes": -11440128,
"scriptDurationMs": 400.15600000000006,
"eventListeners": -16386,
"totalBlockingTimeMs": 16,
"frameDurationMs": 17.776666666666763,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 20688.222999999994,
"styleRecalcs": 178,
"styleRecalcDurationMs": 17.41900000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 20070.660999999996,
"heapDeltaBytes": -14888912,
"heapUsedBytes": 182572816,
"domNodes": -8313,
"jsHeapTotalBytes": -10727424,
"scriptDurationMs": 392.54100000000005,
"eventListeners": -16388,
"totalBlockingTimeMs": 2,
"frameDurationMs": 17.780000000000047,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 463.99500000001126,
"styleRecalcs": 15,
"styleRecalcDurationMs": 23.601000000000003,
"layouts": 3,
"layoutDurationMs": 1.2469999999999999,
"taskDurationMs": 122.342,
"heapDeltaBytes": 4993424,
"heapUsedBytes": 68573232,
"domNodes": 128,
"jsHeapTotalBytes": 5242880,
"scriptDurationMs": 8.902999999999999,
"eventListeners": 99,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 473.2539999999972,
"styleRecalcs": 13,
"styleRecalcDurationMs": 20.498999999999995,
"layouts": 3,
"layoutDurationMs": 0.694,
"taskDurationMs": 112.89500000000001,
"heapDeltaBytes": 4927208,
"heapUsedBytes": 68663256,
"domNodes": 123,
"jsHeapTotalBytes": 4980736,
"scriptDurationMs": 7.369000000000001,
"eventListeners": 97,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
}
]
} |
mattmillerai
added
the
cursor-review
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: 0 reviews are currently available. Based on recent review activity, included reviews refill at 1 per hour. 📝 WalkthroughWalkthrough
ChangesRoot graph reactivity
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized change makes root-graph readiness reactive while preserving existing getter behavior and has passing checks and targeted tests; no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant ComfyApp
participant rootGraphRef
participant LGraph
participant Tests
ComfyApp->>rootGraphRef: assign root graph
rootGraphRef-->>ComfyApp: update readiness state
Tests->>ComfyApp: observe isGraphReady
Tests->>LGraph: register configured listener
LGraph-->>Tests: emit configured event
Tests->>rootGraphRef: replace root graph
rootGraphRef-->>LGraph: rebind configured listener
Possibly related PRs
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: 1
🤖 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 `@src/scripts/rootGraphReadiness.test.ts`:
- Around line 42-53: Update the test around useEventListener to first assign an
initial LGraph, then replace it with a second graph through setRootGraph;
dispatch configured on both graphs and assert onConfigured is called only once
for the replacement graph, covering rebinding after rootGraphRef changes.
🪄 Autofix
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: 77388b8c-0cea-4ca0-9d50-a9af0993c32e
📒 Files selected for processing (6)
src/extensions/core/customWidgets.clone.test.tssrc/extensions/core/customWidgets.subgraphPromotion.test.tssrc/scripts/__tests__/appTestUtils.tssrc/scripts/app.test.tssrc/scripts/app.tssrc/scripts/rootGraphReadiness.test.ts
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Every reviewer in the matrix failed to contribute — see the panel summary for which cells errored, and the run logs for the underlying cause.
Panel: 0/8 reviewers contributed findings.
Reviewers that did not contribute: claude-opus-5-thinking-max:adversarial (error), gemini-3.1-pro:adversarial (error), gpt-5.6-sol-max:adversarial (error), kimi-k3-max:adversarial (error), claude-opus-5-thinking-max:edge-case (error), gemini-3.1-pro:edge-case (error), gpt-5.6-sol-max:edge-case (error), kimi-k3-max:edge-case (error)
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #14976 +/- ##
==========================================
- Coverage 81.32% 81.32% -0.01%
==========================================
Files 1882 1883 +1
Lines 107504 107511 +7
Branches 33840 33369 -471
==========================================
+ Hits 87426 87431 +5
+ Misses 19715 19707 -8
- Partials 363 373 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Contributor
Author
|
@coderabbitai review |
Contributor
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/scripts/rootGraphReadiness.test.ts (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the prescribed isolated Pinia setup, or remove it.
This test creates a real global Pinia instance but does not access a store directly. If
apprequires Pinia indirectly, usecreateTestingPinia. Otherwise, remove the Pinia import andsetActivePiniacall. Verify the indirect dependency before changing the setup.As per path instructions, “Use createTestingPinia for Pinia-based tests, reset mocks between tests, and isolate shared state.”
Also applies to: 14-15
🤖 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 `@src/scripts/rootGraphReadiness.test.ts` at line 2, Update the Pinia setup in the rootGraphReadiness test: verify whether app indirectly requires Pinia, and use createTestingPinia with isolated shared state if it does; otherwise remove the Pinia import and setActivePinia call. Preserve test behavior and reset mocks between tests.Source: Path instructions
🤖 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.
Outside diff comments:
In `@src/scripts/rootGraphReadiness.test.ts`:
- Line 2: Update the Pinia setup in the rootGraphReadiness test: verify whether
app indirectly requires Pinia, and use createTestingPinia with isolated shared
state if it does; otherwise remove the Pinia import and setActivePinia call.
Preserve test behavior and reset mocks between tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 29d8b3b6-f7bb-4a43-8f32-f4946850d340
📒 Files selected for processing (1)
src/scripts/rootGraphReadiness.test.ts
christian-byrne
previously approved these changes
Aug 10, 2026
The suite never reaches a store, so the global Pinia instance was dead setup. Verified the three cases still pass without it.
mattmillerai
dismissed stale reviews from christian-byrne and coderabbitai[bot]
via
August 10, 2026 05:07
d94c094
Contributor
Author
|
Pushed one small commit after approval ( @christian-byrne's two nits are both deferred to follow-up tickets rather than fixed here (replies on each thread): the getter type guard can't be behavior-preserving across 319 call sites and needs a throw-vs-widen decision first, and the store-ownership question is blocked on the |
Contributor
Author
|
🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:
The following carry
|
Contributor
Author
|
Re-requesting review only because the branch-protection rule auto-dismissed your approval when Re-verified at Your two nits stay deferred to BE-6887 / BE-6888 as noted on those threads; I've left both threads for you to resolve as promised there. |
Adding shallowRef left app.ts as the only file in the repo with two value imports from 'vue'. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Reviews-loop pass at Fixed: Merged-result check. Merged Cursor review panel never actually ran. The 0/8 "every reviewer errored" result above is not a code signal — all eight cells failed with the same upstream error:
Re-applying the Not touched: the two open threads are @christian-byrne's, deferred to BE-6887 / BE-6888 with replies in place. Per Only remaining blocker is the dismissed approval; re-review is already requested. |
Contributor
Author
|
Reviews-loop pass at Re-verified since the last pass:
The two open threads remain @christian-byrne's, deferred to BE-6887 / BE-6888 with replies in place and left for you to resolve. Only remaining blocker is the approval that branch protection auto-dismissed on |
…ph-shallow-ref # Conflicts: # src/scripts/app.test.ts
Contributor
Author
|
Rebased onto latest main to resolve the merge conflict; all CI checks are green. Ready for re-review (previous approval was dismissed by the new commits). |
This was referenced Aug 13, 2026
| type AppWithRootGraphRef = { rootGraphRef: ShallowRef<LGraph | undefined> } | ||
|
|
||
| const rootGraphRefOf = (app: ComfyApp) => | ||
| (app as unknown as AppWithRootGraphRef).rootGraphRef |
Contributor
There was a problem hiding this comment.
Can we do this without the type assertions by chance?
Contributor
Author
There was a problem hiding this comment.
Yes — dropped both assertions in 909788e0.
TypeScript sanctions element access as the escape hatch for a private member, so app['rootGraphRef'] reaches the same storage with no cast and no hand-written shadow interface. The seam is now two lines:
export function setRootGraph(app: ComfyApp, graph: LGraph | undefined) {
app['rootGraphRef'].value = graph
}The old as unknown as AppWithRootGraphRef was the worse form of the two problems: it did not just bypass privacy, it re-declared the field's type by hand, so a change to rootGraphRef in app.ts would have gone unnoticed here. Element access keeps the real ShallowRef<LGraph | undefined>.
Verified the type actually survives rather than degrading to any — assigned a number to .value and pnpm typecheck rejected it (Type 'number' is not assignable to type 'LGraph'), then reverted. pnpm typecheck, pnpm lint, pnpm knip, pnpm format all clean; rootGraphReadiness and both customWidgets suites green.
Element access reaches a TypeScript `private` member without an assertion, so the seam keeps the real `ShallowRef<LGraph | undefined>` type instead of laundering it through a hand-written shadow interface.
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
github-actions
Bot
added
the
risk:R3
Contributor
|
CI is failing because #14900 added three Replacing those three calls with |
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.
ELI-5
app.rootGraphwas stored in a plain field, so nothing could "watch" for the graph showing up. Anything that asked "is the graph ready yet?" before startup finished got told "no" once and was never told otherwise. Putting the graph in ashallowRefmakes that answer live, so watchers wake up when startup assigns it.Summary
Back
ComfyApp's root graph with ashallowRefso graph readiness is observable, without changing any getter's signature or behavior.Changes
src/scripts/app.tsswaps the plainrootGraphInternalfield forprivate readonly rootGraphRef = shallowRef<LGraph | undefined>(undefined).get graph(),get rootGraph()andget isGraphReady()readrootGraphRef.value— same signatures, same non-null assertions, sameconsole.error('ComfyApp graph accessed before initialization'). The single production writer (setup()) assignsthis.rootGraphRef.value = graph. No consumer call sites changed.rootGraphInternalno longer exists as a runtime own-property. It was TS-privateand undocumented, so this only affects code that deliberately reached past the getters — inside this repo that was three test files, all updated here.src/scripts/rootGraphReadiness.test.tscovers the two behaviors the plain field could not provide.Review Focus
Why
shallowRefand notref.refwould deep-proxyLGraph; the getters must keep returning the raw graph so litegraph identity checks and hot paths are untouched.shallowRefonly tracks replacement of.value, and.valueis written exactly once persetup(), so the reactive cost is a single false→true invalidation per cold start. Several computeds gated onisGraphReady(executionErrorStore,missingNodesErrorStore,RightSidePanel,useProcessedWidgets) will now legitimately recompute on that transition; their suites are green unchanged.Judgment call — the ticket said "app.ts only", but 32 references lived in tests.
app.test.ts(20),customWidgets.subgraphPromotion.test.ts(8) andcustomWidgets.clone.test.ts(4) wrote the private field directly (Reflect.set(app, 'rootGraphInternal', graph)), so leaving the name behind would have meant keeping a dead alias purely as a test seam. Instead those sites go through a typed seam,src/scripts/__tests__/appTestUtils.ts(setRootGraph/getRootGraph), which writes the samerootGraphRef.valuethatsetup()writes. Making the seam typed surfaced twoapp.test.tssites that had been passing a partial graph mock through an untypedReflect.set; those now usefromPartial<LGraph>(the pattern already used inappModeStore.test.ts).git grep rootGraphInternalis now empty repo-wide.Why the test does not call
setup().setup()needs fourdocument.getElementByIdbody slots, a real canvas fornew LGraphCanvas, workflow sync and extension loading; no test in the repo invokes it. The test therefore uses the seam above, which executes the identical statement (rootGraphRef.value = graph). Both cases were verified red: replacing theshallowRefwith a plain{ value }holder fails both (expected [false] to equal [false, true], and theconfiguredspy called 0 times).The listener case registers
useEventListener(() => app.rootGraph?.events, 'configured', spy)inside an effect scope while the graph is unassigned, matchingappModeStore.ts:225-229verbatim — that binding is dead on every cold start today, currently masked becauseafterLoadNewGraphalso callsloadSelectionsdirectly. Note thatappModeStore.test.tsmocks@/scripts/appwith a plain object and so proves nothing about this; the new test uses the realComfyAppsingleton and restores its previous graph inafterEach.Coordination: no semantic conflict with the open PR that swaps call-site guards to
isGraphReady— that one touches consumers and oneapp.tsline insideclear(), this one touches only the field's storage. Either can land first; the rebase is mechanical.Verified:
pnpm typecheck,pnpm lint,pnpm format:check,pnpm knipall clean. Suites run:app.test.ts,rootGraphReadiness.test.ts, bothcustomWidgetsfiles,appModeStore,executionErrorStore,missingNodesErrorStore,useProcessedWidgets,useErrorClearingHooks(+ promotion),useGraphNodeManager,useErrorGroups,ErrorOverlay,useErrorOverlayState,missingMediaPreviewRegression— 335 tests, all green. One run ofapp.test.tsshowed a 5s timeout in an unrelated drag-and-drop test; it did not reproduce across 6 further runs on this branch or 6 runs onmain, so it is pre-existing load flake, not this change.