Merged
Conversation
dosubot
bot
added
the
size:M
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors widget resolution in AppBuilder by adding a resolveNodeWidget helper to locate nodes and optionally promoted widgets within subgraphs, updates selection and bounding logic to use its results, and changes LinearControls to always clear widget slotMetadata and set widget nodeId. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🎭 Playwright: ✅ 560 passed, 0 failed📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
📦 Bundle: 4.57 MB gzip 🔴 +115 BDetailsSummary
Category Glance App Entry Points — 28.9 kB (baseline 28.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 967 kB (baseline 967 kB) • 🔴 +568 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.4 kB (baseline 72.4 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
User & Accounts — 16.1 kB (baseline 16.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Editors & Dialogs — 77.5 kB (baseline 77.5 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
UI Components — 56.5 kB (baseline 56.5 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 2.77 MB (baseline 2.77 MB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 56.8 kB (baseline 56.8 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.88 MB (baseline 8.88 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 8.04 MB (baseline 8.04 MB) • ⚪ 0 BBundles that do not match a named category
|
⚡ Performance Report
Raw data{
"timestamp": "2026-03-09T18:24:01.443Z",
"gitSha": "c13c63a66649bee6db1d11f53c2844b0a3895683",
"branch": "austin/fix-app-subgraph",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2005.941000000007,
"styleRecalcs": 123,
"styleRecalcDurationMs": 22.708,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 429.44599999999997,
"heapDeltaBytes": -3188532
},
{
"name": "canvas-idle",
"durationMs": 2028.948000000014,
"styleRecalcs": 124,
"styleRecalcDurationMs": 24.191000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 420.4080000000001,
"heapDeltaBytes": -3532572
},
{
"name": "canvas-idle",
"durationMs": 2046.1700000000178,
"styleRecalcs": 123,
"styleRecalcDurationMs": 23.767999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 432.2010000000001,
"heapDeltaBytes": -3702980
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1818.2429999999954,
"styleRecalcs": 164,
"styleRecalcDurationMs": 45.169000000000004,
"layouts": 12,
"layoutDurationMs": 2.9290000000000003,
"taskDurationMs": 847.039,
"heapDeltaBytes": -3487684
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1850.0329999999963,
"styleRecalcs": 168,
"styleRecalcDurationMs": 51.083,
"layouts": 12,
"layoutDurationMs": 3.281,
"taskDurationMs": 845.699,
"heapDeltaBytes": -4308952
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1875.3080000000182,
"styleRecalcs": 172,
"styleRecalcDurationMs": 53.634,
"layouts": 12,
"layoutDurationMs": 4.069,
"taskDurationMs": 844.6039999999999,
"heapDeltaBytes": -2943664
},
{
"name": "dom-widget-clipping",
"durationMs": 630.642000000023,
"styleRecalcs": 46,
"styleRecalcDurationMs": 14.74,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 408.2250000000001,
"heapDeltaBytes": 7713068
},
{
"name": "dom-widget-clipping",
"durationMs": 548.1580000000008,
"styleRecalcs": 40,
"styleRecalcDurationMs": 12.777,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 349.237,
"heapDeltaBytes": 7177508
},
{
"name": "dom-widget-clipping",
"durationMs": 558.4810000000289,
"styleRecalcs": 40,
"styleRecalcDurationMs": 13.800999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 351.627,
"heapDeltaBytes": 7481008
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 633.3970000000022,
"styleRecalcs": 78,
"styleRecalcDurationMs": 20.749999999999996,
"layouts": 1,
"layoutDurationMs": 0.1830000000000001,
"taskDurationMs": 458.4719999999999,
"heapDeltaBytes": 15878184
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 594.7990000000232,
"styleRecalcs": 75,
"styleRecalcDurationMs": 19.86,
"layouts": 1,
"layoutDurationMs": 0.223,
"taskDurationMs": 433.9479999999999,
"heapDeltaBytes": -8222780
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 586.638999999991,
"styleRecalcs": 72,
"styleRecalcDurationMs": 15.071,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 410.07000000000005,
"heapDeltaBytes": -8906824
},
{
"name": "subgraph-idle",
"durationMs": 2025.97099999997,
"styleRecalcs": 124,
"styleRecalcDurationMs": 25.826999999999995,
"layouts": 1,
"layoutDurationMs": 0.2319999999999999,
"taskDurationMs": 413.548,
"heapDeltaBytes": -3043612
},
{
"name": "subgraph-idle",
"durationMs": 2005.2249999999958,
"styleRecalcs": 121,
"styleRecalcDurationMs": 22.932000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 408.184,
"heapDeltaBytes": -3390704
},
{
"name": "subgraph-idle",
"durationMs": 1996.234999999956,
"styleRecalcs": 121,
"styleRecalcDurationMs": 23.418999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 410.09,
"heapDeltaBytes": -3679628
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1968.2849999999803,
"styleRecalcs": 171,
"styleRecalcDurationMs": 54.951,
"layouts": 16,
"layoutDurationMs": 4.6739999999999995,
"taskDurationMs": 958.324,
"heapDeltaBytes": -5134036
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1678.3169999999927,
"styleRecalcs": 153,
"styleRecalcDurationMs": 45.442,
"layouts": 16,
"layoutDurationMs": 3.9710000000000005,
"taskDurationMs": 739.8710000000001,
"heapDeltaBytes": -6638692
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1727.5530000000003,
"styleRecalcs": 158,
"styleRecalcDurationMs": 49.828,
"layouts": 17,
"layoutDurationMs": 4.430999999999999,
"taskDurationMs": 758.629,
"heapDeltaBytes": -5397188
}
]
} |
![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 the current code and only fix it if needed.
Inline comments:
In `@src/components/builder/AppBuilder.vue`:
- Around line 107-130: The selection lookup fails for promoted widgets because
handleClick stores the promoted widget's display name instead of its interior
source name, so update handleClick to detect promoted widgets (use
isPromotedWidgetView/PromotedWidgetView) and store the source identifiers: when
isPromotedWidgetView(widget) push [widget.sourceNodeId, widget.sourceWidgetName]
into appModeStore.selectedInputs (and use those values when checking existence),
otherwise continue storing [node.id, widget.name]; this will make
resolveNodeWidget (which compares w.sourceWidgetName and w.sourceNodeId)
correctly find promoted selections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e634565d-7e0f-4618-9feb-305d9ef97464
📒 Files selected for processing (2)
src/components/builder/AppBuilder.vuesrc/renderer/extensions/linearMode/LinearControls.vue
DrJKL
reviewed
Mar 9, 2026
| for (const widget of nodeData.widgets ?? []) widget.slotMetadata = undefined | ||
| for (const widget of nodeData.widgets ?? []) { | ||
| widget.slotMetadata = undefined | ||
| widget.nodeId = String(node.id) |
Contributor
There was a problem hiding this comment.
I think you might want to use widget.setNodeId to keep the store registration updated.
Collaborator
Author
There was a problem hiding this comment.
I think it's already updated by virtue of being called through extractVueNodeData? At the very least, widget here doesn't have a setNodeId to call 🤔
DrJKL
previously approved these changes
Mar 9, 2026
DrJKL
approved these changes
Mar 9, 2026
AustinMroz
added
needs-backport
github-actions bot
pushed a commit
that referenced
this pull request
Mar 9, 2026
App mode stores the state of selected widgets as a tuple of `[NodeId, WidgetName]`. With recent subgraph changes, for a given node, `widget.name` will no longer uniquely resolve to a single widget. - From both Vue and Litegraph, selecting an input for display in App mode will now resolve the NodeId of the node which owns the widget instead of the selected node. - When displaying selections in litegraph, if the NodeId does not exist in the current graph, instead of resolving the actual node the rootGraph is searched for any subgraphNode which contains a view matching the `[NodeId, WidgetName]` pair. - When displaying widgets in App mode, the widget is always set as being a view of the real widget (This means that they will not display a purple promotion border. Known Issue: - These same subgraph changes made it so that a widget can be linked without being disabled. This PR makes it so widgets which have been linked instead display normally under the assumption that they are incorrectly marked as disabled. As disabled widgets can not be selected as inputs, this should handle normal usage fine, but a better solution is being investigated ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9669-Always-use-interior-nodeId-for-app-mode-31e6d73d365081f8a918d0e43cb659ee) by [Unito](https://www.unito.io)
github-actions bot
pushed a commit
that referenced
this pull request
Mar 9, 2026
App mode stores the state of selected widgets as a tuple of `[NodeId, WidgetName]`. With recent subgraph changes, for a given node, `widget.name` will no longer uniquely resolve to a single widget. - From both Vue and Litegraph, selecting an input for display in App mode will now resolve the NodeId of the node which owns the widget instead of the selected node. - When displaying selections in litegraph, if the NodeId does not exist in the current graph, instead of resolving the actual node the rootGraph is searched for any subgraphNode which contains a view matching the `[NodeId, WidgetName]` pair. - When displaying widgets in App mode, the widget is always set as being a view of the real widget (This means that they will not display a purple promotion border. Known Issue: - These same subgraph changes made it so that a widget can be linked without being disabled. This PR makes it so widgets which have been linked instead display normally under the assumption that they are incorrectly marked as disabled. As disabled widgets can not be selected as inputs, this should handle normal usage fine, but a better solution is being investigated ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9669-Always-use-interior-nodeId-for-app-mode-31e6d73d365081f8a918d0e43cb659ee) by [Unito](https://www.unito.io)
Member
|
@AustinMroz Successfully backported to #9674 |
Member
|
@AustinMroz Successfully backported to #9675 |
github-actions
bot
removed
the
needs-backport
DrJKL
pushed a commit
that referenced
this pull request
Mar 9, 2026
As a temporary fix for widgets being incorrectly hidden, #9669 allowed all disabled widgets to be displayed. This PR provides a more robust implementation to derive whether the widget, as would be displayed from the root graph, is disabled. Potential regression: - Drag drop handlers are applied on node, not widgets. A subgraph containing a "Load Image" node, does not allow dragging and dropping an image onto the subgraphNode in order to load it. Because app mode widgets would display from the original owning node prior to this PR, these drag/drop handlers would apply. Placing "Load Image" nodes. I believe this change makes behavior more consistent, but it warrants consideration. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9671-Restore-hiding-of-linked-inputs-in-app-mode-31e6d73d365081688e37fbb931f3af68) by [Unito](https://www.unito.io)
AustinMroz
added a commit
that referenced
this pull request
Mar 9, 2026
Backport of #9669 to `core/1.41` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9674-backport-core-1-41-Always-use-interior-nodeId-for-app-mode-31e6d73d365081e1a227edf172c0b822) by [Unito](https://www.unito.io) Co-authored-by: AustinMroz <austin@comfy.org>
AustinMroz
added a commit
that referenced
this pull request
Mar 9, 2026
Backport of #9669 to `cloud/1.41` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9675-backport-cloud-1-41-Always-use-interior-nodeId-for-app-mode-31e6d73d365081a8a743c5f23a30a620) by [Unito](https://www.unito.io) Co-authored-by: AustinMroz <austin@comfy.org>
github-actions bot
pushed a commit
that referenced
this pull request
Mar 9, 2026
As a temporary fix for widgets being incorrectly hidden, #9669 allowed all disabled widgets to be displayed. This PR provides a more robust implementation to derive whether the widget, as would be displayed from the root graph, is disabled. Potential regression: - Drag drop handlers are applied on node, not widgets. A subgraph containing a "Load Image" node, does not allow dragging and dropping an image onto the subgraphNode in order to load it. Because app mode widgets would display from the original owning node prior to this PR, these drag/drop handlers would apply. Placing "Load Image" nodes. I believe this change makes behavior more consistent, but it warrants consideration. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9671-Restore-hiding-of-linked-inputs-in-app-mode-31e6d73d365081688e37fbb931f3af68) by [Unito](https://www.unito.io)
github-actions bot
pushed a commit
that referenced
this pull request
Mar 9, 2026
As a temporary fix for widgets being incorrectly hidden, #9669 allowed all disabled widgets to be displayed. This PR provides a more robust implementation to derive whether the widget, as would be displayed from the root graph, is disabled. Potential regression: - Drag drop handlers are applied on node, not widgets. A subgraph containing a "Load Image" node, does not allow dragging and dropping an image onto the subgraphNode in order to load it. Because app mode widgets would display from the original owning node prior to this PR, these drag/drop handlers would apply. Placing "Load Image" nodes. I believe this change makes behavior more consistent, but it warrants consideration. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9671-Restore-hiding-of-linked-inputs-in-app-mode-31e6d73d365081688e37fbb931f3af68) by [Unito](https://www.unito.io)
DrJKL
pushed a commit
that referenced
this pull request
Mar 10, 2026
As a temporary fix for widgets being incorrectly hidden, #9669 allowed all disabled widgets to be displayed. This PR provides a more robust implementation to derive whether the widget, as would be displayed from the root graph, is disabled. Potential regression: - Drag drop handlers are applied on node, not widgets. A subgraph containing a "Load Image" node, does not allow dragging and dropping an image onto the subgraphNode in order to load it. Because app mode widgets would display from the original owning node prior to this PR, these drag/drop handlers would apply. Placing "Load Image" nodes. I believe this change makes behavior more consistent, but it warrants consideration. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9671-Restore-hiding-of-linked-inputs-in-app-mode-31e6d73d365081688e37fbb931f3af68) by [Unito](https://www.unito.io)
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
App mode stores the state of selected widgets as a tuple of
[NodeId, WidgetName]. With recent subgraph changes, for a given node,widget.namewill no longer uniquely resolve to a single widget.[NodeId, WidgetName]pair.Known Issue:
┆Issue is synchronized with this Notion page by Unito