Skip to content

Commit bc741e7

Browse files
committed
fix(assets): harden cursor-pagination in-flight tracking and loaded-id check
Split flat-output in-flight promise tracking into separate refresh and loadMore slots so a refresh fired during an in-flight loadMore always runs its own reset path rather than coalescing into the loadMore result. Add loadedJobIds Set that records every raw job id walked in fetchHistoryAssets (not just displayable ones), cleared on reset, so a page of all-failed / no-preview jobs does not false-negative the loadMore dedup check.
1 parent c296842 commit bc741e7

2 files changed

Lines changed: 154 additions & 17 deletions

File tree

src/stores/assetsStore.test.ts

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -770,6 +770,78 @@ describe('assetsStore - Refactored (Option A)', () => {
770770
})
771771
})
772772

773+
describe('assetsStore - loadedJobIds (all-job dedup)', () => {
774+
let store: ReturnType<typeof useAssetsStore>
775+
776+
const createFailedJobItem = (id: string): JobListItem => ({
777+
id,
778+
status: 'failed',
779+
create_time: 1000,
780+
update_time: 1000,
781+
last_state_update: 1000,
782+
priority: 1
783+
})
784+
785+
const createDisplayableJobItem = (id: string, index = 0): JobListItem => ({
786+
id,
787+
status: 'completed',
788+
create_time: 1000 + index,
789+
update_time: 1000 + index,
790+
last_state_update: 1000 + index,
791+
priority: 1000 + index,
792+
preview_output: {
793+
filename: `output_${id}.png`,
794+
subfolder: '',
795+
type: 'output',
796+
nodeId: 'node_1',
797+
mediaType: 'images'
798+
}
799+
})
800+
801+
beforeEach(() => {
802+
setActivePinia(createTestingPinia({ stubActions: false }))
803+
store = useAssetsStore()
804+
vi.clearAllMocks()
805+
})
806+
807+
it('does not false-negative dedup when the first page is all non-displayable jobs', async () => {
808+
const firstBatch = Array.from({ length: 200 }, (_, i) =>
809+
createFailedJobItem(`failed_${i}`)
810+
)
811+
vi.mocked(api.getHistory).mockResolvedValueOnce(firstBatch)
812+
await store.updateHistory()
813+
814+
expect(store.historyAssets).toHaveLength(0)
815+
816+
const secondBatch = [
817+
createFailedJobItem('failed_0'),
818+
createDisplayableJobItem('new_job', 0)
819+
]
820+
vi.mocked(api.getHistory).mockResolvedValueOnce(secondBatch)
821+
await store.loadMoreHistory()
822+
823+
const ids = store.historyAssets.map((a) => a.id)
824+
expect(ids).toContain('new_job')
825+
expect(ids).not.toContain('failed_0')
826+
})
827+
828+
it('loadedJobIds is cleared on reset (updateHistory)', async () => {
829+
const firstBatch = [createFailedJobItem('job_a')]
830+
vi.mocked(api.getHistory).mockResolvedValueOnce(firstBatch)
831+
await store.updateHistory()
832+
833+
const secondBatch = [createDisplayableJobItem('job_a', 0)]
834+
vi.mocked(api.getHistory)
835+
.mockResolvedValueOnce(secondBatch)
836+
.mockResolvedValueOnce([])
837+
838+
await store.updateHistory()
839+
840+
expect(store.historyAssets).toHaveLength(1)
841+
expect(store.historyAssets[0].id).toBe('job_a')
842+
})
843+
})
844+
773845
describe('assetsStore - Model Assets Cache (Cloud)', () => {
774846
beforeEach(() => {
775847
setActivePinia(createTestingPinia({ stubActions: false }))
@@ -1634,4 +1706,60 @@ describe('assetsStore - Flat Output Assets (cloud-only)', () => {
16341706

16351707
expect(store.flatOutputAssets.map((x) => x.id)).toEqual(['shared-1'])
16361708
})
1709+
1710+
describe('in-flight tracking: refresh vs loadMore', () => {
1711+
it('refresh during an in-flight loadMore runs its reset path and is not dropped', async () => {
1712+
const firstPage = Array.from({ length: FLAT_OUTPUT_PAGE_SIZE }, (_, i) =>
1713+
makeAsset(`a${i}`, `f${i}.png`)
1714+
)
1715+
vi.mocked(assetService.getAssetsByTag).mockResolvedValueOnce(firstPage)
1716+
const store = useAssetsStore()
1717+
await store.updateFlatOutputs()
1718+
1719+
vi.mocked(assetService.getAssetsByTag).mockClear()
1720+
1721+
let resolveLoadMore!: (assets: AssetItem[]) => void
1722+
const loadMorePromise = new Promise<AssetItem[]>((res) => {
1723+
resolveLoadMore = res
1724+
})
1725+
let resolveRefresh!: (assets: AssetItem[]) => void
1726+
const refreshPromise = new Promise<AssetItem[]>((res) => {
1727+
resolveRefresh = res
1728+
})
1729+
1730+
vi.mocked(assetService.getAssetsByTag)
1731+
.mockReturnValueOnce(loadMorePromise)
1732+
.mockReturnValueOnce(refreshPromise)
1733+
1734+
const loadMoreResult = store.loadMoreFlatOutputs()
1735+
const refreshResult = store.updateFlatOutputs()
1736+
1737+
expect(vi.mocked(assetService.getAssetsByTag)).toHaveBeenCalledTimes(2)
1738+
1739+
resolveRefresh([makeAsset('fresh-1', 'fresh.png')])
1740+
resolveLoadMore([makeAsset('extra-1', 'extra.png')])
1741+
await Promise.all([loadMoreResult, refreshResult])
1742+
1743+
expect(store.flatOutputAssets.map((a) => a.id)).toContain('fresh-1')
1744+
})
1745+
1746+
it('a second concurrent refresh coalesces into the first refresh promise', async () => {
1747+
let resolvePage!: (assets: AssetItem[]) => void
1748+
const pagePromise = new Promise<AssetItem[]>((res) => {
1749+
resolvePage = res
1750+
})
1751+
vi.mocked(assetService.getAssetsByTag).mockReturnValueOnce(pagePromise)
1752+
1753+
const store = useAssetsStore()
1754+
const r1 = store.updateFlatOutputs()
1755+
const r2 = store.updateFlatOutputs()
1756+
1757+
expect(vi.mocked(assetService.getAssetsByTag)).toHaveBeenCalledTimes(1)
1758+
1759+
resolvePage([makeAsset('only-1', 'only.png')])
1760+
await Promise.all([r1, r2])
1761+
1762+
expect(store.flatOutputAssets.map((a) => a.id)).toEqual(['only-1'])
1763+
})
1764+
})
16371765
})

src/stores/assetsStore.ts

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ export const useAssetsStore = defineStore('assets', () => {
122122
const allHistoryItems = ref<AssetItem[]>([])
123123

124124
const loadedIds = shallowReactive(new Set<string>())
125+
const loadedJobIds = new Set<string>()
125126

126127
const fetchInputFiles = isCloud
127128
? fetchInputFilesFromCloud
@@ -151,46 +152,43 @@ export const useAssetsStore = defineStore('assets', () => {
151152
* @param loadMore - true for pagination (append), false for initial load (replace)
152153
*/
153154
const fetchHistoryAssets = async (loadMore = false): Promise<AssetItem[]> => {
154-
// Reset state for initial load
155155
if (!loadMore) {
156156
historyOffset.value = 0
157157
hasMoreHistory.value = true
158158
allHistoryItems.value = []
159159
loadedIds.clear()
160+
loadedJobIds.clear()
160161
}
161162

162-
// Fetch from server with offset
163163
const history = await api.getHistory(BATCH_SIZE, {
164164
offset: historyOffset.value
165165
})
166166

167-
// Convert JobListItems to AssetItems
167+
for (const job of history) {
168+
loadedJobIds.add(job.id)
169+
}
170+
168171
const newAssets = mapHistoryToAssets(history)
169172

170173
if (loadMore) {
171-
// Filter out duplicates and insert in sorted order
172174
for (const asset of newAssets) {
173175
if (loadedIds.has(asset.id)) {
174-
continue // Skip duplicates
176+
continue
175177
}
176178
loadedIds.add(asset.id)
177179

178-
// Find insertion index to maintain sorted order (newest first)
179180
const assetTime = new Date(asset.created_at ?? 0).getTime()
180181
const insertIndex = allHistoryItems.value.findIndex(
181182
(item) => new Date(item.created_at ?? 0).getTime() < assetTime
182183
)
183184

184185
if (insertIndex === -1) {
185-
// Asset is oldest, append to end
186186
allHistoryItems.value.push(asset)
187187
} else {
188-
// Insert at the correct position
189188
allHistoryItems.value.splice(insertIndex, 0, asset)
190189
}
191190
}
192191
} else {
193-
// Initial load: replace all
194192
allHistoryItems.value = newAssets
195193
newAssets.forEach((asset) => loadedIds.add(asset.id))
196194
}
@@ -267,23 +265,24 @@ export const useAssetsStore = defineStore('assets', () => {
267265
const flatOutputHasMore = ref(true)
268266
const flatOutputIsLoadingMore = ref(false)
269267
const flatOutputSeenIds = new Set<string>()
270-
let flatOutputInFlight: Promise<AssetItem[]> | null = null
268+
let flatOutputRefreshInFlight: Promise<AssetItem[]> | null = null
269+
let flatOutputLoadMoreInFlight: Promise<AssetItem[]> | null = null
271270

272271
async function fetchFlatOutputs(loadMore: boolean): Promise<AssetItem[]> {
273-
if (flatOutputInFlight) return flatOutputInFlight
274-
275272
if (loadMore) {
276273
if (!flatOutputHasMore.value) return flatOutputAssets.value
274+
if (flatOutputLoadMoreInFlight) return flatOutputLoadMoreInFlight
277275
flatOutputIsLoadingMore.value = true
278276
} else {
277+
if (flatOutputRefreshInFlight) return flatOutputRefreshInFlight
279278
flatOutputLoading.value = true
280279
flatOutputOffset.value = 0
281280
flatOutputHasMore.value = true
282281
flatOutputSeenIds.clear()
283282
}
284283
flatOutputError.value = null
285284

286-
flatOutputInFlight = (async () => {
285+
const inFlight = (async () => {
287286
try {
288287
const page = await assetService.getAssetsByTag(OUTPUT_TAG, true, {
289288
limit: FLAT_OUTPUT_PAGE_SIZE,
@@ -304,13 +303,23 @@ export const useAssetsStore = defineStore('assets', () => {
304303
console.error('Failed to fetch output assets:', err)
305304
return loadMore ? flatOutputAssets.value : []
306305
} finally {
307-
if (loadMore) flatOutputIsLoadingMore.value = false
308-
else flatOutputLoading.value = false
309-
flatOutputInFlight = null
306+
if (loadMore) {
307+
flatOutputIsLoadingMore.value = false
308+
flatOutputLoadMoreInFlight = null
309+
} else {
310+
flatOutputLoading.value = false
311+
flatOutputRefreshInFlight = null
312+
}
310313
}
311314
})()
312315

313-
return flatOutputInFlight
316+
if (loadMore) {
317+
flatOutputLoadMoreInFlight = inFlight
318+
} else {
319+
flatOutputRefreshInFlight = inFlight
320+
}
321+
322+
return inFlight
314323
}
315324

316325
const updateFlatOutputs = () => fetchFlatOutputs(false)

0 commit comments

Comments
 (0)