From 82509e6b5380abd40212c041264f51b081124433 Mon Sep 17 00:00:00 2001 From: DrJKL Date: Wed, 7 Jan 2026 12:33:22 -0800 Subject: [PATCH 1/6] feat: Use store cache for AssetBrowserModal with stale-while-revalidate pattern Amp-Thread-ID: https://ampcode.com/threads/T-019b99d4-72f2-71fe-84d6-7c4f4847a47c Co-authored-by: Amp --- .../components/AssetBrowserModal.test.ts | 84 +++++++++------- .../assets/components/AssetBrowserModal.vue | 97 ++++++++++--------- src/stores/assetsStore.ts | 87 +++++++++++------ 3 files changed, 154 insertions(+), 114 deletions(-) diff --git a/src/platform/assets/components/AssetBrowserModal.test.ts b/src/platform/assets/components/AssetBrowserModal.test.ts index 756dee2628b..d18ad3746ac 100644 --- a/src/platform/assets/components/AssetBrowserModal.test.ts +++ b/src/platform/assets/components/AssetBrowserModal.test.ts @@ -5,18 +5,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import AssetBrowserModal from '@/platform/assets/components/AssetBrowserModal.vue' import type { AssetItem } from '@/platform/assets/schemas/assetSchema' -const mockAssetService = vi.hoisted(() => ({ - getAssetsForNodeType: vi.fn(), - getAssetsByTag: vi.fn(), - getAssetDetails: vi.fn((id: string) => - Promise.resolve({ - id, - name: 'Test Model', - user_metadata: { - filename: 'Test Model' - } - }) - ) +const mockAssetsStore = vi.hoisted(() => ({ + modelAssetsByNodeType: new Map(), + modelLoadingByNodeType: new Map(), + updateModelsForNodeType: vi.fn() })) vi.mock('@/i18n', () => ({ @@ -25,8 +17,8 @@ vi.mock('@/i18n', () => ({ d: (date: Date) => date.toLocaleDateString() })) -vi.mock('@/platform/assets/services/assetService', () => ({ - assetService: mockAssetService +vi.mock('@/stores/assetsStore', () => ({ + useAssetsStore: () => mockAssetsStore })) vi.mock('@/stores/modelToNodeStore', () => ({ @@ -191,8 +183,9 @@ describe('AssetBrowserModal', () => { } beforeEach(() => { - mockAssetService.getAssetsForNodeType.mockReset() - mockAssetService.getAssetsByTag.mockReset() + mockAssetsStore.modelAssetsByNodeType.clear() + mockAssetsStore.modelLoadingByNodeType.clear() + mockAssetsStore.updateModelsForNodeType.mockReset() }) describe('Integration with useAssetBrowser', () => { @@ -201,7 +194,10 @@ describe('AssetBrowserModal', () => { createTestAsset('asset1', 'Model A', 'checkpoints'), createTestAsset('asset2', 'Model B', 'loras') ] - mockAssetService.getAssetsForNodeType.mockResolvedValueOnce(assets) + mockAssetsStore.modelAssetsByNodeType.set( + 'CheckpointLoaderSimple', + assets + ) const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' }) await flushPromises() @@ -218,7 +214,10 @@ describe('AssetBrowserModal', () => { createTestAsset('c1', 'model.safetensors', 'checkpoints'), createTestAsset('l1', 'lora.pt', 'loras') ] - mockAssetService.getAssetsForNodeType.mockResolvedValueOnce(assets) + mockAssetsStore.modelAssetsByNodeType.set( + 'CheckpointLoaderSimple', + assets + ) const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple', @@ -234,31 +233,39 @@ describe('AssetBrowserModal', () => { }) describe('Data fetching', () => { - it('fetches assets for node type', async () => { - mockAssetService.getAssetsForNodeType.mockResolvedValueOnce([]) - + it('triggers store refresh for node type on mount', async () => { createWrapper({ nodeType: 'CheckpointLoaderSimple' }) await flushPromises() - expect(mockAssetService.getAssetsForNodeType).toHaveBeenCalledWith( + expect(mockAssetsStore.updateModelsForNodeType).toHaveBeenCalledWith( 'CheckpointLoaderSimple' ) }) - it('fetches assets for tag when node type not provided', async () => { - mockAssetService.getAssetsByTag.mockResolvedValueOnce([]) + it('displays cached assets immediately from store', async () => { + const assets = [createTestAsset('asset1', 'Cached Model', 'checkpoints')] + mockAssetsStore.modelAssetsByNodeType.set( + 'CheckpointLoaderSimple', + assets + ) - createWrapper({ assetType: 'loras' }) - await flushPromises() + const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' }) + + const assetGrid = wrapper.findComponent({ name: 'AssetGrid' }) + const gridAssets = assetGrid.props('assets') as AssetItem[] - expect(mockAssetService.getAssetsByTag).toHaveBeenCalledWith('loras') + expect(gridAssets).toHaveLength(1) + expect(gridAssets[0].name).toBe('Cached Model') }) }) describe('Asset Selection', () => { it('emits asset-select event when asset is selected', async () => { const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')] - mockAssetService.getAssetsForNodeType.mockResolvedValueOnce(assets) + mockAssetsStore.modelAssetsByNodeType.set( + 'CheckpointLoaderSimple', + assets + ) const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' }) await flushPromises() @@ -271,7 +278,10 @@ describe('AssetBrowserModal', () => { it('executes onSelect callback when provided', async () => { const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')] - mockAssetService.getAssetsForNodeType.mockResolvedValueOnce(assets) + mockAssetsStore.modelAssetsByNodeType.set( + 'CheckpointLoaderSimple', + assets + ) const onSelect = vi.fn() const wrapper = createWrapper({ @@ -289,8 +299,6 @@ describe('AssetBrowserModal', () => { describe('Left Panel Conditional Logic', () => { it('hides left panel by default when showLeftPanel is undefined', async () => { - mockAssetService.getAssetsForNodeType.mockResolvedValueOnce([]) - const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' }) await flushPromises() @@ -299,8 +307,6 @@ describe('AssetBrowserModal', () => { }) it('shows left panel when showLeftPanel prop is explicitly true', async () => { - mockAssetService.getAssetsForNodeType.mockResolvedValueOnce([]) - const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple', showLeftPanel: true @@ -318,7 +324,10 @@ describe('AssetBrowserModal', () => { createTestAsset('asset1', 'Model A', 'checkpoints'), createTestAsset('asset2', 'Model B', 'loras') ] - mockAssetService.getAssetsForNodeType.mockResolvedValueOnce(assets) + mockAssetsStore.modelAssetsByNodeType.set( + 'CheckpointLoaderSimple', + assets + ) const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple', @@ -339,8 +348,6 @@ describe('AssetBrowserModal', () => { describe('Title Management', () => { it('passes custom title to BaseModalLayout when title prop provided', async () => { - mockAssetService.getAssetsForNodeType.mockResolvedValueOnce([]) - const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple', title: 'Custom Title' @@ -353,7 +360,10 @@ describe('AssetBrowserModal', () => { it('passes computed contentTitle to BaseModalLayout when no title prop', async () => { const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')] - mockAssetService.getAssetsForNodeType.mockResolvedValueOnce(assets) + mockAssetsStore.modelAssetsByNodeType.set( + 'CheckpointLoaderSimple', + assets + ) const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' }) await flushPromises() diff --git a/src/platform/assets/components/AssetBrowserModal.vue b/src/platform/assets/components/AssetBrowserModal.vue index cd89591aa41..f7dfb1a0218 100644 --- a/src/platform/assets/components/AssetBrowserModal.vue +++ b/src/platform/assets/components/AssetBrowserModal.vue @@ -63,12 +63,8 @@ diff --git a/src/stores/assetsStore.ts b/src/stores/assetsStore.ts index b75b4540098..3e38a6585ca 100644 --- a/src/stores/assetsStore.ts +++ b/src/stores/assetsStore.ts @@ -1,4 +1,5 @@ import { useAsyncState } from '@vueuse/core' +import { isEqual } from 'es-toolkit' import { defineStore } from 'pinia' import { computed, shallowReactive, ref, watch } from 'vue' import { @@ -280,58 +281,79 @@ export const useAssetsStore = defineStore('assets', () => { ) /** - * Fetch and cache model assets for a specific node type - * Uses VueUse's useAsyncState for automatic loading/error tracking - * @param nodeType The node type to fetch assets for (e.g., 'CheckpointLoaderSimple') - * @returns Promise resolving to the fetched assets + * Internal helper to fetch and cache assets with a given key and fetcher */ - async function updateModelsForNodeType( - nodeType: string + async function updateModelsForKey( + key: string, + fetcher: () => Promise ): Promise { - if (!stateByNodeType.has(nodeType)) { + if (!stateByNodeType.has(key)) { stateByNodeType.set( - nodeType, - useAsyncState( - () => assetService.getAssetsForNodeType(nodeType), - [], - { - immediate: false, - resetOnExecute: false, - onError: (err) => { - console.error( - `Error fetching model assets for ${nodeType}:`, - err - ) - } + key, + useAsyncState(fetcher, [], { + immediate: false, + resetOnExecute: false, + onError: (err) => { + console.error(`Error fetching model assets for ${key}:`, err) } - ) + }) ) } - const state = stateByNodeType.get(nodeType)! + const state = stateByNodeType.get(key)! - modelLoadingByNodeType.set(nodeType, true) - modelErrorByNodeType.set(nodeType, null) + modelLoadingByNodeType.set(key, true) + modelErrorByNodeType.set(key, null) try { await state.execute() const assets = state.state.value - modelAssetsByNodeType.set(nodeType, assets) + const existingAssets = modelAssetsByNodeType.get(key) + + // Only update if assets changed to prevent unnecessary re-renders + if (!isEqual(existingAssets, assets)) { + modelAssetsByNodeType.set(key, assets) + } + modelErrorByNodeType.set( - nodeType, + key, state.error.value instanceof Error ? state.error.value : null ) return assets } finally { - modelLoadingByNodeType.set(nodeType, state.isLoading.value) + modelLoadingByNodeType.set(key, state.isLoading.value) } } + /** + * Fetch and cache model assets for a specific node type + * @param nodeType The node type to fetch assets for (e.g., 'CheckpointLoaderSimple') + * @returns Promise resolving to the fetched assets + */ + async function updateModelsForNodeType( + nodeType: string + ): Promise { + return updateModelsForKey(nodeType, () => + assetService.getAssetsForNodeType(nodeType) + ) + } + + /** + * Fetch and cache model assets for a specific tag + * @param tag The tag to fetch assets for (e.g., 'models') + * @returns Promise resolving to the fetched assets + */ + async function updateModelsForTag(tag: string): Promise { + const key = `tag:${tag}` + return updateModelsForKey(key, () => assetService.getAssetsByTag(tag)) + } + return { modelAssetsByNodeType, modelLoadingByNodeType, modelErrorByNodeType, - updateModelsForNodeType + updateModelsForNodeType, + updateModelsForTag } } @@ -339,7 +361,8 @@ export const useAssetsStore = defineStore('assets', () => { modelAssetsByNodeType: shallowReactive(new Map()), modelLoadingByNodeType: shallowReactive(new Map()), modelErrorByNodeType: shallowReactive(new Map()), - updateModelsForNodeType: async () => [] + updateModelsForNodeType: async () => [], + updateModelsForTag: async () => [] } } @@ -347,7 +370,8 @@ export const useAssetsStore = defineStore('assets', () => { modelAssetsByNodeType, modelLoadingByNodeType, modelErrorByNodeType, - updateModelsForNodeType + updateModelsForNodeType, + updateModelsForTag } = getModelState() // Watch for completed downloads and refresh model caches @@ -403,6 +427,7 @@ export const useAssetsStore = defineStore('assets', () => { modelAssetsByNodeType, modelLoadingByNodeType, modelErrorByNodeType, - updateModelsForNodeType + updateModelsForNodeType, + updateModelsForTag } }) From 14321575330e2f2943c5798a89b2bb446e931dd0 Mon Sep 17 00:00:00 2001 From: DrJKL Date: Wed, 7 Jan 2026 12:57:43 -0800 Subject: [PATCH 2/6] refactor: Simplify AssetBrowserModal and useModelUpload --- .../components/AssetBrowserModal.test.ts | 85 ++++++++++--------- .../assets/components/AssetBrowserModal.vue | 21 +---- .../assets/composables/useModelUpload.ts | 6 +- 3 files changed, 50 insertions(+), 62 deletions(-) diff --git a/src/platform/assets/components/AssetBrowserModal.test.ts b/src/platform/assets/components/AssetBrowserModal.test.ts index d18ad3746ac..339ad65a76b 100644 --- a/src/platform/assets/components/AssetBrowserModal.test.ts +++ b/src/platform/assets/components/AssetBrowserModal.test.ts @@ -4,12 +4,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import AssetBrowserModal from '@/platform/assets/components/AssetBrowserModal.vue' import type { AssetItem } from '@/platform/assets/schemas/assetSchema' - -const mockAssetsStore = vi.hoisted(() => ({ - modelAssetsByNodeType: new Map(), - modelLoadingByNodeType: new Map(), - updateModelsForNodeType: vi.fn() -})) +import { useAssetsStore } from '@/stores/assetsStore' vi.mock('@/i18n', () => ({ t: (key: string, params?: Record) => @@ -17,9 +12,15 @@ vi.mock('@/i18n', () => ({ d: (date: Date) => date.toLocaleDateString() })) -vi.mock('@/stores/assetsStore', () => ({ - useAssetsStore: () => mockAssetsStore -})) +vi.mock('@/stores/assetsStore', () => { + const store = { + modelAssetsByNodeType: new Map(), + modelLoadingByNodeType: new Map(), + updateModelsForNodeType: vi.fn(), + updateModelsForTag: vi.fn() + } + return { useAssetsStore: () => store } +}) vi.mock('@/stores/modelToNodeStore', () => ({ useModelToNodeStore: () => ({ @@ -182,10 +183,12 @@ describe('AssetBrowserModal', () => { }) } + const mockStore = useAssetsStore() + beforeEach(() => { - mockAssetsStore.modelAssetsByNodeType.clear() - mockAssetsStore.modelLoadingByNodeType.clear() - mockAssetsStore.updateModelsForNodeType.mockReset() + mockStore.modelAssetsByNodeType.clear() + mockStore.modelLoadingByNodeType.clear() + vi.clearAllMocks() }) describe('Integration with useAssetBrowser', () => { @@ -194,10 +197,7 @@ describe('AssetBrowserModal', () => { createTestAsset('asset1', 'Model A', 'checkpoints'), createTestAsset('asset2', 'Model B', 'loras') ] - mockAssetsStore.modelAssetsByNodeType.set( - 'CheckpointLoaderSimple', - assets - ) + mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets) const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' }) await flushPromises() @@ -214,10 +214,7 @@ describe('AssetBrowserModal', () => { createTestAsset('c1', 'model.safetensors', 'checkpoints'), createTestAsset('l1', 'lora.pt', 'loras') ] - mockAssetsStore.modelAssetsByNodeType.set( - 'CheckpointLoaderSimple', - assets - ) + mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets) const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple', @@ -237,17 +234,14 @@ describe('AssetBrowserModal', () => { createWrapper({ nodeType: 'CheckpointLoaderSimple' }) await flushPromises() - expect(mockAssetsStore.updateModelsForNodeType).toHaveBeenCalledWith( + expect(mockStore.updateModelsForNodeType).toHaveBeenCalledWith( 'CheckpointLoaderSimple' ) }) it('displays cached assets immediately from store', async () => { const assets = [createTestAsset('asset1', 'Cached Model', 'checkpoints')] - mockAssetsStore.modelAssetsByNodeType.set( - 'CheckpointLoaderSimple', - assets - ) + mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets) const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' }) @@ -257,15 +251,33 @@ describe('AssetBrowserModal', () => { expect(gridAssets).toHaveLength(1) expect(gridAssets[0].name).toBe('Cached Model') }) + + it('triggers store refresh for asset type (tag) on mount', async () => { + createWrapper({ assetType: 'models' }) + await flushPromises() + + expect(mockStore.updateModelsForTag).toHaveBeenCalledWith('models') + }) + + it('uses tag: prefix for cache key when assetType is provided', async () => { + const assets = [createTestAsset('asset1', 'Tagged Model', 'models')] + mockStore.modelAssetsByNodeType.set('tag:models', assets) + + const wrapper = createWrapper({ assetType: 'models' }) + await flushPromises() + + const assetGrid = wrapper.findComponent({ name: 'AssetGrid' }) + const gridAssets = assetGrid.props('assets') as AssetItem[] + + expect(gridAssets).toHaveLength(1) + expect(gridAssets[0].name).toBe('Tagged Model') + }) }) describe('Asset Selection', () => { it('emits asset-select event when asset is selected', async () => { const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')] - mockAssetsStore.modelAssetsByNodeType.set( - 'CheckpointLoaderSimple', - assets - ) + mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets) const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' }) await flushPromises() @@ -278,10 +290,7 @@ describe('AssetBrowserModal', () => { it('executes onSelect callback when provided', async () => { const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')] - mockAssetsStore.modelAssetsByNodeType.set( - 'CheckpointLoaderSimple', - assets - ) + mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets) const onSelect = vi.fn() const wrapper = createWrapper({ @@ -324,10 +333,7 @@ describe('AssetBrowserModal', () => { createTestAsset('asset1', 'Model A', 'checkpoints'), createTestAsset('asset2', 'Model B', 'loras') ] - mockAssetsStore.modelAssetsByNodeType.set( - 'CheckpointLoaderSimple', - assets - ) + mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets) const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple', @@ -360,10 +366,7 @@ describe('AssetBrowserModal', () => { it('passes computed contentTitle to BaseModalLayout when no title prop', async () => { const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')] - mockAssetsStore.modelAssetsByNodeType.set( - 'CheckpointLoaderSimple', - assets - ) + mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets) const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' }) await flushPromises() diff --git a/src/platform/assets/components/AssetBrowserModal.vue b/src/platform/assets/components/AssetBrowserModal.vue index f7dfb1a0218..1d9eda2d36c 100644 --- a/src/platform/assets/components/AssetBrowserModal.vue +++ b/src/platform/assets/components/AssetBrowserModal.vue @@ -64,7 +64,7 @@