Skip to content

Fix selecting loras on cloud #7560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DrJKL merged 1 commit into from
Dec 16, 2025
Merged

Fix selecting loras on cloud #7560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/composables/graph/useGraphNodeManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager {
* Creates a wrapped callback for a widget that maintains LiteGraph/Vue sync
*/
const createWrappedWidgetCallback = (
widget: { value?: unknown; name: string }, // LiteGraph widget with minimal typing
widget: IBaseWidget, // LiteGraph widget with minimal typing
originalCallback: ((value: unknown) => void) | undefined,
nodeId: string
) => {
Expand All @@ -364,10 +364,10 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager {
}

// Always update widget.value to ensure sync
widget.value = value
widget.value = value ?? undefined

// 2. Call the original callback if it exists
if (originalCallback) {
if (originalCallback && widget.type !== 'asset') {
originalCallback.call(widget, value)
}
Comment on lines +370 to 372
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the file and examine lines 360-380 to understand the callback context
cat -n src/composables/graph/useGraphNodeManager.ts | sed -n '360,380p'

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 812

🏁 Script executed:

#!/bin/bash
# Get more context - the full function/section to understand widget handling
cat -n src/composables/graph/useGraphNodeManager.ts | sed -n '320,380p'

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 2350

🏁 Script executed:

#!/bin/bash
# Search for asset widget type usage and callback context
rg -n --type=ts --type=vue 'type.*asset' src/composables/graph/ -A 3 -B 3

# Also search for where createWrappedWidgetCallback is called
rg -n 'createWrappedWidgetCallback' src/composables/graph/ -A 3 -B 3

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1277

🏁 Script executed:

#!/bin/bash
# Search for asset widget handling and understand the rationale
rg -n 'asset' src/composables/graph/ -A 5 -B 5

# Also look for any PR or issue context about this
git log --oneline -n 20 | head -20

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1037

🏁 Script executed:

#!/bin/bash
# Look at the commit that mentioned "Fix selection of assets in vue mode"
git show 1ab474fb --stat
git show 1ab474fb | head -100

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 50383

🏁 Script executed:

#!/bin/bash
# Look for the specific change in the commit related to asset widgets
git show 1ab474fb src/composables/graph/useGraphNodeManager.ts | grep -A 20 -B 20 'asset'

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 2137

🏁 Script executed:

#!/bin/bash
# Search for "emit" in the widget callback context
rg -n 'emit' src/composables/graph/useGraphNodeManager.ts -B 5 -A 5

# Also search for any comments or context about why asset widgets are special
rg -n 'asset.*emit\|emit.*asset\|model.*emit\|callback.*asset' src/ --type=ts -B 3 -A 3

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 52

🏁 Script executed:

#!/bin/bash
# Search for emit patterns in Vue nodes/widgets
rg -n 'emit' src/composables/graph/ -B 3 -A 3 --type=ts | head -50

# Also search for asset widget type definitions
rg -n '"asset"' src/ --type=ts | head -20

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1041

🏁 Script executed:

#!/bin/bash
# Search for PR references or comments about "Models must be updated"
rg -n 'Models.*emit\|model.*update.*emit' src/ --type=ts -B 5 -A 5

# Look at the full context around the widget callback setup
cat -n src/composables/graph/useGraphNodeManager.ts | sed -n '380,420p'

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1330

🏁 Script executed:

#!/bin/bash
# Search for asset widget callback handling in other files
rg -n 'widget.*type.*asset\|type.*asset.*widget' src/ --type=ts -B 5 -A 5

# Look for any recent changes or TODOs about asset widgets
rg -n 'TODO\|FIXME\|asset' src/composables/graph/useGraphNodeManager.ts | head -30

# Check if there are tests for asset widgets
find . -name "*.spec.ts" -o -name "*.test.ts" | xargs grep -l "asset" 2>/dev/null | head -5

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 287

🏁 Script executed:

#!/bin/bash
# Check widget.spec.ts for asset widget tests
rg -n 'asset' browser_tests/tests/widget.spec.ts -B 5 -A 5

# Also search for the PR/issue context in commit messages
git log --oneline --all | grep -i 'asset\|model.*update\|emit' | head -10

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1578

🏁 Script executed:

#!/bin/bash
# Look at the specific PR or issue related to this fix
git log --format="%h %s" --grep='asset' | head -15

# Check for any documentation or comments about the emit pattern
rg -n 'updateVueWidgetState' src/composables/graph/useGraphNodeManager.ts -B 5 -A 10

# Search for how asset widgets are actually used
rg -n 'load.*asset\|asset.*load' src/ --type=ts -i -B 2 -A 2 | head -30

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1078

🏁 Script executed:

#!/bin/bash
# Search for how Vue emits are handled with widgets
rg -n '@update\|@change\|emit' src/components/vue-nodes/ --type=ts --type=vue -B 2 -A 2 | head -50

# Look for asset widget specific handling in Vue components
rg -n 'type.*asset' src/components/vue-nodes/ --type=vue -B 3 -A 3

# Check if there's a model or asset loader component
find src/components -name "*[Aa]sset*" -o -name "*[Ll]oad*" | head -10

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 565

🏁 Script executed:

#!/bin/bash
# Look for asset widget loading/selection behavior
rg -n 'type === .asset.\|type: .asset.' src/ --type=ts -B 5 -A 10 | head -60

# Search for widget callback usage patterns
rg -n 'widget.callback' src/ --type=ts -B 3 -A 3 | head -50

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 3429

🏁 Script executed:

#!/bin/bash
# Search for remote widget which seems to handle asset-like loading
cat -n src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts | sed -n '1,50p'

# Check what type of widgets use callbacks in the Vue nodes
rg -n 'widget\.type' src/renderer/extensions/vueNodes/ --type=ts -B 2 -A 2 | head -40

# Look for asset-specific widget handling
rg -n 'asset' src/renderer/extensions/vueNodes/ --type=ts -B 3 -A 3

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 26224

🏁 Script executed:

#!/bin/bash
# Look at how asset widgets handle their callbacks in tests
rg -n 'asset.*widget\|widget.*asset' browser_tests/tests/ --type=ts -B 3 -A 3 | head -50

# Check if there are any tests for the asset widget dialog behavior
rg -n 'assetBrowserDialog\|onAssetSelected' src/renderer/extensions/vueNodes/ --type=ts -B 2 -A 2 | head -40

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 52

Asset widgets bypass the original callback intentionally—update the comment for clarity.

Asset widgets handle value updates through a dedicated interaction pattern (asset browser dialog selection), not the traditional callback mechanism. They still update the LiteGraph widget value and Vue state, so skipping the original callback prevents duplicate behavior from the dialog's onAssetSelected handler.

Refine the suggested comment to be more precise:

-        // 2. Call the original callback if it exists
-        // Skip for asset widgets to ensure proper value propagation via emit
+        // 2. Call the original callback if it exists
+        // Skip for asset widgets—they handle value updates via the asset browser dialog
         if (originalCallback && widget.type !== 'asset') {
           originalCallback.call(widget, value)
         }
🤖 Prompt for AI Agents 
In src/composables/graph/useGraphNodeManager.ts around lines 370 to 372, the
current behavior skips calling the originalCallback for asset widgets but the
inline comment is unclear; replace or update the comment to explicitly state
that asset widgets handle value updates via the asset browser dialog (its
onAssetSelected handler), that the code still updates the LiteGraph widget value
and Vue state elsewhere, and that skipping originalCallback prevents duplicate
handling/side-effects from the dialog’s selection flow.


Expand Down
6 changes: 1 addition & 5 deletions src/renderer/extensions/vueNodes/components/NodeWidgets.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,7 @@ const processedWidgets = computed((): ProcessedWidget[] => {
// Update the widget value directly
widget.value = value

// Skip callback for asset widgets - their callback opens the modal,
// but Vue asset mode handles selection through the dropdown
if (widget.type !== 'asset') {
widget.callback?.(value)
}
widget.callback?.(value)
}

const tooltipText = getWidgetTooltip(widget)
Expand Down
Loading