fix: remove deleted history items from local state before incremental merge

Adds removeHistoryItems() to assetsStore that evicts items by ID from
allHistoryItems and loadedIds. The delete flow in useMediaAssetActions
now calls it for successfully deleted output assets before updateHistory(),
so deleted items no longer persist after the append-only incremental merge.

Addresses review feedback:
https://github.com/Comfy-Org/ComfyUI_frontend/pull/9005#discussion_r2951082746
This commit is contained in:
bymyself
2026-03-24 12:56:58 -07:00
parent a04ec6b0fb
commit 469c8f3171
3 changed files with 53 additions and 0 deletions

View File

@@ -629,6 +629,14 @@ export function useMediaAssetActions() {
)
if (hasOutputAssets) {
const succeededOutputIds = assetArray
.filter(
(a, i) =>
getAssetType(a) === 'output' &&
results[i].status === 'fulfilled'
)
.map((a) => a.id)
assetsStore.removeHistoryItems(succeededOutputIds)
await assetsStore.updateHistory()
}
if (hasInputAssets) {

View File

@@ -399,6 +399,41 @@ describe('assetsStore - Refactored (Option A)', () => {
})
})
describe('Deletion', () => {
it('should remove items by id from history', async () => {
const mockHistory = Array.from({ length: 5 }, (_, i) =>
createMockJobItem(i)
)
vi.mocked(api.getHistory).mockResolvedValueOnce(mockHistory)
await store.updateHistory()
expect(store.historyAssets).toHaveLength(5)
store.removeHistoryItems(['prompt_1', 'prompt_3'])
expect(store.historyAssets).toHaveLength(3)
const ids = store.historyAssets.map((a) => a.id)
expect(ids).not.toContain('prompt_1')
expect(ids).not.toContain('prompt_3')
})
it('should allow re-inserting a removed item on next updateHistory', async () => {
const mockHistory = Array.from({ length: 3 }, (_, i) =>
createMockJobItem(i)
)
vi.mocked(api.getHistory).mockResolvedValueOnce(mockHistory)
await store.updateHistory()
store.removeHistoryItems(['prompt_1'])
expect(store.historyAssets).toHaveLength(2)
// Simulate the item reappearing from the server (e.g. delete failed)
vi.mocked(api.getHistory).mockResolvedValueOnce(mockHistory)
await store.updateHistory()
expect(store.historyAssets).toHaveLength(3)
})
})
describe('Error Handling', () => {
it('should preserve existing data when loadMore fails', async () => {
// First successful load - full batch

View File

@@ -155,6 +155,15 @@ export const useAssetsStore = defineStore('assets', () => {
return true
}
function removeHistoryItems(ids: string[]) {
const idSet = new Set(ids)
allHistoryItems.value = allHistoryItems.value.filter(
(item) => !idSet.has(item.id)
)
ids.forEach((id) => loadedIds.delete(id))
historyAssets.value = allHistoryItems.value
}
function trimToMaxItems() {
if (allHistoryItems.value.length > MAX_HISTORY_ITEMS) {
const removed = allHistoryItems.value.splice(MAX_HISTORY_ITEMS)
@@ -766,6 +775,7 @@ export const useAssetsStore = defineStore('assets', () => {
updateInputs,
updateHistory,
loadMoreHistory,
removeHistoryItems,
// Input mapping helpers
inputAssetsByFilename,