fix: address review on flat-output in-flight hardening

- Revert unrelated comment-only churn in fetchHistoryAssets; the
  loadedJobIds all-job-dedup mechanism was dead code and the underlying
  "bug #2" does not exist (history pagination advances by offset,
  independent of loadedIds), so no fix is needed there.
- Remove the loadedJobIds tests that passed incidentally on the base
  branch and validated a non-existent mechanism.
- Rename flatOutputGeneration/generation to flatOutputRefreshEpoch/
  capturedEpoch and document the discard invariant.
- Add a reverse-ordering race test proving a stale loadMore that settles
  before a refresh does not pollute seenIds.

Author: mattmillerai

src/stores/assetsStore.test.ts

@@ -774,78 +774,6 @@ describe('assetsStore - Refactored (Option A)', () => {
   })
 })
 
-describe('assetsStore - loadedJobIds (all-job dedup)', () => {
-  let store: ReturnType<typeof useAssetsStore>
-
-  const createFailedJobItem = (id: string): JobListItem => ({
-    id,
-    status: 'failed',
-    create_time: 1000,
-    update_time: 1000,
-    last_state_update: 1000,
-    priority: 1
-  })
-
-  const createDisplayableJobItem = (id: string, index = 0): JobListItem => ({
-    id,
-    status: 'completed',
-    create_time: 1000 + index,
-    update_time: 1000 + index,
-    last_state_update: 1000 + index,
-    priority: 1000 + index,
-    preview_output: {
-      filename: `output_${id}.png`,
-      subfolder: '',
-      type: 'output',
-      nodeId: 'node_1',
-      mediaType: 'images'
-    }
-  })
-
-  beforeEach(() => {
-    setActivePinia(createTestingPinia({ stubActions: false }))
-    store = useAssetsStore()
-    vi.clearAllMocks()
-  })
-
-  it('does not false-negative dedup when the first page is all non-displayable jobs', async () => {
-    const firstBatch = Array.from({ length: 200 }, (_, i) =>
-      createFailedJobItem(`failed_${i}`)
-    )
-    vi.mocked(api.getHistory).mockResolvedValueOnce(firstBatch)
-    await store.updateHistory()
-
-    expect(store.historyAssets).toHaveLength(0)
-
-    const secondBatch = [
-      createFailedJobItem('failed_0'),
-      createDisplayableJobItem('new_job', 0)
-    ]
-    vi.mocked(api.getHistory).mockResolvedValueOnce(secondBatch)
-    await store.loadMoreHistory()
-
-    const ids = store.historyAssets.map((a) => a.id)
-    expect(ids).toContain('new_job')
-    expect(ids).not.toContain('failed_0')
-  })
-
-  it('loadedJobIds is cleared on reset (updateHistory)', async () => {
-    const firstBatch = [createFailedJobItem('job_a')]
-    vi.mocked(api.getHistory).mockResolvedValueOnce(firstBatch)
-    await store.updateHistory()
-
-    const secondBatch = [createDisplayableJobItem('job_a', 0)]
-    vi.mocked(api.getHistory)
-      .mockResolvedValueOnce(secondBatch)
-      .mockResolvedValueOnce([])
-
-    await store.updateHistory()
-
-    expect(store.historyAssets).toHaveLength(1)
-    expect(store.historyAssets[0].id).toBe('job_a')
-  })
-})
-
 describe('assetsStore - Model Assets Cache (Cloud)', () => {
   beforeEach(() => {
     setActivePinia(createTestingPinia({ stubActions: false }))
@@ -1914,6 +1842,57 @@ describe('assetsStore - Flat Output Assets (cloud-only)', () => {
       expect(store.flatOutputAssets.map((a) => a.id)).not.toContain('extra-1')
     })
 
+    it('a stale loadMore that resolves before the refresh does not corrupt seenIds', async () => {
+      const firstPage = Array.from({ length: FLAT_OUTPUT_PAGE_SIZE }, (_, i) =>
+        makeAsset(`a${i}`, `f${i}.png`)
+      )
+      vi.mocked(assetService.getAssetsPageByTag).mockResolvedValueOnce(
+        makePage(firstPage, { hasMore: true })
+      )
+      const store = useAssetsStore()
+      await store.updateFlatOutputs()
+
+      vi.mocked(assetService.getAssetsPageByTag).mockClear()
+
+      let resolveLoadMore!: (page: AssetResponse) => void
+      const loadMorePromise = new Promise<AssetResponse>((res) => {
+        resolveLoadMore = res
+      })
+      let resolveRefresh!: (page: AssetResponse) => void
+      const refreshPromise = new Promise<AssetResponse>((res) => {
+        resolveRefresh = res
+      })
+
+      vi.mocked(assetService.getAssetsPageByTag)
+        .mockReturnValueOnce(loadMorePromise)
+        .mockReturnValueOnce(refreshPromise)
+
+      const loadMoreResult = store.loadMoreFlatOutputs()
+      const refreshResult = store.updateFlatOutputs()
+
+      // The stale loadMore settles first, before the refresh has replaced the
+      // list. Its page must be discarded rather than folded into seenIds.
+      resolveLoadMore(makePage([makeAsset('extra-1', 'extra.png')]))
+      resolveRefresh(
+        makePage([makeAsset('fresh-1', 'fresh.png')], { hasMore: true })
+      )
+      await Promise.all([loadMoreResult, refreshResult])
+
+      expect(store.flatOutputAssets.map((a) => a.id)).toEqual(['fresh-1'])
+
+      // If the discarded loadMore had leaked 'extra-1' into seenIds, this
+      // legitimate next page would be filtered out and never shown.
+      vi.mocked(assetService.getAssetsPageByTag).mockResolvedValueOnce(
+        makePage([makeAsset('extra-1', 'extra.png')])
+      )
+      await store.loadMoreFlatOutputs()
+
+      expect(store.flatOutputAssets.map((a) => a.id)).toEqual([
+        'fresh-1',
+        'extra-1'
+      ])
+    })
+
     it('a second concurrent refresh coalesces into the first refresh promise', async () => {
       let resolvePage!: (page: AssetResponse) => void
       const pagePromise = new Promise<AssetResponse>((res) => {

src/stores/assetsStore.ts

@@ -151,38 +151,46 @@ export const useAssetsStore = defineStore('assets', () => {
    * @param loadMore - true for pagination (append), false for initial load (replace)
    */
   const fetchHistoryAssets = async (loadMore = false): Promise<AssetItem[]> => {
+    // Reset state for initial load
     if (!loadMore) {
       historyOffset.value = 0
       hasMoreHistory.value = true
       allHistoryItems.value = []
       loadedIds.clear()
     }
 
+    // Fetch from server with offset
     const history = await api.getHistory(BATCH_SIZE, {
       offset: historyOffset.value
     })
 
+    // Convert JobListItems to AssetItems
     const newAssets = mapHistoryToAssets(history)
 
     if (loadMore) {
+      // Filter out duplicates and insert in sorted order
       for (const asset of newAssets) {
         if (loadedIds.has(asset.id)) {
-          continue
+          continue // Skip duplicates
         }
         loadedIds.add(asset.id)
 
+        // Find insertion index to maintain sorted order (newest first)
         const assetTime = new Date(asset.created_at ?? 0).getTime()
         const insertIndex = allHistoryItems.value.findIndex(
           (item) => new Date(item.created_at ?? 0).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
       newAssets.forEach((asset) => loadedIds.add(asset.id))
     }
@@ -262,7 +270,9 @@ export const useAssetsStore = defineStore('assets', () => {
   let flatOutputNextCursor: string | undefined
   let flatOutputRefreshInFlight: Promise<AssetItem[]> | null = null
   let flatOutputLoadMoreInFlight: Promise<AssetItem[]> | null = null
-  let flatOutputGeneration = 0
+  // Incremented on each refresh; loadMore results captured from a prior epoch
+  // are discarded so a stale page can't append onto a freshly-refreshed list.
+  let flatOutputRefreshEpoch = 0
 
   async function fetchFlatOutputs(loadMore: boolean): Promise<AssetItem[]> {
     if (loadMore) {
@@ -271,7 +281,7 @@ export const useAssetsStore = defineStore('assets', () => {
       flatOutputIsLoadingMore.value = true
     } else {
       if (flatOutputRefreshInFlight) return flatOutputRefreshInFlight
-      flatOutputGeneration++
+      flatOutputRefreshEpoch++
       flatOutputLoading.value = true
       flatOutputOffset.value = 0
       flatOutputNextCursor = undefined
@@ -280,7 +290,7 @@ export const useAssetsStore = defineStore('assets', () => {
     }
     flatOutputError.value = null
 
-    const generation = flatOutputGeneration
+    const capturedEpoch = flatOutputRefreshEpoch
 
     const inFlight = (async () => {
       const requestedAfter = loadMore ? flatOutputNextCursor : undefined
@@ -291,7 +301,7 @@ export const useAssetsStore = defineStore('assets', () => {
             ? { after: requestedAfter }
             : { offset: flatOutputOffset.value })
         })
-        if (loadMore && generation !== flatOutputGeneration) {
+        if (loadMore && capturedEpoch !== flatOutputRefreshEpoch) {
           return flatOutputAssets.value
         }
         const batch = page.assets
@@ -325,6 +335,10 @@ export const useAssetsStore = defineStore('assets', () => {
       }
     })()
 
+    // The in-flight promise is assigned synchronously here, before any await
+    // in inFlight can settle, so the loading flag set above and the guard below
+    // stay in lockstep. This relies on single-entry via updateFlatOutputs /
+    // loadMoreFlatOutputs; do not call fetchFlatOutputs directly.
     if (loadMore) {
       flatOutputLoadMoreInFlight = inFlight
     } else {