refactor: address code review feedback for media assets pagination

- Use shallowReactive for loadedIds Set to fix memory leak (High)
- Optimize sorting with insertion-based approach instead of full sort (Medium)
- Fix race condition in loadMoreHistory with atomic guard (Low)
- Add consistent error handling to preserve data on failures (Medium)
- Implement 300ms debounce for scroll pagination handler (Medium)
- Make IAssetsProvider pagination properties required (Low)
- Add offset validation in fetchHistoryV1 and V2 (Medium)

Author: viva-jinyi

src/components/sidebar/tabs/AssetsSidebarTab.vue

@@ -148,6 +148,7 @@
 </template>
 
 <script setup lang="ts">
+import { useDebounceFn } from '@vueuse/core'
 import ProgressSpinner from 'primevue/progressspinner'
 import { useToast } from 'primevue/usetoast'
 import { computed, onMounted, onUnmounted, ref, watch } from 'vue'
@@ -398,15 +399,14 @@ const handleDeleteSelected = async () => {
   clearSelection()
 }
 
-const handleApproachEnd = async () => {
+const handleApproachEnd = useDebounceFn(async () => {
   if (
     activeTab.value === 'output' &&
     !isInFolderView.value &&
-    outputAssets.loadMore &&
-    outputAssets.hasMore?.value &&
-    !outputAssets.isLoadingMore?.value
+    outputAssets.hasMore.value &&
+    !outputAssets.isLoadingMore.value
   ) {
     await outputAssets.loadMore()
   }
-}
+}, 300)
 </script>

src/platform/assets/composables/media/IAssetsProvider.ts

@@ -30,15 +30,15 @@ export interface IAssetsProvider {
   /**
    * Load more items (for pagination)
    */
-  loadMore?: () => Promise<void>
+  loadMore: () => Promise<void>
 
   /**
    * Whether there are more items to load
    */
-  hasMore?: Ref<boolean>
+  hasMore: Ref<boolean>
 
   /**
    * Whether currently loading more items
    */
-  isLoadingMore?: Ref<boolean>
+  isLoadingMore: Ref<boolean>
 }

src/platform/remote/comfyui/history/fetchers/fetchHistoryV1.ts

@@ -15,13 +15,22 @@ import type {
  * Fetches history from V1 API endpoint
  * @param api - API instance with fetchApi method
  * @param maxItems - Maximum number of history items to fetch
+ * @param offset - Offset for pagination (must be non-negative integer)
  * @returns Promise resolving to V1 history response
+ * @throws Error if offset is invalid (negative or non-integer)
  */
 export async function fetchHistoryV1(
   fetchApi: (url: string) => Promise<Response>,
   maxItems: number = 200,
   offset?: number
 ): Promise<HistoryV1Response> {
+  // Validate offset parameter
+  if (offset !== undefined && (offset < 0 || !Number.isInteger(offset))) {
+    throw new Error(
+      `Invalid offset parameter: ${offset}. Must be a non-negative integer.`
+    )
+  }
+
   let url = `/history?max_items=${maxItems}`
   if (offset !== undefined) {
     url += `&offset=${offset}`

src/platform/remote/comfyui/history/fetchers/fetchHistoryV2.ts

@@ -14,13 +14,22 @@ import type { HistoryResponseV2 } from '../types/historyV2Types'
  * Fetches history from V2 API endpoint and adapts to V1 format
  * @param fetchApi - API instance with fetchApi method
  * @param maxItems - Maximum number of history items to fetch
+ * @param offset - Offset for pagination (must be non-negative integer)
  * @returns Promise resolving to V1 history response (adapted from V2)
+ * @throws Error if offset is invalid (negative or non-integer)
  */
 export async function fetchHistoryV2(
   fetchApi: (url: string) => Promise<Response>,
   maxItems: number = 200,
   offset?: number
 ): Promise<HistoryV1Response> {
+  // Validate offset parameter
+  if (offset !== undefined && (offset < 0 || !Number.isInteger(offset))) {
+    throw new Error(
+      `Invalid offset parameter: ${offset}. Must be a non-negative integer.`
+    )
+  }
+
   let url = `/history_v2?max_items=${maxItems}`
   if (offset !== undefined) {
     url += `&offset=${offset}`

src/stores/assetsStore.ts

@@ -100,7 +100,7 @@ export const useAssetsStore = defineStore('assets', () => {
 
   const allHistoryItems = ref<AssetItem[]>([])
 
-  const loadedIds = new Set<string>()
+  const loadedIds = shallowReactive(new Set<string>())
 
   const fetchInputFiles = isCloud
     ? fetchInputFilesFromCloud
@@ -141,22 +141,27 @@ export const useAssetsStore = defineStore('assets', () => {
     const newAssets = mapHistoryToAssets(history.History)
 
     if (loadMore) {
-      // Filter out duplicates using Set
-      const uniqueAssets = newAssets.filter((asset) => {
+      // Filter out duplicates and insert in sorted order
+      for (const asset of newAssets) {
         if (loadedIds.has(asset.id)) {
-          return false // Already loaded
+          continue // Skip duplicates
         }
         loadedIds.add(asset.id)
-        return true
-      })
 
-      allHistoryItems.value.push(...uniqueAssets)
-
-      // Sort by date (newest first)
-      allHistoryItems.value.sort(
-        (a, b) =>
-          new Date(b.created_at).getTime() - new Date(a.created_at).getTime()
-      )
+        // Find insertion index to maintain sorted order (newest first)
+        const assetTime = new Date(asset.created_at).getTime()
+        const insertIndex = allHistoryItems.value.findIndex(
+          (item) => new Date(item.created_at).getTime() < assetTime
+        )
+
+        if (insertIndex === -1) {
+          // Asset is oldest, append to end
+          allHistoryItems.value.push(asset)
+        } else {
+          // Insert at the correct position
+          allHistoryItems.value.splice(insertIndex, 0, asset)
+        }
+      }
     } else {
       // Initial load: replace all
       allHistoryItems.value = newAssets
@@ -207,11 +212,8 @@ export const useAssetsStore = defineStore('assets', () => {
    * Load more history items (infinite scroll)
    */
   const loadMoreHistory = async () => {
-    // Guard: check if more items available
-    if (!hasMoreHistory.value) return
-
-    // Guard: prevent concurrent loads
-    if (isLoadingMore.value) return
+    // Guard: prevent concurrent loads and check if more items available
+    if (!hasMoreHistory.value || isLoadingMore.value) return
 
     isLoadingMore.value = true
     historyError.value = null
@@ -222,6 +224,10 @@ export const useAssetsStore = defineStore('assets', () => {
     } catch (err) {
       console.error('Error loading more history:', err)
       historyError.value = err
+      // Keep existing data when error occurs (consistent with updateHistory)
+      if (!historyAssets.value.length) {
+        historyAssets.value = []
+      }
     } finally {
       isLoadingMore.value = false
     }