feat(historyV2): reconcile completed workflows (#6340)

## Summary

Running + Finished + History tasks now all work and reconcile correctly
in the queue.

## Changes

1. Reconcile complete workflows so they show up in history.
2. Do the above in a way that minimizes recreation of `TaskItemImpls`
3. Address some CR feedback on #6336 

## Review Focus

I tried to optimize `TaskItemImpls` so we aren't recreating ones for
history items tat already exist. Please give me feedback on if I did
this correctly, or if it was even necessary.

## Screenshots 🎃 



https://github.com/user-attachments/assets/afc08f31-cc09-4082-8e9d-cee977bc1e22

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6340-feat-historyV2-reconcile-completed-workflows-29a6d73d36508145a56aeb99cfa0e6ba)
by [Unito](https://www.unito.io)
This commit is contained in:
Arjan Singh
2025-10-28 17:40:55 -07:00
committed by GitHub
parent 32688b8e34
commit 32a803c31e
7 changed files with 666 additions and 201 deletions

View File

@@ -13,8 +13,21 @@ import {
} from '@tests-ui/fixtures/historyFixtures'
import {
historyV2FiveItemsSorting,
historyV2MultipleNoTimestamp,
historyV2WithMissingTimestamp
} from '@tests-ui/fixtures/historySortingFixtures'
import type { HistoryTaskItem } from '@/platform/remote/comfyui/history/types/historyV1Types'
function findResultByPromptId(
result: HistoryTaskItem[],
promptId: string
): HistoryTaskItem {
const item = result.find((item) => item.prompt[1] === promptId)
if (!item) {
throw new Error(`Expected item with promptId ${promptId} not found`)
}
return item
}
describe('mapHistoryV2toHistory', () => {
describe('fixture validation', () => {
@@ -128,22 +141,9 @@ describe('mapHistoryV2toHistory', () => {
expect(result).toHaveLength(3)
const item1000 = result.find(
(item) => item.prompt[1] === 'item-timestamp-1000'
)
const item2000 = result.find(
(item) => item.prompt[1] === 'item-timestamp-2000'
)
const itemNoTimestamp = result.find(
(item) => item.prompt[1] === 'item-no-timestamp'
)
expect(item1000).toBeDefined()
expect(item2000).toBeDefined()
expect(itemNoTimestamp).toBeDefined()
if (!item1000 || !item2000 || !itemNoTimestamp) {
throw new Error('Expected items not found in result')
}
const item1000 = findResultByPromptId(result, 'item-timestamp-1000')
const item2000 = findResultByPromptId(result, 'item-timestamp-2000')
const itemNoTimestamp = findResultByPromptId(result, 'item-no-timestamp')
expect(item2000.prompt[0]).toBe(2)
expect(item1000.prompt[0]).toBe(1)
@@ -155,30 +155,11 @@ describe('mapHistoryV2toHistory', () => {
expect(result).toHaveLength(5)
const item1000 = result.find(
(item) => item.prompt[1] === 'item-timestamp-1000'
)
const item2000 = result.find(
(item) => item.prompt[1] === 'item-timestamp-2000'
)
const item3000 = result.find(
(item) => item.prompt[1] === 'item-timestamp-3000'
)
const item4000 = result.find(
(item) => item.prompt[1] === 'item-timestamp-4000'
)
const item5000 = result.find(
(item) => item.prompt[1] === 'item-timestamp-5000'
)
expect(item1000).toBeDefined()
expect(item2000).toBeDefined()
expect(item3000).toBeDefined()
expect(item4000).toBeDefined()
expect(item5000).toBeDefined()
if (!item1000 || !item2000 || !item3000 || !item4000 || !item5000) {
throw new Error('Expected items not found in result')
}
const item1000 = findResultByPromptId(result, 'item-timestamp-1000')
const item2000 = findResultByPromptId(result, 'item-timestamp-2000')
const item3000 = findResultByPromptId(result, 'item-timestamp-3000')
const item4000 = findResultByPromptId(result, 'item-timestamp-4000')
const item5000 = findResultByPromptId(result, 'item-timestamp-5000')
expect(item5000.prompt[0]).toBe(5)
expect(item4000.prompt[0]).toBe(4)
@@ -186,5 +167,19 @@ describe('mapHistoryV2toHistory', () => {
expect(item2000.prompt[0]).toBe(2)
expect(item1000.prompt[0]).toBe(1)
})
it('assigns priority 0 to all items when multiple items lack timestamps', () => {
const result = mapHistoryV2toHistory(historyV2MultipleNoTimestamp)
expect(result).toHaveLength(3)
const item1 = findResultByPromptId(result, 'item-no-timestamp-1')
const item2 = findResultByPromptId(result, 'item-no-timestamp-2')
const item3 = findResultByPromptId(result, 'item-no-timestamp-3')
expect(item1.prompt[0]).toBe(0)
expect(item2.prompt[0]).toBe(0)
expect(item3.prompt[0]).toBe(0)
})
})
})

View File

@@ -0,0 +1,335 @@
/**
* @fileoverview Tests for history reconciliation (V1 and V2)
*/
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { reconcileHistory } from '@/platform/remote/comfyui/history/reconciliation'
import type { TaskItem } from '@/schemas/apiSchema'
// Mock distribution types
vi.mock('@/platform/distribution/types', () => ({
isCloud: false,
isDesktop: true
}))
function createHistoryItem(promptId: string, queueIndex = 0): TaskItem {
return {
taskType: 'History',
prompt: [queueIndex, promptId, {}, {}, []],
status: { status_str: 'success', completed: true, messages: [] },
outputs: {}
}
}
function getAllPromptIds(result: { items: TaskItem[] }): string[] {
return result.items.map((item) => item.prompt[1])
}
describe('reconcileHistory (V1)', () => {
beforeEach(async () => {
const distTypes = await import('@/platform/distribution/types')
vi.mocked(distTypes).isCloud = false
})
describe('when filtering by queueIndex', () => {
it('should retain items with queueIndex greater than lastKnownQueueIndex', () => {
const serverHistory = [
createHistoryItem('new-1', 11),
createHistoryItem('new-2', 10),
createHistoryItem('old', 5)
]
const clientHistory = [createHistoryItem('old', 5)]
const result = reconcileHistory(serverHistory, clientHistory, 10, 9)
const promptIds = getAllPromptIds(result)
expect(promptIds).toHaveLength(3)
expect(promptIds).toContain('new-1')
expect(promptIds).toContain('new-2')
expect(promptIds).toContain('old')
})
it('should evict items with queueIndex less than or equal to lastKnownQueueIndex', () => {
const serverHistory = [
createHistoryItem('new', 11),
createHistoryItem('existing', 10),
createHistoryItem('old-should-not-appear', 5)
]
const clientHistory = [createHistoryItem('existing', 10)]
const result = reconcileHistory(serverHistory, clientHistory, 10, 10)
const promptIds = getAllPromptIds(result)
expect(promptIds).toHaveLength(2)
expect(promptIds).toContain('new')
expect(promptIds).toContain('existing')
expect(promptIds).not.toContain('old-should-not-appear')
})
it('should retain all server items when lastKnownQueueIndex is undefined', () => {
const serverHistory = [
createHistoryItem('item-1', 5),
createHistoryItem('item-2', 4)
]
const result = reconcileHistory(serverHistory, [], 10, undefined)
expect(result.items).toHaveLength(2)
expect(result.items[0].prompt[1]).toBe('item-1')
expect(result.items[1].prompt[1]).toBe('item-2')
})
})
describe('when reconciling with existing client items', () => {
it('should retain client items that still exist on server', () => {
const serverHistory = [
createHistoryItem('new', 11),
createHistoryItem('existing-1', 9),
createHistoryItem('existing-2', 8)
]
const clientHistory = [
createHistoryItem('existing-1', 9),
createHistoryItem('existing-2', 8)
]
const result = reconcileHistory(serverHistory, clientHistory, 10, 10)
const promptIds = getAllPromptIds(result)
expect(promptIds).toHaveLength(3)
expect(promptIds).toContain('new')
expect(promptIds).toContain('existing-1')
expect(promptIds).toContain('existing-2')
})
it('should evict client items that no longer exist on server', () => {
const serverHistory = [
createHistoryItem('new', 11),
createHistoryItem('keep', 9)
]
const clientHistory = [
createHistoryItem('keep', 9),
createHistoryItem('removed-from-server', 8)
]
const result = reconcileHistory(serverHistory, clientHistory, 10, 10)
const promptIds = getAllPromptIds(result)
expect(promptIds).toHaveLength(2)
expect(promptIds).toContain('new')
expect(promptIds).toContain('keep')
expect(promptIds).not.toContain('removed-from-server')
})
})
describe('when limiting the result count', () => {
it('should respect the maxItems constraint', () => {
const serverHistory = Array.from({ length: 10 }, (_, i) =>
createHistoryItem(`item-${i}`, 20 + i)
)
const result = reconcileHistory(serverHistory, [], 5, 15)
const promptIds = getAllPromptIds(result)
expect(promptIds).toHaveLength(5)
})
it('should evict lowest priority items when exceeding capacity', () => {
const serverHistory = [
createHistoryItem('new-1', 13),
createHistoryItem('new-2', 12),
createHistoryItem('new-3', 11),
createHistoryItem('existing', 9)
]
const clientHistory = [createHistoryItem('existing', 9)]
const result = reconcileHistory(serverHistory, clientHistory, 2, 10)
expect(result.items).toHaveLength(2)
expect(result.items[0].prompt[1]).toBe('new-1')
expect(result.items[1].prompt[1]).toBe('new-2')
})
})
describe('when handling empty collections', () => {
it('should return all server items when client history is empty', () => {
const serverHistory = [
createHistoryItem('item-1', 10),
createHistoryItem('item-2', 9)
]
const result = reconcileHistory(serverHistory, [], 10, 8)
const promptIds = getAllPromptIds(result)
expect(promptIds).toHaveLength(2)
})
it('should return empty result when server history is empty', () => {
const clientHistory = [createHistoryItem('item-1', 5)]
const result = reconcileHistory([], clientHistory, 10, 5)
expect(result.items).toHaveLength(0)
})
it('should return empty result when both collections are empty', () => {
const result = reconcileHistory([], [], 10, undefined)
expect(result.items).toHaveLength(0)
})
})
})
describe('reconcileHistory (V2/Cloud)', () => {
beforeEach(async () => {
const distTypes = await import('@/platform/distribution/types')
vi.mocked(distTypes).isCloud = true
})
describe('when adding new items from server', () => {
it('should retain items with promptIds not present in client history', () => {
const serverHistory = [
createHistoryItem('new-item'),
createHistoryItem('existing-item')
]
const clientHistory = [createHistoryItem('existing-item')]
const result = reconcileHistory(serverHistory, clientHistory, 10)
const promptIds = getAllPromptIds(result)
expect(promptIds).toHaveLength(2)
expect(promptIds).toContain('new-item')
expect(promptIds).toContain('existing-item')
})
it('should respect priority ordering when retaining multiple new items', () => {
const serverHistory = [
createHistoryItem('new-1'),
createHistoryItem('new-2'),
createHistoryItem('existing')
]
const clientHistory = [createHistoryItem('existing')]
const result = reconcileHistory(serverHistory, clientHistory, 10)
const promptIds = getAllPromptIds(result)
expect(promptIds).toHaveLength(3)
expect(promptIds).toContain('new-1')
expect(promptIds).toContain('new-2')
expect(promptIds).toContain('existing')
})
})
describe('when reconciling with existing client items', () => {
it('should retain client items that still exist on server', () => {
const serverHistory = [
createHistoryItem('item-1'),
createHistoryItem('item-2')
]
const clientHistory = [
createHistoryItem('item-1'),
createHistoryItem('item-2')
]
const result = reconcileHistory(serverHistory, clientHistory, 10)
const promptIds = getAllPromptIds(result)
expect(promptIds).toHaveLength(2)
expect(promptIds).toContain('item-1')
expect(promptIds).toContain('item-2')
})
it('should evict client items that no longer exist on server', () => {
const serverHistory = [createHistoryItem('item-1')]
const clientHistory = [
createHistoryItem('item-1'),
createHistoryItem('old-item')
]
const result = reconcileHistory(serverHistory, clientHistory, 10)
const promptIds = getAllPromptIds(result)
expect(promptIds).toHaveLength(1)
expect(promptIds).toContain('item-1')
expect(promptIds).not.toContain('old-item')
})
})
describe('when detecting new items by promptId', () => {
it('should retain new items regardless of queueIndex values', () => {
const serverHistory = [
createHistoryItem('existing', 100),
createHistoryItem('new-item', 50)
]
const clientHistory = [createHistoryItem('existing', 100)]
const result = reconcileHistory(serverHistory, clientHistory, 10)
const promptIds = getAllPromptIds(result)
expect(promptIds).toContain('new-item')
expect(promptIds).toContain('existing')
})
})
describe('when limiting the result count', () => {
it('should respect the maxItems constraint', () => {
const serverHistory = Array.from({ length: 10 }, (_, i) =>
createHistoryItem(`server-${i}`)
)
const clientHistory = Array.from({ length: 5 }, (_, i) =>
createHistoryItem(`client-${i}`)
)
const result = reconcileHistory(serverHistory, clientHistory, 5)
const promptIds = getAllPromptIds(result)
expect(promptIds).toHaveLength(5)
})
it('should evict lowest priority items when exceeding capacity', () => {
const serverHistory = [
createHistoryItem('new-1'),
createHistoryItem('new-2'),
createHistoryItem('existing')
]
const clientHistory = [createHistoryItem('existing')]
const result = reconcileHistory(serverHistory, clientHistory, 2)
expect(result.items).toHaveLength(2)
expect(result.items[0].prompt[1]).toBe('new-1')
expect(result.items[1].prompt[1]).toBe('new-2')
})
})
describe('when handling empty collections', () => {
it('should return all server items when client history is empty', () => {
const serverHistory = [
createHistoryItem('item-1'),
createHistoryItem('item-2')
]
const result = reconcileHistory(serverHistory, [], 10)
expect(result.items).toHaveLength(2)
expect(result.items[0].prompt[1]).toBe('item-1')
expect(result.items[1].prompt[1]).toBe('item-2')
})
it('should return empty result when server history is empty', () => {
const clientHistory = [
createHistoryItem('item-1'),
createHistoryItem('item-2')
]
const result = reconcileHistory([], clientHistory, 10)
expect(result.items).toHaveLength(0)
})
it('should return empty result when both collections are empty', () => {
const result = reconcileHistory([], [], 10)
expect(result.items).toHaveLength(0)
})
})
})

View File

@@ -361,131 +361,6 @@ describe('useQueueStore', () => {
})
})
describe('update() - history reconciliation by promptId', () => {
it('should keep existing history items that are still in server response', async () => {
const hist1 = createHistoryTask(10, 'existing-1')
const hist2 = createHistoryTask(9, 'existing-2')
mockGetQueue.mockResolvedValue({ Running: [], Pending: [] })
mockGetHistory.mockResolvedValue({ History: [hist1, hist2] })
await store.update()
expect(store.historyTasks).toHaveLength(2)
const hist3 = createHistoryTask(11, 'new-1')
mockGetHistory.mockResolvedValue({
History: [hist3, hist1, hist2]
})
await store.update()
expect(store.historyTasks).toHaveLength(3)
expect(store.historyTasks.map((t) => t.promptId)).toEqual([
'new-1',
'existing-1',
'existing-2'
])
})
it('should remove history items no longer in server response', async () => {
const hist1 = createHistoryTask(10, 'remove-me')
const hist2 = createHistoryTask(9, 'keep-me')
mockGetQueue.mockResolvedValue({ Running: [], Pending: [] })
mockGetHistory.mockResolvedValue({ History: [hist1, hist2] })
await store.update()
expect(store.historyTasks).toHaveLength(2)
mockGetHistory.mockResolvedValue({ History: [hist2] })
await store.update()
expect(store.historyTasks).toHaveLength(1)
expect(store.historyTasks[0].promptId).toBe('keep-me')
})
it('should add new history items with queueIndex > lastHistoryQueueIndex', async () => {
const hist1 = createHistoryTask(5, 'old-1')
mockGetQueue.mockResolvedValue({ Running: [], Pending: [] })
mockGetHistory.mockResolvedValue({ History: [hist1] })
await store.update()
expect(store.lastHistoryQueueIndex).toBe(5)
const hist2 = createHistoryTask(6, 'new-1')
const hist3 = createHistoryTask(7, 'new-2')
mockGetHistory.mockResolvedValue({
History: [hist3, hist2, hist1]
})
await store.update()
expect(store.historyTasks).toHaveLength(3)
expect(store.historyTasks.map((t) => t.promptId)).toContain('new-1')
expect(store.historyTasks.map((t) => t.promptId)).toContain('new-2')
})
it('should NOT add history items with queueIndex <= lastHistoryQueueIndex', async () => {
const hist1 = createHistoryTask(10, 'existing-1')
mockGetQueue.mockResolvedValue({ Running: [], Pending: [] })
mockGetHistory.mockResolvedValue({ History: [hist1] })
await store.update()
const oldHist = createHistoryTask(5, 'old-task-should-not-appear')
mockGetHistory.mockResolvedValue({ History: [hist1, oldHist] })
await store.update()
expect(store.historyTasks).toHaveLength(1)
expect(store.historyTasks[0].promptId).toBe('existing-1')
})
it('should handle complete history replacement', async () => {
const hist1 = createHistoryTask(5, 'old-1')
const hist2 = createHistoryTask(4, 'old-2')
mockGetQueue.mockResolvedValue({ Running: [], Pending: [] })
mockGetHistory.mockResolvedValue({ History: [hist1, hist2] })
await store.update()
expect(store.historyTasks).toHaveLength(2)
const newHist1 = createHistoryTask(10, 'new-1')
const newHist2 = createHistoryTask(9, 'new-2')
mockGetHistory.mockResolvedValue({
History: [newHist1, newHist2]
})
await store.update()
expect(store.historyTasks).toHaveLength(2)
expect(store.historyTasks.map((t) => t.promptId)).toEqual([
'new-1',
'new-2'
])
})
it('should handle empty history from server', async () => {
const hist1 = createHistoryTask(5, 'will-be-removed')
mockGetQueue.mockResolvedValue({ Running: [], Pending: [] })
mockGetHistory.mockResolvedValue({ History: [hist1] })
await store.update()
expect(store.historyTasks).toHaveLength(1)
mockGetHistory.mockResolvedValue({ History: [] })
await store.update()
expect(store.historyTasks).toHaveLength(0)
})
})
describe('update() - queue index collision (THE BUG FIX)', () => {
it('should NOT confuse different prompts with same queueIndex', async () => {
const hist1 = createHistoryTask(50, 'prompt-uuid-aaa')
@@ -560,6 +435,70 @@ describe('useQueueStore', () => {
})
})
describe('update() - history reconciliation', () => {
it('should keep existing items still on server (by promptId)', async () => {
const hist1 = createHistoryTask(10, 'existing-1')
const hist2 = createHistoryTask(9, 'existing-2')
mockGetQueue.mockResolvedValue({ Running: [], Pending: [] })
mockGetHistory.mockResolvedValue({ History: [hist1, hist2] })
await store.update()
expect(store.historyTasks).toHaveLength(2)
const hist3 = createHistoryTask(11, 'new-1')
mockGetHistory.mockResolvedValue({
History: [hist3, hist1, hist2]
})
await store.update()
expect(store.historyTasks).toHaveLength(3)
expect(store.historyTasks.map((t) => t.promptId)).toContain('existing-1')
expect(store.historyTasks.map((t) => t.promptId)).toContain('existing-2')
expect(store.historyTasks.map((t) => t.promptId)).toContain('new-1')
})
it('should remove items no longer on server', async () => {
const hist1 = createHistoryTask(10, 'remove-me')
const hist2 = createHistoryTask(9, 'keep-me')
mockGetQueue.mockResolvedValue({ Running: [], Pending: [] })
mockGetHistory.mockResolvedValue({ History: [hist1, hist2] })
await store.update()
expect(store.historyTasks).toHaveLength(2)
mockGetHistory.mockResolvedValue({ History: [hist2] })
await store.update()
expect(store.historyTasks).toHaveLength(1)
expect(store.historyTasks[0].promptId).toBe('keep-me')
})
it('should add new items from server', async () => {
const hist1 = createHistoryTask(5, 'old-1')
mockGetQueue.mockResolvedValue({ Running: [], Pending: [] })
mockGetHistory.mockResolvedValue({ History: [hist1] })
await store.update()
const hist2 = createHistoryTask(6, 'new-1')
const hist3 = createHistoryTask(7, 'new-2')
mockGetHistory.mockResolvedValue({
History: [hist3, hist2, hist1]
})
await store.update()
expect(store.historyTasks).toHaveLength(3)
expect(store.historyTasks.map((t) => t.promptId)).toContain('new-1')
expect(store.historyTasks.map((t) => t.promptId)).toContain('new-2')
})
})
describe('update() - maxHistoryItems limit', () => {
it('should enforce maxHistoryItems limit', async () => {
store.maxHistoryItems = 3