feat(cloud): add asset widget support for PrimitiveNode model selection #8598
+193
−80
Conversation
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
On Comfy Cloud, PrimitiveNode now creates asset widgets (opening Asset Browser) instead of combo widgets for model-eligible inputs like checkpoints, LoRAs, etc. - Add cloud asset widget creation in #createWidget() using isAssetBrowserEligible() - Add #createAssetWidget() helper following useComboWidget.ts pattern - Add #finalizeWidget() helper to DRY up widget sizing/callback setup - Pass target node's comfyClass to Asset Browser for correct model filtering Amp-Thread-ID: https://ampcode.com/threads/T-019c0839-bbdc-754a-9d3b-151417058ded Co-authored-by: Amp <amp@ampcode.com>
- Extract createAssetWidget factory to src/platform/assets/utils/ - Refactor useComboWidget.ts to use the shared factory - Simplify PrimitiveNode to use shared factory - Convert JS # privates to underscore convention - Add knip ignore for isAssetWidget (litegraph public API) Amp-Thread-ID: https://ampcode.com/threads/T-019c0839-bbdc-754a-9d3b-151417058ded Co-authored-by: Amp <amp@ampcode.com>
- Add Comfy.Assets.UseAssetAPI toggle check (matches useComboWidget behavior) - Sync existing target widget value to asset widget (fixes placeholder issue) Amp-Thread-ID: https://ampcode.com/threads/T-019c0839-bbdc-754a-9d3b-151417058ded Co-authored-by: Amp <amp@ampcode.com>
…odo.md - Add toast notifications for asset validation errors (surfacing to user) - Add i18n translations for invalidAsset and invalidFilename errors - Remove todo.md that was accidentally committed Amp-Thread-ID: https://ampcode.com/threads/T-019c0c78-3249-72eb-9c45-0db1bf7067d6 Co-authored-by: Amp <amp@ampcode.com>
- Merge latest main to resolve conflicts
- Fix asset browser filtering: pass target input name (e.g.,
'ckpt_name') instead of PrimitiveNode's widget name ('value')
**Changes:**
- Add `inputNameForBrowser` param to `createAssetWidget`
- Pass `targetInputName` from `PrimitiveNode._createAssetWidget`
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8596-fix-merge-main-and-pass-target-input-name-to-asset-browser-2fd6d73d36508112bb17cf5d3fe54687)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Subagent 5 <subagent@example.com>
Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: guill <jacob.e.segal@gmail.com>
Co-authored-by: Jin Yi <jin12cc@gmail.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: AustinMroz <austin@comfy.org>
Co-authored-by: Comfy Org PR Bot <snomiao+comfy-pr@gmail.com>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Johnpaul Chiwetelu <49923152+Myestery@users.noreply.github.com>
Co-authored-by: Rizumu Ayaka <rizumu@ayaka.moe>
Co-authored-by: Kelly Yang <124ykl@gmail.com>
Co-authored-by: sno <snomiao@gmail.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Benjamin Lu <benjaminlu1107@gmail.com>
Co-authored-by: Terry Jia <terryjia88@gmail.com>
Co-authored-by: Luke Mino-Altherr <luke@comfy.org>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019c25ec-864d-75be-8964-2056b6662bc2
dosubot
bot
added
the
size:L
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/04/2026, 12:50:48 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Contributor
📝 WalkthroughWalkthroughCloud-asset widget integration is introduced for model-eligible inputs in node widget creation, alongside a new asset widget utility function. Supporting changes include export restructuring, translation keys for asset error handling, and refactored widget composition logic. Changes
Sequence DiagramsequenceDiagram
participant Node as PrimitiveNode
participant Widget as Asset Widget
participant Dialog as Asset Browser Dialog
participant Validation as Validation Service
participant Callback as onValueChange
Node->>Widget: _createAssetWidget(InputSpec)
Widget->>Widget: Set default value from InputSpec
Node->>Widget: _finalizeWidget()
Widget->>Dialog: Open Asset Browser (user opens widget)
Dialog->>Dialog: User selects asset
Widget->>Validation: Validate asset schema
Validation-->>Widget: Asset valid/invalid
alt Asset Valid
Widget->>Validation: Get & validate filename
Validation-->>Widget: Filename valid/invalid
alt Filename Valid
Widget->>Widget: Update widget value
Widget->>Callback: Invoke onValueChange(newValue, oldValue)
Callback->>Node: Trigger node change
else Filename Invalid
Widget->>Widget: Show error toast
end
else Asset Invalid
Widget->>Widget: Show error toast
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Comment |
🎭 Playwright Tests:
|
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.5 kB (baseline 22.5 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 838 kB (baseline 837 kB) • 🔴 +327 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 69 kB (baseline 69 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 446 kB (baseline 446 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 12 added / 12 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 3.47 kB (baseline 3.47 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 37.8 kB (baseline 37.8 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 2.1 MB (baseline 2.1 MB) • 🔴 +1.81 kBStores, services, APIs, and repositories
Status: 11 added / 11 removed Utilities & Hooks — 234 kB (baseline 234 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 14 added / 14 removed Vendor & Third-Party — 9.37 MB (baseline 9.37 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7 MB (baseline 6.99 MB) • 🔴 +1.57 kBBundles that do not match a named category
Status: 55 added / 55 removed |
![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: 2
🤖 Fix all issues with AI agents
In `@src/platform/assets/utils/createAssetWidget.ts`:
- Around line 67-72: The console.error call in createAssetWidget.ts currently
logs the entire asset payload (asset) on validation failure; change it to avoid
printing sensitive data by removing the asset object from the log and instead
log only safe identifiers or metadata (e.g., asset.id, asset.name, or
validatedAsset.error.errors) or a redacted placeholder; update the call that
references validatedAsset and asset so it emits a concise message with those
identifiers and the validation errors rather than the full payload.
In `@src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts`:
- Around line 85-99: The asset browser is missing the inputNameForBrowser so
nodes with multiple model inputs can get incorrect asset lists; update
createAssetBrowserWidget to pass inputNameForBrowser into the createAssetWidget
call (use inputSpec.inputNameForBrowser, falling back to inputSpec.name if
absent) so the widget receives the correct input identifier for asset filtering;
modify the object passed to createAssetWidget in createAssetBrowserWidget to
include this property while keeping the existing onValueChange, node,
widgetName, nodeTypeForBrowser and defaultValue fields.
Comment on lines
+67
to
+72
| console.error( | ||
| 'Invalid asset item:', | ||
| validatedAsset.error.errors, | ||
| 'Received:', | ||
| asset | ||
| ) |
Contributor
There was a problem hiding this comment.
Avoid logging full asset payloads.
Line 67 logs the entire asset object on validation failure, which can expose user-provided paths/URLs in logs. Log only safe identifiers or omit the payload entirely.
🛡️ Suggested change
- console.error(
- 'Invalid asset item:',
- validatedAsset.error.errors,
- 'Received:',
- asset
- )
+ console.error('Invalid asset item:', validatedAsset.error.errors)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| console.error( | |
| 'Invalid asset item:', | |
| validatedAsset.error.errors, | |
| 'Received:', | |
| asset | |
| ) | |
| console.error('Invalid asset item:', validatedAsset.error.errors) |
🤖 Prompt for AI Agents
In `@src/platform/assets/utils/createAssetWidget.ts` around lines 67 - 72, The
console.error call in createAssetWidget.ts currently logs the entire asset
payload (asset) on validation failure; change it to avoid printing sensitive
data by removing the asset object from the log and instead log only safe
identifiers or metadata (e.g., asset.id, asset.name, or
validatedAsset.error.errors) or a redacted placeholder; update the call that
references validatedAsset and asset so it emits a concise message with those
identifiers and the validation errors rather than the full payload.
Comment on lines
+85
to
99
| function createAssetBrowserWidget( | ||
| node: LGraphNode, | ||
| inputSpec: ComboInputSpec, | ||
| defaultValue: string | undefined | ||
| ): IBaseWidget => { | ||
| const currentValue = defaultValue | ||
| const displayLabel = currentValue ?? t('widgets.selectModel') | ||
| const assetBrowserDialog = useAssetBrowserDialog() | ||
|
|
||
| async function openModal(widget: IBaseWidget) { | ||
| if (!isAssetWidget(widget)) { | ||
| throw new Error(`Expected asset widget but received ${widget.type}`) | ||
| ): IBaseWidget { | ||
| return createAssetWidget({ | ||
| node, | ||
| widgetName: inputSpec.name, | ||
| nodeTypeForBrowser: node.comfyClass ?? '', | ||
| defaultValue, | ||
| onValueChange: (widget, newValue, oldValue) => { | ||
| node.onWidgetChanged?.(widget.name, newValue, oldValue, widget) | ||
| } | ||
| await assetBrowserDialog.show({ | ||
| nodeType: node.comfyClass || '', | ||
| inputName: inputSpec.name, | ||
| currentValue: widget.value, | ||
| onAssetSelected: (asset) => { | ||
| const validatedAsset = assetItemSchema.safeParse(asset) | ||
|
|
||
| if (!validatedAsset.success) { | ||
| console.error( | ||
| 'Invalid asset item:', | ||
| validatedAsset.error.errors, | ||
| 'Received:', | ||
| asset | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| const filename = getAssetFilename(validatedAsset.data) | ||
| const validatedFilename = assetFilenameSchema.safeParse(filename) | ||
|
|
||
| if (!validatedFilename.success) { | ||
| console.error( | ||
| 'Invalid asset filename:', | ||
| validatedFilename.error.errors, | ||
| 'for asset:', | ||
| validatedAsset.data.id | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| const oldValue = widget.value | ||
| widget.value = validatedFilename.data | ||
| node.onWidgetChanged?.( | ||
| widget.name, | ||
| validatedFilename.data, | ||
| oldValue, | ||
| widget | ||
| ) | ||
| } | ||
| }) | ||
| } | ||
| const options: IWidgetAssetOptions = { openModal } | ||
|
|
||
| const widget = node.addWidget( | ||
| 'asset', | ||
| inputSpec.name, | ||
| displayLabel, | ||
| () => undefined, | ||
| options | ||
| ) | ||
|
|
||
| return widget | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Pass inputNameForBrowser to keep asset filtering accurate.
Without it, nodes with multiple model inputs (e.g., checkpoint vs LoRA) can surface the wrong asset set.
Suggested fix
return createAssetWidget({
node,
widgetName: inputSpec.name,
nodeTypeForBrowser: node.comfyClass ?? '',
+ inputNameForBrowser: inputSpec.name,
defaultValue,
onValueChange: (widget, newValue, oldValue) => {
node.onWidgetChanged?.(widget.name, newValue, oldValue, widget)
}
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function createAssetBrowserWidget( | |
| node: LGraphNode, | |
| inputSpec: ComboInputSpec, | |
| defaultValue: string | undefined | |
| ): IBaseWidget => { | |
| const currentValue = defaultValue | |
| const displayLabel = currentValue ?? t('widgets.selectModel') | |
| const assetBrowserDialog = useAssetBrowserDialog() | |
| async function openModal(widget: IBaseWidget) { | |
| if (!isAssetWidget(widget)) { | |
| throw new Error(`Expected asset widget but received ${widget.type}`) | |
| ): IBaseWidget { | |
| return createAssetWidget({ | |
| node, | |
| widgetName: inputSpec.name, | |
| nodeTypeForBrowser: node.comfyClass ?? '', | |
| defaultValue, | |
| onValueChange: (widget, newValue, oldValue) => { | |
| node.onWidgetChanged?.(widget.name, newValue, oldValue, widget) | |
| } | |
| await assetBrowserDialog.show({ | |
| nodeType: node.comfyClass || '', | |
| inputName: inputSpec.name, | |
| currentValue: widget.value, | |
| onAssetSelected: (asset) => { | |
| const validatedAsset = assetItemSchema.safeParse(asset) | |
| if (!validatedAsset.success) { | |
| console.error( | |
| 'Invalid asset item:', | |
| validatedAsset.error.errors, | |
| 'Received:', | |
| asset | |
| ) | |
| return | |
| } | |
| const filename = getAssetFilename(validatedAsset.data) | |
| const validatedFilename = assetFilenameSchema.safeParse(filename) | |
| if (!validatedFilename.success) { | |
| console.error( | |
| 'Invalid asset filename:', | |
| validatedFilename.error.errors, | |
| 'for asset:', | |
| validatedAsset.data.id | |
| ) | |
| return | |
| } | |
| const oldValue = widget.value | |
| widget.value = validatedFilename.data | |
| node.onWidgetChanged?.( | |
| widget.name, | |
| validatedFilename.data, | |
| oldValue, | |
| widget | |
| ) | |
| } | |
| }) | |
| } | |
| const options: IWidgetAssetOptions = { openModal } | |
| const widget = node.addWidget( | |
| 'asset', | |
| inputSpec.name, | |
| displayLabel, | |
| () => undefined, | |
| options | |
| ) | |
| return widget | |
| }) | |
| } | |
| function createAssetBrowserWidget( | |
| node: LGraphNode, | |
| inputSpec: ComboInputSpec, | |
| defaultValue: string | undefined | |
| ): IBaseWidget { | |
| return createAssetWidget({ | |
| node, | |
| widgetName: inputSpec.name, | |
| nodeTypeForBrowser: node.comfyClass ?? '', | |
| inputNameForBrowser: inputSpec.name, | |
| defaultValue, | |
| onValueChange: (widget, newValue, oldValue) => { | |
| node.onWidgetChanged?.(widget.name, newValue, oldValue, widget) | |
| } | |
| }) | |
| } |
🤖 Prompt for AI Agents
In `@src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts`
around lines 85 - 99, The asset browser is missing the inputNameForBrowser so
nodes with multiple model inputs can get incorrect asset lists; update
createAssetBrowserWidget to pass inputNameForBrowser into the createAssetWidget
call (use inputSpec.inputNameForBrowser, falling back to inputSpec.name if
absent) so the widget receives the correct input identifier for asset filtering;
modify the object passed to createAssetWidget in createAssetBrowserWidget to
include this property while keeping the existing onValueChange, node,
widgetName, nodeTypeForBrowser and defaultValue fields.
Contributor
Author
|
This was approved on the previous PR (this one is just a re-opening of that) |
Contributor
Author
|
@coderabbitai make a tracking Issue: After #8594 is merged, refactor this code to be simpler if possible. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 57
🧠 Learnings used |
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.
Add cloud asset widget creation in
_createWidget()usingisAssetBrowserEligible()createAssetWidgetfactory tosrc/platform/assets/utils/useComboWidget.tsto use the shared factory_finalizeWidget()helper to DRY up widget sizing/callback setupcomfyClassand input name to Asset Browser for correct model filteringComfy.Assets.UseAssetAPIsetting (matchesuseComboWidget.tsbehavior)Supersedes #8461 (clean rebase, no merge commits)
┆Issue is synchronized with this Notion page by Unito
Summary by CodeRabbit