-
Notifications
You must be signed in to change notification settings - Fork 447
fix: Refresh model dropdowns after upload #7232
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.
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
Conversation
📝 WalkthroughWalkthroughThe change enhances the upload model wizard by adding post-upload cache refresh logic. After successful upload, the wizard now fetches all node providers for the selected model type and updates models for each node type using the assets and model-to-node stores. Changes
Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (10)src/**/*.{vue,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.ts📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/{services,composables}/**/*.{ts,tsx}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/**/{composables,components}/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/**/{components,composables}/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,vue}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/composables/**/use*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
🔇 Additional comments (2)
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 Test Results⏰ Completed at: 12/08/2025, 11:43:57 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/08/2025, 11:35:18 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Model selection dropdowns now update automatically after uploading a new model via the upload wizard. The fix dispatches a model-upload-success event with the model type, and widgets listen for matching events to refetch their data. Also refactored to use VueUse's useEventListener for cleaner lifecycle management. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
acbea43 to
c45ffef
Compare
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.21 MB (baseline 3.21 MB) • ⚪ 0 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 978 kB (baseline 978 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 177 kB (baseline 177 kB) • 🔴 +451 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
Status: 17 added / 17 removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/platform/assets/composables/useUploadModelWizard.ts(1 hunks)src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.tssrc/platform/assets/composables/useUploadModelWizard.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.tssrc/platform/assets/composables/useUploadModelWizard.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.tssrc/platform/assets/composables/useUploadModelWizard.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.tssrc/platform/assets/composables/useUploadModelWizard.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.tssrc/platform/assets/composables/useUploadModelWizard.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.tssrc/platform/assets/composables/useUploadModelWizard.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.tssrc/platform/assets/composables/useUploadModelWizard.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.tssrc/platform/assets/composables/useUploadModelWizard.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.tssrc/platform/assets/composables/useUploadModelWizard.ts
**/composables/**/use*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables with
useXyz.tspattern
Files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.tssrc/platform/assets/composables/useUploadModelWizard.ts
🧠 Learnings (10)
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners
Applied to files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.vue : Use `computed` instead of a `ref` and `watch` if possible
Applied to files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use ref/reactive for state management in Vue 3 Composition API
Applied to files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize ref and reactive for reactive state
Applied to files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Implement computed() for derived state in Vue 3 Composition API
Applied to files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.{vue,ts} : Leverage VueUse functions for performance-enhancing styles
Applied to files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Use watch and watchEffect for side effects
Applied to files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.vue : Leverage VueUse functions for performance-enhancing styles
Applied to files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Implement computed properties with computed()
Applied to files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use lifecycle hooks: onMounted, onUpdated in Vue 3 Composition API
Applied to files:
src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
🔇 Additional comments (2)
src/platform/assets/composables/useUploadModelWizard.ts (1)
146-152: LGTM! Event-driven cache invalidation implemented correctly.The event is dispatched after successful upload with the model type in the detail payload. The
canUploadModelguard ensuresselectedModelType.valueis truthy before upload proceeds, so the event detail will contain a valid model type at this point.src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts (1)
3-3: LGTM! VueUse composable correctly imported.Using
useEventListenerfrom VueUse ensures automatic cleanup of the event listener when the component unmounts, preventing memory leaks.Based on learnings and coding guidelines.
| // Listen for model upload events and refetch data | ||
| if (typeof window !== 'undefined') { | ||
| useEventListener(window, 'model-upload-success', async (event: Event) => { | ||
| const currentNodeType = toValue(nodeType) | ||
| if (!currentNodeType) return | ||
|
|
||
| const customEvent = event as CustomEvent<{ modelType: string }> | ||
| const uploadedModelType = customEvent.detail?.modelType | ||
|
|
||
| // Only refetch if the uploaded model type matches this widget's category | ||
| if (uploadedModelType && uploadedModelType === category.value) { | ||
| await assetsStore.updateModelsForNodeType(currentNodeType) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Add error handling for async store update.
The event listener correctly filters by category to minimize unnecessary API calls. However, the call to assetsStore.updateModelsForNodeType is not wrapped in error handling.
Apply this diff to add error handling:
// Listen for model upload events and refetch data
if (typeof window !== 'undefined') {
useEventListener(window, 'model-upload-success', async (event: Event) => {
const currentNodeType = toValue(nodeType)
if (!currentNodeType) return
const customEvent = event as CustomEvent<{ modelType: string }>
const uploadedModelType = customEvent.detail?.modelType
// Only refetch if the uploaded model type matches this widget's category
if (uploadedModelType && uploadedModelType === category.value) {
- await assetsStore.updateModelsForNodeType(currentNodeType)
+ try {
+ await assetsStore.updateModelsForNodeType(currentNodeType)
+ } catch (error) {
+ console.error('Failed to refresh models after upload:', error)
+ }
}
})
}As per coding guidelines: "Implement proper error handling"
🤖 Prompt for AI Agents
In src/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData.ts
around lines 84 to 98, the async call to assetsStore.updateModelsForNodeType
inside the model-upload-success event listener lacks error handling; wrap the
await call in a try/catch, and in the catch block log the error with contextual
information (including currentNodeType and category.value) so failures are
visible and safe to ignore for the UI; ensure the listener remains async and
does not rethrow the error.
Replaced window event-based cache invalidation with direct Pinia store method calls for better type safety and encapsulation. Changes: - Added refreshModelsByType() to assetsStore to invalidate all node types for a given model category - Upload wizard now calls store method directly instead of dispatching window events - Removed event listener from useAssetWidgetData - no longer needed Benefits: - Better type safety (no CustomEvent casting) - More testable (no global window dependency) - Clearer data flow and dependencies 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/platform/assets/composables/useUploadModelWizard.ts(3 hunks)src/stores/assetsStore.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/platform/assets/composables/useUploadModelWizard.tssrc/stores/assetsStore.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/platform/assets/composables/useUploadModelWizard.tssrc/stores/assetsStore.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/platform/assets/composables/useUploadModelWizard.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/platform/assets/composables/useUploadModelWizard.tssrc/stores/assetsStore.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/platform/assets/composables/useUploadModelWizard.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/assets/composables/useUploadModelWizard.tssrc/stores/assetsStore.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/platform/assets/composables/useUploadModelWizard.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
src/platform/assets/composables/useUploadModelWizard.tssrc/stores/assetsStore.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
src/platform/assets/composables/useUploadModelWizard.tssrc/stores/assetsStore.ts
**/composables/**/use*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables with
useXyz.tspattern
Files:
src/platform/assets/composables/useUploadModelWizard.ts
src/**/stores/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/stores/**/*.{ts,tsx}: Maintain clear public interfaces and restrict extension access in stores
Use TypeScript for type safety in state management stores
Files:
src/stores/assetsStore.ts
**/stores/**/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia stores with
*Store.tspattern
Files:
src/stores/assetsStore.ts
🧬 Code graph analysis (2)
src/platform/assets/composables/useUploadModelWizard.ts (1)
src/stores/assetsStore.ts (1)
useAssetsStore(95-398)
src/stores/assetsStore.ts (1)
src/stores/modelToNodeStore.ts (1)
useModelToNodeStore(22-168)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: collect
🔇 Additional comments (1)
src/platform/assets/composables/useUploadModelWizard.ts (1)
7-8: UsinguseAssetsStoreinside the composable looks appropriateInstantiating the assets store within
useUploadModelWizardto trigger model cache refresh is a clean way to reuse centralized cache logic without additional wiring. No lifecycle cleanup is needed since Pinia stores are app‑scoped.Also applies to: 22-22
Moved model cache refresh logic from assetsStore to useUploadModelWizard to avoid tight coupling between stores. The caller now orchestrates the refresh by using both stores directly. Benefits: - Eliminates store-to-store coupling - Each store maintains single responsibility - Clearer ownership of orchestration logic 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary
Model selection dropdowns now automatically refresh after uploading a new model, ensuring users see newly uploaded models immediately.
Changes
modelToNodeStoreandassetsStoreCheckpointLoaderSimplebut notLoraLoader)Technical Details
After successful upload,
useUploadModelWizard:modelToNodeStoreassetsStore.updateModelsForNodeType()for each affected node typeReview Focus