refactor: unify widget promotion to use store as single source of truth#9550
Closed
refactor: unify widget promotion to use store as single source of truth#9550
Conversation
SubgraphInput.connect() dispatches widget-promotion-requested event for widget-backed slots instead of creating input slots and links. EmptySubgraphInput.connect() checks for widgets before creating input slots. All promotions now flow through promotionStore. Removed ~200 lines of linked-promotion infrastructure from SubgraphNode (LinkedPromotionEntry, _resolveLinkedPromotionByInputName, _syncPromotions, _buildPromotionReconcileState, _setWidget, etc.). Removed _widget property from SubgraphInput, NodeInputSlot, INodeInputSlot, and LGraphNode.removeWidget(). Amp-Thread-ID: https://ampcode.com/threads/T-019ccb06-9ebd-7161-a547-ab84d6ac2afd Co-authored-by: Amp <amp@ampcode.com>
dosubot
bot
added
the
size:XXL
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ❌ 532 passed, 13 failed · 1 flaky❌ Failed Tests📊 Browser Reports
|
Contributor
📝 WalkthroughWalkthroughThis PR removes internal widget reference plumbing from node inputs and subgraph inputs, replacing proxy widget patterns with a promotion-store-backed event-driven approach. Complex promotion reconciliation logic in SubgraphNode is replaced with a streamlined widget-promotion-requested event handler. Widget assignment timing in tests is adjusted to occur after link connections. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
📦 Bundle: 4.56 MB gzip 🟢 -1.43 kBDetailsSummary
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 — 954 kB (baseline 954 kB) • ⚪ 0 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.76 MB (baseline 2.77 MB) • 🟢 -8.79 kBStores, 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.87 MB (baseline 8.87 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 |
⚡ Performance Report
Raw data{
"timestamp": "2026-03-08T01:27:39.961Z",
"gitSha": "83580909b34eae2420d0e59e57b1243c4024f88e",
"branch": "drjkl/one-promotion-at-a-time",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2030.9639999999831,
"styleRecalcs": 124,
"styleRecalcDurationMs": 27.965999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 523.466,
"heapDeltaBytes": -3434408
},
{
"name": "canvas-idle",
"durationMs": 2018.8890000000015,
"styleRecalcs": 122,
"styleRecalcDurationMs": 25.197,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 462.368,
"heapDeltaBytes": -4807500
},
{
"name": "canvas-idle",
"durationMs": 2039.124999999956,
"styleRecalcs": 124,
"styleRecalcDurationMs": 27.471999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 513.242,
"heapDeltaBytes": -3931248
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2082.6979999999935,
"styleRecalcs": 188,
"styleRecalcDurationMs": 67.42399999999999,
"layouts": 13,
"layoutDurationMs": 4.173,
"taskDurationMs": 1122.9089999999999,
"heapDeltaBytes": -2294440
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1841.766000000007,
"styleRecalcs": 169,
"styleRecalcDurationMs": 52.154999999999994,
"layouts": 12,
"layoutDurationMs": 3.764,
"taskDurationMs": 868.153,
"heapDeltaBytes": -3143764
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1847.2300000000246,
"styleRecalcs": 168,
"styleRecalcDurationMs": 62.894999999999996,
"layouts": 12,
"layoutDurationMs": 3.42,
"taskDurationMs": 887.7539999999999,
"heapDeltaBytes": -3165112
},
{
"name": "dom-widget-clipping",
"durationMs": 551.7250000000047,
"styleRecalcs": 40,
"styleRecalcDurationMs": 13.082,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 358.121,
"heapDeltaBytes": 6499864
},
{
"name": "dom-widget-clipping",
"durationMs": 616.6359999999997,
"styleRecalcs": 44,
"styleRecalcDurationMs": 14.722,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 392.243,
"heapDeltaBytes": 6388300
},
{
"name": "dom-widget-clipping",
"durationMs": 591.6389999999865,
"styleRecalcs": 43,
"styleRecalcDurationMs": 13.829,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 377.61300000000006,
"heapDeltaBytes": 7200236
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 644.4459999999879,
"styleRecalcs": 76,
"styleRecalcDurationMs": 17.367,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 440.005,
"heapDeltaBytes": 15589336
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 650.527000000011,
"styleRecalcs": 76,
"styleRecalcDurationMs": 17.243000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 446.133,
"heapDeltaBytes": -8821732
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 644.8570000000018,
"styleRecalcs": 76,
"styleRecalcDurationMs": 17.370000000000005,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 443.958,
"heapDeltaBytes": 14556040
},
{
"name": "subgraph-idle",
"durationMs": 2022.8810000000124,
"styleRecalcs": 122,
"styleRecalcDurationMs": 27.829000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 504.43800000000005,
"heapDeltaBytes": -2857588
},
{
"name": "subgraph-idle",
"durationMs": 2006.9070000000124,
"styleRecalcs": 121,
"styleRecalcDurationMs": 26.506999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 476.60900000000004,
"heapDeltaBytes": -3357532
},
{
"name": "subgraph-idle",
"durationMs": 1998.960000000011,
"styleRecalcs": 121,
"styleRecalcDurationMs": 26.912,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 481.345,
"heapDeltaBytes": -3692216
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1717.4149999999884,
"styleRecalcs": 158,
"styleRecalcDurationMs": 53.133,
"layouts": 17,
"layoutDurationMs": 4.761,
"taskDurationMs": 805.544,
"heapDeltaBytes": 20406968
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1741.1189999999976,
"styleRecalcs": 157,
"styleRecalcDurationMs": 51.641999999999996,
"layouts": 16,
"layoutDurationMs": 4.8,
"taskDurationMs": 840.567,
"heapDeltaBytes": -6683140
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1995.8030000000235,
"styleRecalcs": 172,
"styleRecalcDurationMs": 61.237,
"layouts": 16,
"layoutDurationMs": 4.944999999999999,
"taskDurationMs": 1057.0499999999997,
"heapDeltaBytes": -5687636
}
]
} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/composables/graph/useGraphNodeManager.test.ts (1)
302-310: Hide this slot-wiring workaround behind a fixture helper.The
_internalConfigureAfterSlots()+ temporary widget stripping sequence bakesSubgraphNodeinternals into a composable test. Moving that setup intosubgraphHelperswould keep this test focused on nested-promotion behavior and make future slot/promotion refactors less brittle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/graph/useGraphNodeManager.test.ts` around lines 302 - 310, Wrap the slot-wiring workaround (createTestSubgraphNode -> subgraphB.add -> subgraphNodeA._internalConfigureAfterSlots -> temporarily delete inputs[0].widget -> subgraphB.inputNode.slots[0].connect(...) -> restore widget) into a test fixture helper inside subgraphHelpers (e.g., createAndConnectSubgraphNode or connectSubgraphNodeToSubgraphInput) and replace the inline sequence in the test with a single call to that helper; ensure the helper accepts parameters for the subgraph owner and node id and performs the temporary widget strip/restore around the connect so callers (tests using subgraphNodeA, subgraphB, inputs, _internalConfigureAfterSlots) no longer need to manipulate internals directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/composables/graph/useGraphNodeManager.test.ts`:
- Around line 302-310: Wrap the slot-wiring workaround (createTestSubgraphNode
-> subgraphB.add -> subgraphNodeA._internalConfigureAfterSlots -> temporarily
delete inputs[0].widget -> subgraphB.inputNode.slots[0].connect(...) -> restore
widget) into a test fixture helper inside subgraphHelpers (e.g.,
createAndConnectSubgraphNode or connectSubgraphNodeToSubgraphInput) and replace
the inline sequence in the test with a single call to that helper; ensure the
helper accepts parameters for the subgraph owner and node id and performs the
temporary widget strip/restore around the connect so callers (tests using
subgraphNodeA, subgraphB, inputs, _internalConfigureAfterSlots) no longer need
to manipulate internals directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1b91a5af-37a9-4cb2-a098-2791abdb7030
📒 Files selected for processing (14)
src/composables/graph/useGraphNodeManager.test.tssrc/composables/graph/useGraphNodeManager.tssrc/core/graph/subgraph/promotedWidgetView.test.tssrc/core/graph/subgraph/resolveSubgraphInputLink.test.tssrc/core/graph/subgraph/resolveSubgraphInputTarget.test.tssrc/core/graph/subgraph/subgraphNodePromotion.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/infrastructure/SubgraphInputEventMap.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/node/NodeInputSlot.tssrc/lib/litegraph/src/subgraph/EmptySubgraphInput.tssrc/lib/litegraph/src/subgraph/SubgraphInput.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.ts
💤 Files with no reviewable changes (5)
- src/lib/litegraph/src/subgraph/SubgraphMemory.test.ts
- src/lib/litegraph/src/LGraphNode.ts
- src/composables/graph/useGraphNodeManager.ts
- src/lib/litegraph/src/interfaces.ts
- src/lib/litegraph/src/node/NodeInputSlot.ts
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
1 participant
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
Unify subgraph widget promotion to use promotionStore as single source of truth, removing ~350 lines of linked-promotion infrastructure.
Changes
widget-promotion-requestedevent for widget-backed slots instead of creating input slots/links. EmptySubgraphInput.connect() checks for widgets before creating input slots. SubgraphNode handles the event via extracted_handleWidgetPromotionRequested(). Removed LinkedPromotionEntry,_resolveLinkedPromotionByInputName,_syncPromotions,_buildPromotionReconcileState,_setWidget,_resolveInputWidget, and related methods from SubgraphNode. Removed_widget/_widgetRefproperty from SubgraphInput, NodeInputSlot, INodeInputSlot, and LGraphNode.removeWidget()._widgetproperty removed from SubgraphInput and NodeInputSlot; linked-promotion methods removed from SubgraphNode. Extensions relying on these internals will need to use promotionStore instead.Review Focus
┆Issue is synchronized with this Notion page by Unito