Restore hiding of linked inputs in app mode#9671
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces an inline resolveNodeWidget with a shared helper in Changes
Sequence Diagram(s)sequenceDiagram
participant LinearControls
participant LitegraphUtil as resolveNodeWidget
participant SubgraphModule as isPromotedWidgetView
participant NodeStore as NodeData
LinearControls->>LitegraphUtil: resolveNodeWidget(nodeId, widgetName)
alt direct widget found on node
LitegraphUtil-->>LinearControls: [node, widget]
else widget not found directly
LitegraphUtil->>SubgraphModule: inspect promoted widgets / source mappings
SubgraphModule-->>LitegraphUtil: match info (owningSubgraphNode, promotedWidget)
LitegraphUtil-->>LinearControls: [owningSubgraphNode, promotedWidget]
end
LinearControls->>NodeStore: filter/map widgets, reset slotMetadata & nodeId
NodeStore-->>LinearControls: updated node/widget structures
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 🔴 +150 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) • 🟢 -107 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
Status: 9 added / 9 removed Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16.1 kB (baseline 16.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 77.5 kB (baseline 77.5 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 56.5 kB (baseline 56.5 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.77 MB (baseline 2.77 MB) • 🔴 +550 BStores, services, APIs, and repositories
Status: 14 added / 14 removed Utilities & Hooks — 56.8 kB (baseline 56.8 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 11 added / 11 removed 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
Status: 50 added / 50 removed |
| remove(nodeData.widgets ?? [], (w) => !inputGroup.includes(w.name)) | ||
| remove(nodeData.widgets ?? [], (w) => !!w.slotMetadata?.linked) | ||
| if (node.isSubgraphNode()) { | ||
| remove(nodeData.widgets ?? [], (vueWidget) => { |
There was a problem hiding this comment.
Non-blocking, it does feel like there's a common filter function that could be extracted to avoid the branches here.
There was a problem hiding this comment.
Was conflicted on this as well. With hindsight, it's definitely cleaner. 👍
⚡ Performance Report
Raw data{
"timestamp": "2026-03-09T20:07:42.223Z",
"gitSha": "6a11b19a08dd3f92f20bdbb80737ebd8c301f924",
"branch": "austin/fix-disabled-app-inputs",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2051.191000000017,
"styleRecalcs": 127,
"styleRecalcDurationMs": 28.252,
"layouts": 1,
"layoutDurationMs": 0.26599999999999996,
"taskDurationMs": 445.624,
"heapDeltaBytes": -2713576
},
{
"name": "canvas-idle",
"durationMs": 2025.2039999999738,
"styleRecalcs": 123,
"styleRecalcDurationMs": 20.628999999999994,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 381.24600000000004,
"heapDeltaBytes": -3523088
},
{
"name": "canvas-idle",
"durationMs": 2012.7029999999877,
"styleRecalcs": 122,
"styleRecalcDurationMs": 21.797,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 398.726,
"heapDeltaBytes": -3700032
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2085.239999999999,
"styleRecalcs": 186,
"styleRecalcDurationMs": 72.52,
"layouts": 13,
"layoutDurationMs": 4.007999999999999,
"taskDurationMs": 1058.8509999999999,
"heapDeltaBytes": -2410224
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1823.172999999997,
"styleRecalcs": 167,
"styleRecalcDurationMs": 47.154,
"layouts": 12,
"layoutDurationMs": 3.429,
"taskDurationMs": 809.9060000000001,
"heapDeltaBytes": -2958700
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1833.4839999999986,
"styleRecalcs": 167,
"styleRecalcDurationMs": 46.120000000000005,
"layouts": 12,
"layoutDurationMs": 3.4920000000000004,
"taskDurationMs": 795.729,
"heapDeltaBytes": -3909176
},
{
"name": "dom-widget-clipping",
"durationMs": 573.7129999999979,
"styleRecalcs": 41,
"styleRecalcDurationMs": 12.959,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 340.61199999999997,
"heapDeltaBytes": 6233560
},
{
"name": "dom-widget-clipping",
"durationMs": 595.7690000000184,
"styleRecalcs": 44,
"styleRecalcDurationMs": 13.485999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 360.308,
"heapDeltaBytes": 6779968
},
{
"name": "dom-widget-clipping",
"durationMs": 566.7910000000234,
"styleRecalcs": 42,
"styleRecalcDurationMs": 12.443,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 371.316,
"heapDeltaBytes": 7912964
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 611.438000000021,
"styleRecalcs": 74,
"styleRecalcDurationMs": 15.507,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 417.632,
"heapDeltaBytes": -8594312
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 593.5499999999934,
"styleRecalcs": 73,
"styleRecalcDurationMs": 16.726,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 421.619,
"heapDeltaBytes": -8647980
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 609.0500000000247,
"styleRecalcs": 74,
"styleRecalcDurationMs": 14.291,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 398.6580000000001,
"heapDeltaBytes": -8825748
},
{
"name": "subgraph-idle",
"durationMs": 1999.0269999999555,
"styleRecalcs": 121,
"styleRecalcDurationMs": 21.774,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 404.98400000000004,
"heapDeltaBytes": -3463004
},
{
"name": "subgraph-idle",
"durationMs": 2007.1760000000154,
"styleRecalcs": 123,
"styleRecalcDurationMs": 26.947000000000003,
"layouts": 1,
"layoutDurationMs": 0.2739999999999999,
"taskDurationMs": 419.41499999999996,
"heapDeltaBytes": -2748928
},
{
"name": "subgraph-idle",
"durationMs": 2006.6019999999867,
"styleRecalcs": 123,
"styleRecalcDurationMs": 27.432,
"layouts": 1,
"layoutDurationMs": 0.19200000000000012,
"taskDurationMs": 425.28899999999993,
"heapDeltaBytes": -4065296
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 2115.7350000000292,
"styleRecalcs": 180,
"styleRecalcDurationMs": 75.62200000000001,
"layouts": 16,
"layoutDurationMs": 5.656,
"taskDurationMs": 1256.3580000000002,
"heapDeltaBytes": -5478264
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1782.3629999999753,
"styleRecalcs": 160,
"styleRecalcDurationMs": 52.69499999999999,
"layouts": 16,
"layoutDurationMs": 4.871,
"taskDurationMs": 818.304,
"heapDeltaBytes": -6141384
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 2029.0089999999736,
"styleRecalcs": 175,
"styleRecalcDurationMs": 60.111000000000004,
"layouts": 16,
"layoutDurationMs": 4.679,
"taskDurationMs": 1109.424,
"heapDeltaBytes": -4995700
}
]
} |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/renderer/extensions/linearMode/LinearControls.vue`:
- Around line 74-82: The loop over unprocessedInputs can hang because the guard
"if (!node) continue" skips advancing unprocessedInputs; update the code in the
loop that processes unprocessedInputs (referencing variables/functions:
unprocessedInputs, node, takeWhile, inputGroup, LGraphEventMode.ALWAYS) by
either removing the unreachable guard altogether if resolveNodeWidget guarantees
node is always defined, or change the guard to advance the array before
continuing (e.g., slice off the first element or otherwise increment the
consumed count) so the loop always makes progress.
In `@src/utils/litegraphUtil.ts`:
- Around line 323-347: In resolveNodeWidget, make the sourceNodeId comparison
type-safe by coercing nodeId to string (e.g., use String(nodeId) ===
w.sourceNodeId) so numeric NodeId values match the widget's string sourceNodeId;
also address the search scope: either document in the function JSDoc that
promoted widgets are only looked for among direct subgraph nodes (graph.nodes)
or extend the search to recursively traverse nested subgraphs (similar to
resolveNode using graph.subgraphs.values()) and update the implementation
accordingly so promoted widgets in nested subgraphs are found.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e29ab1cb-a9df-4a10-bbce-3ded8a2dc817
📒 Files selected for processing (3)
src/components/builder/AppBuilder.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/utils/litegraphUtil.ts
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/renderer/extensions/linearMode/LinearControls.vue`:
- Around line 67-72: The current mapping for unprocessedInputs uses
resolveNodeWidget on entries from appModeStore.selectedInputs but only guards on
widget truthiness, allowing cases where node is undefined to slip through;
update the mapping for unprocessedInputs (the flatMap that calls
resolveNodeWidget) to only return ([[node, widget]] as const) when both node and
widget are truthy (i.e., explicitly check node && widget) so downstream code
that assumes a defined node/widget pair no longer receives undefined entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d29805bd-c9b5-4179-8db5-cc83a7a46d28
📒 Files selected for processing (1)
src/renderer/extensions/linearMode/LinearControls.vue
|
|
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)
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 Successfully backported to #9676 |
|
@AustinMroz Successfully backported to #9677 |
) Backport of #9671 to `cloud/1.41` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9677-backport-cloud-1-41-Restore-hiding-of-linked-inputs-in-app-mode-31e6d73d36508192af4ac6a5178d97c4) by [Unito](https://www.unito.io) Co-authored-by: AustinMroz <austin@comfy.org>
Backport of #9671 to `core/1.41` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9676-backport-core-1-41-Restore-hiding-of-linked-inputs-in-app-mode-31e6d73d365081f7bab6ee781b782810) by [Unito](https://www.unito.io) Co-authored-by: AustinMroz <austin@comfy.org>
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)
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:
┆Issue is synchronized with this Notion page by Unito