[feat] Add ownership filter to model browser #7201
Conversation
luke-mino-altherr
requested review from
a team,
KarryCharon,
Yorha4D and
shinshin86
as code owners
dosubot
bot
added
the
size:L
📝 WalkthroughWalkthroughAdds an ownership filter end-to-end: new i18n keys, ownership UI in AssetFilterBar (shown when mutable assets exist), ownership filtering in useAssetBrowser, fixture support for asset immutability, AssetBrowserModal passing full asset list, and updated tests covering ownership behavior. Changes
sequenceDiagram
participant User
participant AssetFilterBar
participant AssetBrowserModal
participant useAssetBrowser
participant AssetStore
User->>AssetFilterBar: select ownership option
AssetFilterBar->>AssetBrowserModal: emit filterChange { ownership, sortBy, fileFormats, baseModels }
AssetBrowserModal->>useAssetBrowser: updateFilters({ ownership, ... })
useAssetBrowser->>AssetStore: read assets (fetched/all)
useAssetBrowser->>useAssetBrowser: apply base-model, file-format, then ownership filters
useAssetBrowser-->>AssetBrowserModal: return filteredAssets
AssetBrowserModal-->>User: render filtered asset list
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🧠 Learnings (1)📚 Learning: 2025-12-09T04:35:40.491ZApplied to 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). (1)
🔇 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 |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/09/2025, 08:39:04 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/09/2025, 09:35:43 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.22 MB (baseline 3.22 MB) • 🔴 +125 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 988 kB (baseline 986 kB) • 🔴 +1.64 kBGraph 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 — 176 kB (baseline 176 kB) • ⚪ 0 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 — 3.18 kB (baseline 3.18 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
248-255: FilterState updates are consistent; align sortBy value with implementationIncluding
ownership: 'all'in the variousupdateFilterscalls keeps these tests in sync with the extendedFilterStateand with the composable’s default behavior.One small clean‑up: in the “sorts assets by name” test you still use
sortBy: 'name', whileuseAssetBrowser’s switch handles'name-asc' | 'name-desc' | 'recent' | 'popular'and falls back to'name-asc'for unknown values. To avoid relying on the fallback branch, consider updating that test to use'name-asc'explicitly.Also applies to: 285-290, 340-345, 365-370
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/locales/en/main.json(1 hunks)src/platform/assets/components/AssetFilterBar.vue(5 hunks)src/platform/assets/composables/useAssetBrowser.ts(3 hunks)src/platform/assets/fixtures/ui-mock-assets.ts(1 hunks)tests-ui/platform/assets/components/AssetFilterBar.test.ts(1 hunks)tests-ui/platform/assets/composables/useAssetBrowser.test.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tssrc/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/composables/useAssetBrowser.test.tssrc/platform/assets/composables/useAssetBrowser.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:
tests-ui/platform/assets/components/AssetFilterBar.test.tssrc/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/composables/useAssetBrowser.test.tssrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Avoid writing change detector tests that just assert default values
Avoid writing tests dependent on non-behavioral features like utility classes or styles
Avoid writing redundant tests
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
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/fixtures/ui-mock-assets.tssrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
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/fixtures/ui-mock-assets.tssrc/platform/assets/composables/useAssetBrowser.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/fixtures/ui-mock-assets.tssrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/locales/**/*.json
📄 CodeRabbit inference engine (AGENTS.md)
Place new i18n translation entries in
src/locales/en/main.jsonand usevue-i18nin Composition API for string literals
Files:
src/locales/en/main.json
**/composables/**/use*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables with
useXyz.tspattern
Files:
tests-ui/platform/assets/composables/useAssetBrowser.test.tssrc/platform/assets/composables/useAssetBrowser.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/useAssetBrowser.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/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
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/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.vue: Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Use setup() function for component logic
Utilize ref and reactive for reactive state
Implement computed properties with computed()
Use watch and watchEffect for side effects
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection
Use vue 3.5 style of default prop declaration
Use Tailwind CSS for styling
Implement proper props and emits definitions
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventions
Files:
src/platform/assets/components/AssetFilterBar.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
**/*.vue: Use Vue 3 SFCs with Composition API only (use<script setup lang="ts">, not Options API)
Avoid using<style>blocks; use Tailwind 4 for all styling
UsedefinePropswith TypeScript style default declaration; do not usewithDefaultsor runtime props declaration
PreferuseModelover separately defining a prop and emit
Usecomputedinstead of arefandwatchif possible
Avoid usingrefif a prop would accomplish the design goals; avoid usingcomputedif arefor prop directly would work
Do not import Vue macros unnecessarily
Never use thedark:Tailwind variant; use semantic values from thestyle.csstheme instead (e.g.,bg-node-component-surface)
Never use:class="[]"to merge class names; always usecn()from@/utils/tailwindUtil(e.g.,<div :class="cn('text-node-component-header-icon', hasError && 'text-danger')" />)
Leverage VueUse functions for performance-enhancing styles
Avoid new usage of PrimeVue components
Use Vue 3 Teleport component when needed
Use Vue 3 Suspense for async components
Files:
src/platform/assets/components/AssetFilterBar.vue
**/components/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g.,
MenuHamburger.vue)
Files:
src/platform/assets/components/AssetFilterBar.vue
🧠 Learnings (16)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Check assets/ directory for test data when writing tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.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 **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.test.ts : Avoid writing change detector tests that just assert default values
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.test.ts : Avoid writing tests dependent on non-behavioral features like utility classes or styles
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 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 setup() function in Vue 3 Composition API
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 : Avoid using `ref` if a prop would accomplish the design goals; avoid using `computed` if a `ref` or prop directly would work
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 setup() function for component logic
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 : Replace PrimeVue Dropdown component with Select
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 : Replace PrimeVue Chips component with AutoComplete with multiple enabled
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 Vue 3 SFCs with Composition API only (use `<script setup lang="ts">`, not Options API)
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
🧬 Code graph analysis (3)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
src/platform/assets/fixtures/ui-mock-assets.ts (1)
createAssetWithSpecificExtension(149-159)
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
src/platform/assets/composables/useAssetBrowser.ts (1)
useAssetBrowser(67-225)
src/platform/assets/composables/useAssetBrowser.ts (1)
src/platform/assets/schemas/assetSchema.ts (1)
AssetItem(73-73)
⏰ 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). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (5)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (2)
251-295: Ownership filter visibility tests match the intended UXThese tests correctly assert that the ownership control is:
- Hidden when all assets are immutable,
- Shown when any asset is mutable (including mixed sets).
Using the
label === 'assetBrowser.ownership'filter makes the assertions robust against additionalSingleSelectinstances.
298-337: Good coverage of ownership emissions; consider extending the structure testThe new tests validate that:
- Changing the ownership selector emits
filterChangewith the expectedownershipvalue.ownershipdefaults to'all'when other filters (e.g., sort) change.That’s solid coverage. As a small improvement, you could extend the existing “ensures FilterState interface compliance” test to also assert that
typeof filterState.ownership === 'string'(and possibly that it’s defined), so it stays aligned when FilterState evolves.⛔ Skipped due to learnings
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 **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressionstests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
381-463: Ownership filter behavior is well-covered in the composable testsThe new tests for:
'all'(no filtering),'my-models'(!is_immutable),'public-models'(is_immutable),nicely validate the core ownership logic in
useAssetBrowser. They directly exercise the newownershipfield inFilterStateand confirm that the pipeline returns the expected asset counts and predicates.src/platform/assets/components/AssetFilterBar.vue (1)
30-38: Ownership selector UI is wired correctlyThe new
SingleSelectfor ownership:
- Is gated by
hasMutableAssets, matching the requirement to show it only when mutable assets exist.- Uses the new
assetBrowser.ownershiplabel andownershipOptions.- Hooks into
v-model="ownership"and@update:model-value="handleFilterChange"in line with the existing filters.This is a good, minimal addition to the filter bar.
src/locales/en/main.json (1)
2139-2142: Ownership i18n keys correctly added and namedThe new
assetBrowser.ownership*keys are well‑named, concise, and match their usage inAssetFilterBar.vueand the ownership options. No issues here.
luke-mino-altherr
force-pushed
the
feat/model-modal-filter-by-ownership
branch
from
1dfba42 to
03791f7
Compare
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/platform/assets/composables/useAssetBrowser.ts (1)
38-45: Consider using stricter typing for the ownership parameter.The filter logic is correct and handles
undefinedvalues properly with strict equality. However, theownershipparameter is typed asstringrather than using the narrower type fromFilterState['ownership'].Apply this diff for improved type safety:
-function filterByOwnership(ownership: string) { +function filterByOwnership(ownership: FilterState['ownership']) { return (asset: AssetItem) => { if (ownership === 'all') return true if (ownership === 'my-models') return asset.is_immutable === false if (ownership === 'public-models') return asset.is_immutable === true return true } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
src/locales/en/main.json(1 hunks)src/platform/assets/components/AssetBrowserModal.vue(1 hunks)src/platform/assets/components/AssetFilterBar.vue(4 hunks)src/platform/assets/composables/useAssetBrowser.ts(3 hunks)src/platform/assets/fixtures/ui-mock-assets.ts(1 hunks)tests-ui/platform/assets/components/AssetFilterBar.test.ts(2 hunks)tests-ui/platform/assets/composables/useAssetBrowser.test.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
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/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
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/fixtures/ui-mock-assets.tssrc/platform/assets/composables/useAssetBrowser.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/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
src/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/components/AssetFilterBar.test.tssrc/platform/assets/composables/useAssetBrowser.tstests-ui/platform/assets/composables/useAssetBrowser.test.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/fixtures/ui-mock-assets.tstests-ui/platform/assets/components/AssetFilterBar.test.tssrc/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vuetests-ui/platform/assets/composables/useAssetBrowser.test.ts
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Avoid writing change detector tests that just assert default values
Avoid writing tests dependent on non-behavioral features like utility classes or styles
Avoid writing redundant tests
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
src/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.vue: Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Use setup() function for component logic
Utilize ref and reactive for reactive state
Implement computed properties with computed()
Use watch and watchEffect for side effects
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection
Use vue 3.5 style of default prop declaration
Use Tailwind CSS for styling
Implement proper props and emits definitions
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventions
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/components/AssetFilterBar.vue
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/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
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/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
**/*.vue: Use Vue 3 SFCs with Composition API only (use<script setup lang="ts">, not Options API)
Avoid using<style>blocks; use Tailwind 4 for all styling
UsedefinePropswith TypeScript style default declaration; do not usewithDefaultsor runtime props declaration
PreferuseModelover separately defining a prop and emit
Usecomputedinstead of arefandwatchif possible
Avoid usingrefif a prop would accomplish the design goals; avoid usingcomputedif arefor prop directly would work
Do not import Vue macros unnecessarily
Never use thedark:Tailwind variant; use semantic values from thestyle.csstheme instead (e.g.,bg-node-component-surface)
Never use:class="[]"to merge class names; always usecn()from@/utils/tailwindUtil(e.g.,<div :class="cn('text-node-component-header-icon', hasError && 'text-danger')" />)
Leverage VueUse functions for performance-enhancing styles
Avoid new usage of PrimeVue components
Use Vue 3 Teleport component when needed
Use Vue 3 Suspense for async components
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/components/AssetFilterBar.vue
**/components/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g.,
MenuHamburger.vue)
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/components/AssetFilterBar.vue
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/useAssetBrowser.ts
**/composables/**/use*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables with
useXyz.tspattern
Files:
src/platform/assets/composables/useAssetBrowser.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
src/locales/**/*.json
📄 CodeRabbit inference engine (AGENTS.md)
Place new i18n translation entries in
src/locales/en/main.jsonand usevue-i18nin Composition API for string literals
Files:
src/locales/en/main.json
🧠 Learnings (15)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Check assets/ directory for test data when writing tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 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 : Replace PrimeVue Dropdown component with Select
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 setup() function in Vue 3 Composition API
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 : Replace PrimeVue Chips component with AutoComplete with multiple enabled
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 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 Vue 3 SFCs with Composition API only (use `<script setup lang="ts">`, not Options API)
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 setup() function for component logic
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 : Avoid using `ref` if a prop would accomplish the design goals; avoid using `computed` if a `ref` or prop directly would work
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
🧬 Code graph analysis (3)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
src/platform/assets/fixtures/ui-mock-assets.ts (1)
createAssetWithSpecificExtension(149-159)
src/platform/assets/composables/useAssetBrowser.ts (1)
src/platform/assets/schemas/assetSchema.ts (1)
AssetItem(73-73)
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
src/platform/assets/composables/useAssetBrowser.ts (1)
useAssetBrowser(67-225)
⏰ 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: test
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (18)
src/platform/assets/fixtures/ui-mock-assets.ts (1)
149-159: LGTM!The optional
isImmutableparameter cleanly extends the fixture helper to support ownership-related tests while maintaining backward compatibility with existing call sites.src/platform/assets/components/AssetBrowserModal.vue (1)
49-53: LGTM!Correctly passes the full asset set to
AssetFilterBarviaall-assets, enabling the ownership filter visibility check to operate on all fetched assets rather than just the category-filtered subset.src/locales/en/main.json (1)
2139-2142: LGTM!New i18n keys for the ownership filter are appropriately placed in the
assetBrowsersection and follow the existing camelCase naming convention.src/platform/assets/composables/useAssetBrowser.ts (2)
74-79: LGTM!The filter state correctly initializes
ownershipto'all', ensuring all models are shown by default as specified in the PR objectives.
185-190: LGTM!The ownership filter is correctly integrated into the filtering pipeline, applied after file format and base model filters, maintaining consistency with the existing filter chain pattern.
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (4)
381-405: Test coverage for 'all' ownership is correct.The test properly verifies that when ownership is set to
'all', all assets are returned regardless of theiris_immutablestate.
407-434: Good assertion pattern for 'my-models' filter.The test correctly verifies both the count and that all returned assets have
is_immutable === false(mutable assets).
436-463: Good assertion pattern for 'public-models' filter.The test correctly verifies both the count and that all returned assets have
is_immutable === true(immutable/public assets).
249-254: LGTM!Existing tests correctly updated to include the new
ownershipfield in filter updates.tests-ui/platform/assets/components/AssetFilterBar.test.ts (3)
253-266: LGTM!The test correctly verifies that the ownership filter is hidden when all assets are immutable.
268-297: LGTM!These tests provide good coverage for ownership filter visibility with both mutable-only and mixed asset scenarios.
300-340: Comprehensive ownership filter behavior coverage.These tests properly verify that ownership changes are emitted and that the default value is 'all'. The non-null assertion concern mentioned in the earlier comment also applies here.
src/platform/assets/components/AssetFilterBar.vue (6)
30-38: LGTM!The ownership filter UI follows the established pattern for SingleSelect components and correctly uses conditional rendering based on
hasMutableAssets.
59-59: LGTM!The
computedimport is correctly added and necessary for thehasMutableAssetscomputed property.
78-86: LGTM!The ownership options follow the same pattern as
SORT_OPTIONS, providing excellent type safety with the derivedOwnershipOptionliteral union type.
88-93: LGTM!The
FilterStateinterface correctly typesownershipasOwnershipOptionrather than a plain string, providing type safety for consumers. This addresses the past review feedback appropriately.
95-110: LGTM!The implementation correctly:
- Adds
allAssetsprop for visibility logic (separate from filteredassets)- Initializes
ownershipto 'all' as the default- Uses strict comparison (
=== false) inhasMutableAssetsto treat undefined/missingis_immutableas immutableThis addresses the past review feedback about explicit mutability checks.
120-121: LGTM!The
handleFilterChangefunction correctly includesownershipin the emitted filter state, maintaining consistency with the other filter fields.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests-ui/platform/assets/components/AssetFilterBar.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.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:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Avoid writing change detector tests that just assert default values
Avoid writing tests dependent on non-behavioral features like utility classes or styles
Avoid writing redundant tests
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
🧠 Learnings (10)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Check assets/ directory for test data when writing tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.test.ts : Avoid writing change detector tests that just assert default values
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.test.ts : Avoid writing tests dependent on non-behavioral features like utility classes or styles
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid using ts-expect-error; use proper TypeScript types instead
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.test.ts : Avoid writing redundant tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.{ts,vue} : Never use `as any` type assertions; fix the underlying type issue instead
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
🧬 Code graph analysis (1)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
src/platform/assets/fixtures/ui-mock-assets.ts (1)
createAssetWithSpecificExtension(149-159)
⏰ 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)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
108-112: Sort select lookup pattern looks solidFinding the
SingleSelectby itslabeland asserting its existence before emitting keeps the test robust against component order changes and avoids cryptic failures. The remaining non‑null assertion is acceptable here given the explicitexpect.
DrJKL
force-pushed
the
feat/model-modal-filter-by-ownership
branch
from
3d12904 to
7abb8c3
Compare
| "sortRecent": "Recent", | ||
| "sortPopular": "Popular", | ||
| "ownership": "Ownership", | ||
| "ownershipAll": "All", |
There was a problem hiding this comment.
For consistency, how about All models?
There was a problem hiding this comment.
i tried this suggestion and it actually visually looks a bit verbose. What we have here also matches what design requested
| v-model="ownership" | ||
| :label="$t('assetBrowser.ownership')" | ||
| :options="ownershipOptions" | ||
| class="min-w-42" |
There was a problem hiding this comment.
Just noting for now, this could be either a lot of space or not very much depending on the locale.
There was a problem hiding this comment.
interesting!
| } | ||
| } | ||
|
|
||
| function filterByOwnership(ownership: string) { |
There was a problem hiding this comment.
If you pull the types/options out of the component, you could use the same value types here.
There was a problem hiding this comment.
good catch
| .findAllComponents({ name: 'SingleSelect' }) | ||
| .find((component) => component.props('label') === 'assetBrowser.sortBy') | ||
| expect(sortSelect).toBeTruthy() | ||
| await sortSelect!.vm.$emit('update:modelValue', 'popular') |
There was a problem hiding this comment.
There was a problem hiding this comment.
Can we do the test as if we were a user?
There was a problem hiding this comment.
(sidenote: Thank you for the tests)
| const ownershipSelects = wrapper | ||
| .findAllComponents({ name: 'SingleSelect' }) | ||
| .filter( | ||
| (component) => component.props('label') === 'assetBrowser.ownership' |
There was a problem hiding this comment.
For all the places with this same find, how do you feel about extracting a function, then getting it by a user facing or a11y property?
luke-mino-altherr
force-pushed
the
feat/model-modal-filter-by-ownership
branch
from
7abb8c3 to
37cbd99
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
223-293: Ownership visibility & behavior tests are solid; consider aligning sort test value with actual optionsThe new conditional-visibility and ownership tests correctly cover:
- Hidden file/base-model filters when no options.
- Ownership filter hidden with no mutable assets, and shown for mutable/mixed sets.
- Emission of
filterChangewithownershipand defaulting to'all'.One small consistency tweak: in the “handles multiple simultaneous filter changes” test,
sortByis set to'popular', butSORT_OPTIONSinAssetFilterBar.vueonly exposes'recent' | 'name-asc' | 'name-desc'. For clarity, consider either:
- Switching the test to use one of the actual option values (e.g.
'name-asc'), or- Reintroducing a
'popular'sort option inSORT_OPTIONSif that mode is still intended to be user-selectable.This keeps tests and UI behavior in lockstep.
Also applies to: 296-333
♻️ Duplicate comments (1)
src/platform/assets/fixtures/ui-mock-assets.ts (1)
149-157: Optionally tighten types oncreateAssetWithSpecificExtensionFunctionality is fine, but you can get slightly stronger typing by reusing the schema type and annotating the return:
-export function createAssetWithSpecificExtension( - extension: string, - isImmutable?: boolean -) { +export function createAssetWithSpecificExtension( + extension: string, + isImmutable?: AssetItem['is_immutable'] +): AssetItem { const asset = createMockAssets(1)[0] asset.name = `test-model.${extension}` if (isImmutable !== undefined) { asset.is_immutable = isImmutable } return asset }This keeps fixtures aligned with
AssetItemwhile preserving current behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
src/locales/en/main.json(1 hunks)src/platform/assets/components/AssetBrowserModal.vue(1 hunks)src/platform/assets/components/AssetFilterBar.vue(4 hunks)src/platform/assets/composables/useAssetBrowser.ts(4 hunks)src/platform/assets/fixtures/ui-mock-assets.ts(1 hunks)tests-ui/platform/assets/components/AssetFilterBar.test.ts(7 hunks)tests-ui/platform/assets/composables/useAssetBrowser.test.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
src/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.vue: Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Use setup() function for component logic
Utilize ref and reactive for reactive state
Implement computed properties with computed()
Use watch and watchEffect for side effects
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection
Use vue 3.5 style of default prop declaration
Use Tailwind CSS for styling
Implement proper props and emits definitions
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventions
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/components/AssetFilterBar.vue
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/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetFilterBar.vue
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/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetFilterBar.vue
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/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetFilterBar.vue
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/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
**/*.vue: Use Vue 3 SFCs with Composition API only (use<script setup lang="ts">, not Options API)
Avoid using<style>blocks; use Tailwind 4 for all styling
UsedefinePropswith TypeScript style default declaration; do not usewithDefaultsor runtime props declaration
PreferuseModelover separately defining a prop and emit
Usecomputedinstead of arefandwatchif possible
Avoid usingrefif a prop would accomplish the design goals; avoid usingcomputedif arefor prop directly would work
Do not import Vue macros unnecessarily
Never use thedark:Tailwind variant; use semantic values from thestyle.csstheme instead (e.g.,bg-node-component-surface)
Never use:class="[]"to merge class names; always usecn()from@/utils/tailwindUtil(e.g.,<div :class="cn('text-node-component-header-icon', hasError && 'text-danger')" />)
Leverage VueUse functions for performance-enhancing styles
Avoid new usage of PrimeVue components
Use Vue 3 Teleport component when needed
Use Vue 3 Suspense for async components
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/components/AssetFilterBar.vue
**/*.{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/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.tssrc/platform/assets/components/AssetFilterBar.vue
**/components/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g.,
MenuHamburger.vue)
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/components/AssetFilterBar.vue
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/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.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/useAssetBrowser.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
src/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
**/composables/**/use*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables with
useXyz.tspattern
Files:
src/platform/assets/composables/useAssetBrowser.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
src/locales/**/*.json
📄 CodeRabbit inference engine (AGENTS.md)
Place new i18n translation entries in
src/locales/en/main.jsonand usevue-i18nin Composition API for string literals
Files:
src/locales/en/main.json
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Avoid writing change detector tests that just assert default values
Avoid writing tests dependent on non-behavioral features like utility classes or styles
Avoid writing redundant tests
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
🧠 Learnings (26)
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Check assets/ directory for test data when writing tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Use existing test utilities rather than writing custom utilities
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.test.ts : Avoid writing tests dependent on non-behavioral features like utility classes or styles
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.test.ts : Avoid writing change detector tests that just assert default values
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Check tests-ui/README.md for test guidelines
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid using ts-expect-error; use proper TypeScript types instead
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 **/*.test.ts : Avoid writing redundant tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Always prefer vitest mock functions over writing verbose manual mocks
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Mocks should be cleanly written and easy to understand, with reusable mocks where possible
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 defineExpose only for imperative operations (such as form.validate(), modal.open())
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Mock external dependencies in tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 src/**/*.test.ts : Write unit and component tests with Vitest using happy-dom
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 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 Vue 3 SFCs with Composition API only (use `<script setup lang="ts">`, not Options API)
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 : Replace PrimeVue Dropdown component with Select
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 : Extract complex conditionals to computed properties
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 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 : Avoid using `ref` if a prop would accomplish the design goals; avoid using `computed` if a `ref` or prop directly would work
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 setup() function in Vue 3 Composition API
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
🧬 Code graph analysis (3)
src/platform/assets/composables/useAssetBrowser.ts (1)
src/platform/assets/schemas/assetSchema.ts (1)
AssetItem(73-73)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
src/platform/assets/fixtures/ui-mock-assets.ts (2)
createAssetWithoutBaseModel(137-141)createAssetWithSpecificExtension(149-159)
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
src/platform/assets/composables/useAssetBrowser.ts (1)
useAssetBrowser(69-227)
⏰ 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: test
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (5)
src/locales/en/main.json (1)
2140-2143: Ownership i18n keys are well-scoped and consistentThe new
assetBrowser.ownership*strings are clearly named, localized in the right namespace, and match the ownership filter UI usage. No issues here.src/platform/assets/components/AssetBrowserModal.vue (1)
48-53: Correctly passes full asset list for ownership visibilityBinding
:all-assets="fetchedAssets"while keepingassets="categoryFilteredAssets"cleanly enables the ownership filter to reason over the full set of assets without changing existing category/file/base-model filtering behavior.src/platform/assets/composables/useAssetBrowser.ts (1)
14-15: Ownership filtering is correctly typed and integrated into the pipeline
OwnershipOptionprovides a precise union for ownership values,filterByOwnershipuses strictis_immutable === false/truechecks, and the ownership filter is applied after file-format/base-model filters with a default of'all'. The composable’s public API stays aligned with the UI via the shared types.Also applies to: 40-47, 76-81, 187-192
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
248-255: Ownership filter tests exercise all key branches and updated APIThe test updates to include
ownershipinupdateFilterspayloads, plus the dedicated “Ownership filtering” suite for'all' | 'my-models' | 'public-models', give good coverage of the new behavior and validate the strictis_immutablesemantics end to end.Also applies to: 284-291, 340-345, 365-370, 382-465
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
76-96: Selector helpers improve readability and test robustnessUsing helper functions that locate filters via
data-component-idmakes the tests less brittle than relying on component order or internals, and better reflects how a user “sees” the UI structure.
luke-mino-altherr
force-pushed
the
feat/model-modal-filter-by-ownership
branch
from
73ee63b to
b4fb295
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
340-355: Consider adding explicit emitted length assertion.Per the previous review feedback, adding an explicit length assertion would make test failures clearer and verify the expected number of emissions:
const emitted = wrapper.emitted('filterChange') + expect(emitted).toHaveLength(1) const filterState = emitted![0][0] as FilterState expect(filterState.ownership).toBe('all')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests-ui/platform/assets/components/AssetFilterBar.test.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
**/*.{js,ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively (no new JavaScript)
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Do not useanytype in TypeScript code
Do not useas anytype assertions in TypeScript code; fix the underlying type issue
Style formatting: 2 space indent, single quotes, no trailing semicolons, 80 character width
Import statements should be sorted and grouped by plugin; runpnpm formatbefore committing
ESLint rules: no floating promises, no unused imports, i18n raw text restrictions in templates
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Write unit and component tests in**/*.test.ts
Use Vitest with happy-dom for unit and component tests
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Use vue-i18n in Composition API for string literals and place new translation entries in
src/locales/en/main.json
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
🧠 Learnings (17)
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Check assets/ directory for test data when writing tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Use existing test utilities rather than writing custom utilities
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Check tests-ui/README.md for test guidelines
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid using ts-expect-error; use proper TypeScript types instead
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Tests should be cross-platform compatible using `path.resolve`, `path.join`, and `path.sep` for Windows, macOS, and Linux compatibility
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Do not write tests dependent on non-behavioral features like utility classes or styles
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Do not write change detector tests that only assert default values without testing behavior
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Always prefer vitest mock functions over writing verbose manual mocks
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Favor pure functions (especially testable ones)
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Mocks should be cleanly written and easy to understand, with reusable mocks where possible
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Avoid writing redundant tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 defineExpose only for imperative operations (such as form.validate(), modal.open())
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Applies to **/*.vue : Use `provide`/`inject` for dependency injection only when simpler alternatives (Store or composable) are not suitable
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
🧬 Code graph analysis (1)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
src/platform/assets/fixtures/ui-mock-assets.ts (3)
createAssetWithSpecificExtension(149-159)createAssetWithSpecificBaseModel(161-165)createAssetWithoutBaseModel(137-141)
⏰ 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: test
- GitHub Check: setup
- GitHub Check: collect
- GitHub Check: lint-and-format
🔇 Additional comments (4)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (4)
14-62: LGTM!The mock setup is clean and appropriately minimal. Using native
<select>elements in the mocks enables DOM-based interaction testing, which aligns well with test-utils best practices for testing user-facing behavior rather than implementation details.
76-95: LGTM!The helper functions using
data-component-idselectors are a solid approach that addresses the previous reviewer feedback about extracting reusable selectors. This makes tests more resilient to component restructuring and aligns with testing best practices of selecting by user-facing or accessibility attributes.
189-228: LGTM!These tests effectively verify that filter options are dynamically generated from asset data and properly sorted alphabetically. The approach of mapping option elements to verify both name and value is thorough.
272-315: LGTM!The ownership filter visibility tests comprehensively cover all scenarios: immutable-only assets (hidden), mutable assets (shown), mixed assets (shown), and the
allAssetsprop integration. This addresses the previous review feedback aboutallAssetsusage consistency.
| const finalState: FilterState = emitted![ | ||
| emitted!.length - 1 | ||
| ][0] as FilterState | ||
| expect(finalState.fileFormats).toEqual(['ckpt', 'safetensors']) | ||
| expect(finalState.baseModels).toEqual(['sdxl']) | ||
| expect(finalState.sortBy).toBe('popular') | ||
| expect(finalState.sortBy).toBe('name-desc') |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider verifying ownership in the final state for completeness.
Since ownership is now part of FilterState, this test for "multiple simultaneous filter changes" could also assert the ownership field in the final state to ensure full interface compliance.
expect(finalState.fileFormats).toEqual(['ckpt', 'safetensors'])
expect(finalState.baseModels).toEqual(['sdxl'])
expect(finalState.sortBy).toBe('name-desc')
+ expect(finalState.ownership).toBe('all')📝 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.
| const finalState: FilterState = emitted![ | |
| emitted!.length - 1 | |
| ][0] as FilterState | |
| expect(finalState.fileFormats).toEqual(['ckpt', 'safetensors']) | |
| expect(finalState.baseModels).toEqual(['sdxl']) | |
| expect(finalState.sortBy).toBe('popular') | |
| expect(finalState.sortBy).toBe('name-desc') | |
| const finalState: FilterState = emitted![ | |
| emitted!.length - 1 | |
| ][0] as FilterState | |
| expect(finalState.fileFormats).toEqual(['ckpt', 'safetensors']) | |
| expect(finalState.baseModels).toEqual(['sdxl']) | |
| expect(finalState.sortBy).toBe('name-desc') | |
| expect(finalState.ownership).toBe('all') |
🤖 Prompt for AI Agents
In tests-ui/platform/assets/components/AssetFilterBar.test.ts around lines 147
to 152, the finalState assertions omit the new ownership field; update the test
to also assert the expected ownership value on finalState (e.g.,
expect(finalState.ownership).toBe(/* expected ownership value */) or
toEqual([...]) matching how ownership is represented) so the test verifies the
complete FilterState interface after multiple simultaneous filter changes.
| describe('Ownership Filter', () => { | ||
| it('emits ownership filter changes', async () => { | ||
| const assets = [ | ||
| createAssetWithSpecificExtension('safetensors', false) // mutable | ||
| ] | ||
| const wrapper = mountAssetFilterBar({ assets }) | ||
|
|
||
| const ownershipSelect = findOwnershipFilter(wrapper) | ||
| expect(ownershipSelect.exists()).toBe(true) | ||
|
|
||
| const ownershipSelectElement = ownershipSelect.find('select') | ||
| ownershipSelectElement.element.value = 'my-models' | ||
| await ownershipSelectElement.trigger('change') | ||
| await nextTick() | ||
|
|
||
| const emitted = wrapper.emitted('filterChange') | ||
| expect(emitted).toBeTruthy() | ||
|
|
||
| const filterState = emitted![emitted!.length - 1][0] as FilterState | ||
| expect(filterState.ownership).toBe('my-models') | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding test for 'public-models' ownership option.
The PR summary mentions three ownership options: "All", "My models", and "Public models". While 'all' and 'my-models' are tested, 'public-models' is not explicitly verified. Consider adding a test case for completeness:
it('emits public-models ownership filter', async () => {
const assets = [
createAssetWithSpecificExtension('safetensors', false) // mutable
]
const wrapper = mountAssetFilterBar({ assets })
const ownershipSelect = findOwnershipFilter(wrapper)
expect(ownershipSelect.exists()).toBe(true)
const ownershipSelectElement = ownershipSelect.find('select')
ownershipSelectElement.element.value = 'public-models'
await ownershipSelectElement.trigger('change')
await nextTick()
const emitted = wrapper.emitted('filterChange')
expect(emitted).toBeTruthy()
const filterState = emitted![emitted!.length - 1][0] as FilterState
expect(filterState.ownership).toBe('public-models')
})🤖 Prompt for AI Agents
In tests-ui/platform/assets/components/AssetFilterBar.test.ts around lines 318
to 338, add a sibling test that mirrors the existing 'my-models' ownership test
but sets the ownership select value to 'public-models'; mount the component with
a sample asset, locate the ownership select, set its select element value to
'public-models', trigger the change and await nextTick, then assert that a
'filterChange' event was emitted and that the emitted FilterState.ownership
equals 'public-models'.
christian-byrne
added
the
needs-backport
|
Seems to be a merge conflicts on the locales after we did a release. |
We should sort those JSON. |
Add dropdown to filter models by ownership (All/My models/Public models). The filter only appears when user has uploaded models. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Change FilterState.ownership to use OwnershipOption literal union type - Use strict comparison (=== false/true) for is_immutable checks - Move type definitions before interface to follow dependency order 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Use findAllComponents with label filter to specifically find the sort dropdown instead of the first SingleSelect, which could be the ownership filter when mutable assets are present. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Pass allAssets to AssetFilterBar to check for mutable assets across all categories, not just the currently filtered ones. This ensures the ownership filter remains visible when switching categories, providing better UX consistency. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add explicit toBeTruthy() checks before using non-null assertions to provide clear error messages when components aren't found - Move ownership filtering tests from 'Sorting' to dedicated 'Ownership filtering' describe block for better organization 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The ownership filter visibility depends on the allAssets prop to determine if mutable assets exist. Updated test cases to pass both assets and allAssets props to properly test the ownership filter functionality.
- Add helper functions to find filters by data-component-id attributes - Replace direct vm.$emit calls with actual DOM interactions - Trigger change events on select elements instead of emitting directly - Simplify visibility checks using .exists() instead of array length - Check rendered options in DOM instead of component props This follows Vue Test Utils best practices of testing from a user's perspective rather than testing implementation details. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add missing asset fixtures for ckpt and sdxl options - Use find() instead of array indexing to locate specific options - Check last emission instead of expecting exact count 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The ownership filter now checks allAssets first, but falls back to assets when allAssets is empty. This allows the filter to work in contexts where allAssets is not provided but assets are available. - Update hasMutableAssets computed to use fallback logic - Update tests to verify both scenarios (with and without allAssets) - Add test case for allAssets override behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The test now checks for at least 3 emissions instead of exactly 3, since the ownership filter may emit an additional change when shown. Uses the last emission to verify final state. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Changed from 'popular' to 'name-desc' which is a valid sort option. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
luke-mino-altherr
force-pushed
the
feat/model-modal-filter-by-ownership
branch
from
ce44553 to
6470c04
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
src/locales/en/main.json(1 hunks)src/platform/assets/components/AssetBrowserModal.vue(1 hunks)src/platform/assets/components/AssetFilterBar.vue(4 hunks)src/platform/assets/composables/useAssetBrowser.ts(4 hunks)src/platform/assets/fixtures/ui-mock-assets.ts(1 hunks)tests-ui/platform/assets/components/AssetFilterBar.test.ts(6 hunks)tests-ui/platform/assets/composables/useAssetBrowser.test.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/platform/assets/composables/useAssetBrowser.test.tstests-ui/platform/assets/components/AssetFilterBar.test.ts
**/*.{js,ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively (no new JavaScript)
Files:
tests-ui/platform/assets/composables/useAssetBrowser.test.tssrc/platform/assets/components/AssetFilterBar.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/components/AssetFilterBar.test.tssrc/platform/assets/components/AssetBrowserModal.vue
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Do not useanytype in TypeScript code
Do not useas anytype assertions in TypeScript code; fix the underlying type issue
Style formatting: 2 space indent, single quotes, no trailing semicolons, 80 character width
Import statements should be sorted and grouped by plugin; runpnpm formatbefore committing
ESLint rules: no floating promises, no unused imports, i18n raw text restrictions in templates
Files:
tests-ui/platform/assets/composables/useAssetBrowser.test.tssrc/platform/assets/components/AssetFilterBar.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/components/AssetFilterBar.test.tssrc/platform/assets/components/AssetBrowserModal.vue
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Write unit and component tests in**/*.test.ts
Use Vitest with happy-dom for unit and component tests
Files:
tests-ui/platform/assets/composables/useAssetBrowser.test.tstests-ui/platform/assets/components/AssetFilterBar.test.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Use vue-i18n in Composition API for string literals and place new translation entries in
src/locales/en/main.json
Files:
tests-ui/platform/assets/composables/useAssetBrowser.test.tssrc/platform/assets/components/AssetFilterBar.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/components/AssetFilterBar.test.tssrc/platform/assets/components/AssetBrowserModal.vue
src/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.vue: Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Use setup() function for component logic
Utilize ref and reactive for reactive state
Implement computed properties with computed()
Use watch and watchEffect for side effects
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection
Use vue 3.5 style of default prop declaration
Use Tailwind CSS for styling
Implement proper props and emits definitions
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventions
Files:
src/platform/assets/components/AssetFilterBar.vuesrc/platform/assets/components/AssetBrowserModal.vue
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/components/AssetFilterBar.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetBrowserModal.vue
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
src/**/*.{ts,tsx,vue}: Write expressive and self-documenting code to reduce the need for comments; clean redundant comments as you go
Consider if there is a simpler way to introduce functionality before writing code; choose the simpler approach when possible
Use refactoring to make complex code simpler
Keep functions short and functional
Minimize nesting in code (e.g.,if () { ... }orfor () { ... }) to avoid arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Favor pure functions (especially testable ones)
Watch out for code smells and refactor to avoid them
Implement proper error handling in code
If a complex type definition is inlined in multiple related places, extract and name it for reuse
Files:
src/platform/assets/components/AssetFilterBar.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetBrowserModal.vue
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/components/AssetFilterBar.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetBrowserModal.vue
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/assets/components/AssetFilterBar.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetBrowserModal.vue
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/components/AssetFilterBar.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetBrowserModal.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
**/*.vue: Use Vue 3 SFCs with Composition API only (.vuefiles)
Use Tailwind 4 styling and avoid<style>blocks in Vue components
Use<script setup lang="ts">for component logic in Vue components
Use Vue 3.5 TypeScript style of default prop declaration with reactive props destructuring
Do not usewithDefaultsor runtime props declaration in Vue components
PreferuseModelto separately defining a prop and emit in Vue components
Usereffor reactive state in Vue components
Implement computed properties withcomputed()instead of usingrefandwatch
UsewatchandwatchEffectfor side effects in Vue components
Useprovide/injectfor dependency injection only when simpler alternatives (Store or composable) are not suitable
Do not use thedark:Tailwind variant; use semantic values fromstyle.csstheme instead (e.g.,bg-node-component-surface)
Useimport { cn } from '@/utils/tailwindUtil'to merge class names instead of:class="[]"
Avoid new usage of PrimeVue components
Use VueUse functions for performance-enhancing styles in Vue components
Do not import Vue macros unnecessarily in components
Be judicious with addition of new refs or other state; prefer using props or composables when possible
Do not add acomputedif it's possible to use arefor prop directly
Do not use awatchif acomputedwould work instead
Implement proper props and emits definitions in Vue components
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventions
Files:
src/platform/assets/components/AssetFilterBar.vuesrc/platform/assets/components/AssetBrowserModal.vue
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
src/**/*.ts: Minimize the surface area (exported values) of each module and composable
Use es-toolkit for utility functions
Files:
src/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.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/useAssetBrowser.ts
🧠 Learnings (32)
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Check assets/ directory for test data when writing tests
Applied to files:
tests-ui/platform/assets/composables/useAssetBrowser.test.tstests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/platform/assets/composables/useAssetBrowser.test.tstests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
tests-ui/platform/assets/composables/useAssetBrowser.test.tssrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid mutable state; prefer immutability and assignment at point of declaration
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Applies to **/*.vue : Be judicious with addition of new refs or other state; prefer using props or composables when possible
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Applies to **/*.vue : Use `ref` for reactive state in Vue components
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Applies to **/*.vue : Use `<script setup lang="ts">` for component logic in Vue components
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Applies to **/*.vue : Implement computed properties with `computed()` instead of using `ref` and `watch`
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Applies to **/*.vue : Do not add a `computed` if it's possible to use a `ref` or prop directly
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 : Replace PrimeVue Dropdown component with Select
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 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 : Extract complex conditionals to computed properties
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-09T03:49:52.828Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/platform/updates/components/WhatsNewPopup.vue:5-13
Timestamp: 2025-12-09T03:49:52.828Z
Learning: In Vue files across the ComfyUI_frontend repo, when a button is needed, prefer the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) over plain HTML <button> elements. These components wrap PrimeVue with the project’s design system styling. Use only the common button components for consistency and theming, and import them from src/components/button/ as needed.
Applied to files:
src/platform/assets/components/AssetFilterBar.vuesrc/platform/assets/components/AssetBrowserModal.vue
📚 Learning: 2025-12-09T04:35:40.491Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/locales/en/main.json:774-780
Timestamp: 2025-12-09T04:35:40.491Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, locale files other than `src/locales/en/main.json` are generated automatically on every release. Developers only need to add English (en) key/values in `src/locales/en/main.json` when making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.
Applied to files:
src/locales/en/main.json
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/{components,composables}/**/*.{ts,tsx,vue} : Use vue-i18n for ALL user-facing strings by adding them to `src/locales/en/main.json`
Applied to files:
src/locales/en/main.json
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Use existing test utilities rather than writing custom utilities
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Check tests-ui/README.md for test guidelines
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid using ts-expect-error; use proper TypeScript types instead
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Tests should be cross-platform compatible using `path.resolve`, `path.join`, and `path.sep` for Windows, macOS, and Linux compatibility
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Applies to **/*.{ts,tsx,vue} : Do not use `as any` type assertions in TypeScript code; fix the underlying type issue
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Do not write tests dependent on non-behavioral features like utility classes or styles
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Do not write change detector tests that only assert default values without testing behavior
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Always prefer vitest mock functions over writing verbose manual mocks
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Favor pure functions (especially testable ones)
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Mocks should be cleanly written and easy to understand, with reusable mocks where possible
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Avoid writing redundant tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.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 defineExpose only for imperative operations (such as form.validate(), modal.open())
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-09T02:44:38.234Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:44:38.234Z
Learning: Applies to **/*.vue : Use `provide`/`inject` for dependency injection only when simpler alternatives (Store or composable) are not suitable
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
🧬 Code graph analysis (3)
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
src/platform/assets/composables/useAssetBrowser.ts (1)
useAssetBrowser(69-227)
src/platform/assets/composables/useAssetBrowser.ts (1)
src/platform/assets/schemas/assetSchema.ts (1)
AssetItem(73-73)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
src/platform/assets/fixtures/ui-mock-assets.ts (3)
createAssetWithSpecificExtension(149-159)createAssetWithSpecificBaseModel(161-165)createAssetWithoutBaseModel(137-141)
⏰ 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). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: collect
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
🔇 Additional comments (8)
src/platform/assets/components/AssetBrowserModal.vue (1)
48-53: All-assets binding correctly wires modal into new ownership logicPassing
:all-assets="fetchedAssets"ensures AssetFilterBar can compute ownership visibility from the full asset set while:assets="categoryFilteredAssets"continues to drive the visible grid. The ref is safely unwrapped in templates, so this integration looks correct and non-breaking.src/platform/assets/composables/useAssetBrowser.ts (1)
14-15: Ownership filter implementation is coherent and type-safeThe introduction of
OwnershipOption,filterByOwnership, the'all'default infilters, and the extra filter stage infilteredAssetsall line up cleanly with the UI’s ownership options and the AssetFilterBarFilterState. Usingasset.is_immutable === false/truegives explicit semantics for missing flags and avoids the earlier ambiguity around!asset.is_immutable.Also applies to: 40-47, 76-81, 187-192
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
248-255: Ownership filter tests thoroughly validate new filter dimensionExtending
updateFilterspayloads withownershipand adding the dedicated “Ownership filtering” tests gives good coverage for'all','my-models', and'public-models'cases againstis_immutable. The expectations on lengths and invariants (!asset.is_immutablevsasset.is_immutable) match the composable’s logic and help guard future regressions.Also applies to: 284-295, 340-345, 365-370, 382-466
src/platform/assets/fixtures/ui-mock-assets.ts (1)
149-157: Mutability-aware fixture helper is straightforward and non-breakingAllowing
createAssetWithSpecificExtensionto optionally setasset.is_immutablegives tests an easy way to model owned vs public assets without impacting existing usages. The conditional assignment onisImmutable !== undefinedavoids accidentally overriding fixtures when the flag isn’t passed.tests-ui/platform/assets/components/AssetFilterBar.test.ts (3)
76-96: Helper selectors and DOM-driven option checks align tests with user behaviorThe new
findFileFormatsFilter/findBaseModelsFilter/findOwnershipFilter/findSortFilterhelpers and the switch to inspecting<option>elements on the mocked selects keep tests focused on what the user sees (labels and values) rather than component implementation details. The dynamic options assertions for file formats and base models accurately reflect the expected SelectOption shapes and ordering.Also applies to: 199-223
100-153: Filter state aggregation tests correctly exercise combined filterChange behaviorThe “multiple simultaneous filter changes” and “FilterState interface compliance” tests now drive the component through realistic interactions (multi-selects then sort select) and assert that the final emitted
FilterStatecontains the expectedfileFormats,baseModels, andsortByvalues, with the payload shape matching the updated interface. This gives good confidence that the bar emits a coherent filter state after chained changes.Also applies to: 163-186
236-270: Ownership visibility and emission tests match hasMutableAssets semanticsThe conditional visibility tests for the ownership filter (
no mutable,mutable only,mixed, andallAssetsoverrides) line up with thehasMutableAssetsimplementation (checkingallAssetswhen present, otherwiseassets, foris_immutable === false). The ownership tests then validate that:
- Selecting
'my-models'updates the emittedFilterState.ownership, and- Changing sort leaves
ownershipat its default'all'in the emitted payload.Together these tests tightly cover the new ownership UI behavior and its impact on emitted filter state.
Also applies to: 272-316, 318-355
src/platform/assets/components/AssetFilterBar.vue (1)
30-38: Ownership filter UI and state are well-integrated with existing filter modelThe new ownership SingleSelect,
ownershipOptions, andownershipref are correctly typed viaOwnershipOptionand wired intoFilterState, so emittedfilterChangepayloads stay in sync with the composable.hasMutableAssetsusing(allAssets.length ? allAssets : assets)withasset.is_immutable === falsegives clear, strict semantics for when the ownership filter should appear, both in the modal (withallAssets) and in potential standalone usage. Overall, the ownership dimension is added without complicating the existing filter logic.Also applies to: 59-60, 65-66, 79-83, 85-90, 92-100, 105-108, 114-121
| { name: t('assetBrowser.ownershipAll'), value: 'all' }, | ||
| { name: t('assetBrowser.ownershipMyModels'), value: 'my-models' }, | ||
| { name: t('assetBrowser.ownershipPublicModels'), value: 'public-models' } | ||
| ] |
There was a problem hiding this comment.
Did you get rid of the as const and type inference? 😢
| ) | ||
| } | ||
|
|
||
| function findBaseModelsFilter(wrapper: ReturnType<typeof mountAssetFilterBar>) { |
There was a problem hiding this comment.
InstanceType<AssetFilterBar>?
|
@luke-mino-altherr Backport to Please manually cherry-pick commit Conflicting files
|
## Summary Adds a dropdown filter to the model browser that allows users to filter assets by ownership (All, My models, Public models), based on the `is_immutable` property. ## Changes - **Filter UI**: Added ownership dropdown in [AssetFilterBar.vue](src/platform/assets/components/AssetFilterBar.vue#L30-L38) that only appears when user has uploaded models - **Filter Logic**: Implemented `filterByOwnership` function in [useAssetBrowser.ts](src/platform/assets/composables/useAssetBrowser.ts#L38-L45) to filter by `is_immutable` property - **i18n**: Added translation strings for ownership filter options - **Tests**: Added comprehensive tests for ownership filtering in both composable and component test files ## Review Focus - The ownership filter visibility logic correctly checks for mutable assets (`!is_immutable`) - Default filter value is 'all' to show all models initially - Filter integrates cleanly with existing file format and base model filters 🤖 Generated with [Claude Code](https://claude.com/claude-code) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7201-feat-Add-ownership-filter-to-model-browser-2c16d73d365081f280f6d1e42e5400af) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: GitHub Action <action@github.com>
## Summary Adds a dropdown filter to the model browser that allows users to filter assets by ownership (All, My models, Public models), based on the `is_immutable` property. ## Changes - **Filter UI**: Added ownership dropdown in [AssetFilterBar.vue](src/platform/assets/components/AssetFilterBar.vue#L30-L38) that only appears when user has uploaded models - **Filter Logic**: Implemented `filterByOwnership` function in [useAssetBrowser.ts](src/platform/assets/composables/useAssetBrowser.ts#L38-L45) to filter by `is_immutable` property - **i18n**: Added translation strings for ownership filter options - **Tests**: Added comprehensive tests for ownership filtering in both composable and component test files ## Review Focus - The ownership filter visibility logic correctly checks for mutable assets (`!is_immutable`) - Default filter value is 'all' to show all models initially - Filter integrates cleanly with existing file format and base model filters 🤖 Generated with [Claude Code](https://claude.com/claude-code) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7201-feat-Add-ownership-filter-to-model-browser-2c16d73d365081f280f6d1e42e5400af) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: GitHub Action <action@github.com>
## Summary Adds a dropdown filter to the model browser that allows users to filter assets by ownership (All, My models, Public models), based on the `is_immutable` property. ## Changes - **Filter UI**: Added ownership dropdown in [AssetFilterBar.vue](src/platform/assets/components/AssetFilterBar.vue#L30-L38) that only appears when user has uploaded models - **Filter Logic**: Implemented `filterByOwnership` function in [useAssetBrowser.ts](src/platform/assets/composables/useAssetBrowser.ts#L38-L45) to filter by `is_immutable` property - **i18n**: Added translation strings for ownership filter options - **Tests**: Added comprehensive tests for ownership filtering in both composable and component test files ## Review Focus - The ownership filter visibility logic correctly checks for mutable assets (`!is_immutable`) - Default filter value is 'all' to show all models initially - Filter integrates cleanly with existing file format and base model filters 🤖 Generated with [Claude Code](https://claude.com/claude-code) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7201-feat-Add-ownership-filter-to-model-browser-2c16d73d365081f280f6d1e42e5400af) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: GitHub Action <action@github.com>
Summary
Adds a dropdown filter to the model browser that allows users to filter assets by ownership (All, My models, Public models), based on the
is_immutableproperty.Changes
filterByOwnershipfunction in useAssetBrowser.ts to filter byis_immutablepropertyReview Focus
!is_immutable)🤖 Generated with Claude Code
┆Issue is synchronized with this Notion page by Unito