From 32a803c31e8ed5643480d3a82a578667e8e2848a Mon Sep 17 00:00:00 2001 From: Arjan Singh <1598641+arjansingh@users.noreply.github.com> Date: Tue, 28 Oct 2025 17:40:55 -0700 Subject: [PATCH] feat(historyV2): reconcile completed workflows (#6340) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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) --- .../comfyui/history/adapters/v2ToV1Adapter.ts | 2 +- .../remote/comfyui/history/reconciliation.ts | 138 ++++++++ src/stores/queueStore.ts | 69 ++-- tests-ui/fixtures/historySortingFixtures.ts | 59 +++ .../history/adapters/v2ToV1Adapter.test.ts | 75 ++-- .../comfyui/history/reconciliation.test.ts | 335 ++++++++++++++++++ tests-ui/tests/store/queueStore.test.ts | 189 ++++------ 7 files changed, 666 insertions(+), 201 deletions(-) create mode 100644 src/platform/remote/comfyui/history/reconciliation.ts create mode 100644 tests-ui/tests/platform/remote/comfyui/history/reconciliation.test.ts diff --git a/src/platform/remote/comfyui/history/adapters/v2ToV1Adapter.ts b/src/platform/remote/comfyui/history/adapters/v2ToV1Adapter.ts index bf3cbdcfb..52801484e 100644 --- a/src/platform/remote/comfyui/history/adapters/v2ToV1Adapter.ts +++ b/src/platform/remote/comfyui/history/adapters/v2ToV1Adapter.ts @@ -34,7 +34,7 @@ function getExecutionSuccessTimestamp(item: RawHistoryItemV2): number { export function mapHistoryV2toHistory( historyV2Response: HistoryResponseV2 ): HistoryTaskItem[] { - const history = historyV2Response.history + const { history } = historyV2Response // Sort by execution_success timestamp, descending (newest first) history.sort((a, b) => { diff --git a/src/platform/remote/comfyui/history/reconciliation.ts b/src/platform/remote/comfyui/history/reconciliation.ts new file mode 100644 index 000000000..969566879 --- /dev/null +++ b/src/platform/remote/comfyui/history/reconciliation.ts @@ -0,0 +1,138 @@ +/** + * @fileoverview History reconciliation for V1 and V2 APIs + * @module platform/remote/comfyui/history/reconciliation + * + * Returns list of items that should be displayed, sorted by queueIndex (newest first). + * Caller is responsible for mapping to their own class instances. + * + * V1: QueueIndex-based filtering for stable monotonic indices + * V2: PromptId-based merging for synthetic priorities (V2 assigns synthetic + * priorities after timestamp sorting, so new items may have lower priority + * than existing items) + */ +import { isCloud } from '@/platform/distribution/types' +import type { TaskItem } from '@/schemas/apiSchema' + +interface ReconciliationResult { + /** All items to display, sorted by queueIndex descending (newest first) */ + items: TaskItem[] +} + +/** + * V1 reconciliation: QueueIndex-based filtering works because V1 has stable, + * monotonically increasing queue indices. + * + * Sort order: Sorts serverHistory by queueIndex descending (newest first) to ensure + * consistent ordering. JavaScript .filter() maintains iteration order, so filtered + * results remain sorted. clientHistory is assumed already sorted from previous update. + */ +function reconcileHistoryV1( + serverHistory: TaskItem[], + clientHistory: TaskItem[], + maxItems: number, + lastKnownQueueIndex: number | undefined +): ReconciliationResult { + const sortedServerHistory = serverHistory.sort( + (a, b) => b.prompt[0] - a.prompt[0] + ) + + const serverPromptIds = new Set( + sortedServerHistory.map((item) => item.prompt[1]) + ) + + // If undefined, treat as initial sync (all items are new) + const itemsAddedSinceLastSync = + lastKnownQueueIndex === undefined + ? sortedServerHistory + : sortedServerHistory.filter( + (item) => item.prompt[0] > lastKnownQueueIndex + ) + + const clientItemsStillOnServer = clientHistory.filter((item) => + serverPromptIds.has(item.prompt[1]) + ) + + // Merge new and reused items, sort by queueIndex descending, limit to maxItems + const allItems = [...itemsAddedSinceLastSync, ...clientItemsStillOnServer] + .sort((a, b) => b.prompt[0] - a.prompt[0]) + .slice(0, maxItems) + + return { + items: allItems + } +} + +/** + * V2 reconciliation: PromptId-based merging because V2 assigns synthetic + * priorities after sorting by timestamp. + * + * Sort order: Sorts serverHistory by queueIndex descending (newest first) to ensure + * consistent ordering. JavaScript .filter() maintains iteration order, so filtered + * results remain sorted. clientHistory is assumed already sorted from previous update. + */ +function reconcileHistoryV2( + serverHistory: TaskItem[], + clientHistory: TaskItem[], + maxItems: number +): ReconciliationResult { + const sortedServerHistory = serverHistory.sort( + (a, b) => b.prompt[0] - a.prompt[0] + ) + + const serverPromptIds = new Set( + sortedServerHistory.map((item) => item.prompt[1]) + ) + const clientPromptIds = new Set(clientHistory.map((item) => item.prompt[1])) + + const newPromptIds = new Set( + [...serverPromptIds].filter((id) => !clientPromptIds.has(id)) + ) + + const newItems = sortedServerHistory.filter((item) => + newPromptIds.has(item.prompt[1]) + ) + + const retainedPromptIds = new Set( + [...serverPromptIds].filter((id) => clientPromptIds.has(id)) + ) + const clientItemsStillOnServer = clientHistory.filter((item) => + retainedPromptIds.has(item.prompt[1]) + ) + + // Merge new and reused items, sort by queueIndex descending, limit to maxItems + const allItems = [...newItems, ...clientItemsStillOnServer] + .sort((a, b) => b.prompt[0] - a.prompt[0]) + .slice(0, maxItems) + + return { + items: allItems + } +} + +/** + * Reconciles server history with client history. + * Automatically uses V1 (queueIndex-based) or V2 (promptId-based) algorithm based on + * distribution type. + * + * @param serverHistory - Server's current history items + * @param clientHistory - Client's existing history items + * @param maxItems - Maximum number of items to return + * @param lastKnownQueueIndex - Last queue index seen (V1 only, optional for V2) + * @returns All items that should be displayed, sorted by queueIndex descending + */ +export function reconcileHistory( + serverHistory: TaskItem[], + clientHistory: TaskItem[], + maxItems: number, + lastKnownQueueIndex?: number +): ReconciliationResult { + if (isCloud) { + return reconcileHistoryV2(serverHistory, clientHistory, maxItems) + } + return reconcileHistoryV1( + serverHistory, + clientHistory, + maxItems, + lastKnownQueueIndex + ) +} diff --git a/src/stores/queueStore.ts b/src/stores/queueStore.ts index 99d8d0eb7..17483c04b 100644 --- a/src/stores/queueStore.ts +++ b/src/stores/queueStore.ts @@ -1,12 +1,14 @@ import _ from 'es-toolkit/compat' import { defineStore } from 'pinia' -import { computed, ref, toRaw } from 'vue' +import { computed, ref, shallowRef, toRaw, toValue } from 'vue' +import { reconcileHistory } from '@/platform/remote/comfyui/history/reconciliation' import type { ComfyWorkflowJSON, NodeId } from '@/platform/workflow/validation/schemas/workflowSchema' import type { + HistoryTaskItem, ResultItem, StatusWsMessageStatus, TaskItem, @@ -423,14 +425,18 @@ export class TaskItemImpl { ) ) } + + public toTaskItem(): TaskItem { + const item: HistoryTaskItem = { + taskType: 'History', + prompt: this.prompt, + status: this.status!, + outputs: this.outputs + } + return item + } } -const extractPromptIds = (tasks: TaskItem[]): Set => - new Set(tasks.map((task) => task.prompt[1])) - -const isAddedAfter = (queueIndex: number) => (task: TaskItem) => - task.prompt[0] > queueIndex - const sortNewestFirst = (a: TaskItemImpl, b: TaskItemImpl) => b.queueIndex - a.queueIndex @@ -445,31 +451,12 @@ const toTaskItemImpls = (tasks: TaskItem[]): TaskItemImpl[] => ) ) -const reconcileHistoryWithServer = ( - serverHistory: TaskItem[], - clientHistory: TaskItemImpl[], - lastKnownQueueIndex: number, - maxItems: number -): TaskItemImpl[] => { - const serverPromptIds = extractPromptIds(serverHistory) - - const itemsAddedSinceLastSync = toTaskItemImpls( - serverHistory.filter(isAddedAfter(lastKnownQueueIndex)) - ) - - const itemsStillOnServer = clientHistory.filter((item) => - serverPromptIds.has(item.promptId) - ) - - return [...itemsAddedSinceLastSync, ...itemsStillOnServer] - .sort(sortNewestFirst) - .slice(0, maxItems) -} - export const useQueueStore = defineStore('queue', () => { - const runningTasks = ref([]) - const pendingTasks = ref([]) - const historyTasks = ref([]) + // Use shallowRef because TaskItemImpl instances are immutable and arrays are + // replaced entirely (not mutated), so deep reactivity would waste performance + const runningTasks = shallowRef([]) + const pendingTasks = shallowRef([]) + const historyTasks = shallowRef([]) const maxHistoryItems = ref(64) const isLoading = ref(false) @@ -503,11 +490,23 @@ export const useQueueStore = defineStore('queue', () => { runningTasks.value = toTaskItemImpls(queue.Running).sort(sortNewestFirst) pendingTasks.value = toTaskItemImpls(queue.Pending).sort(sortNewestFirst) - historyTasks.value = reconcileHistoryWithServer( + const currentHistory = toValue(historyTasks) + + const { items } = reconcileHistory( history.History, - historyTasks.value, - lastHistoryQueueIndex.value, - maxHistoryItems.value + currentHistory.map((impl) => impl.toTaskItem()), + toValue(maxHistoryItems), + toValue(lastHistoryQueueIndex) + ) + + // Reuse existing TaskItemImpl instances or create new + const existingByPromptId = new Map( + currentHistory.map((impl) => [impl.promptId, impl]) + ) + + historyTasks.value = items.map( + (item) => + existingByPromptId.get(item.prompt[1]) ?? toTaskItemImpls([item])[0] ) } finally { isLoading.value = false diff --git a/tests-ui/fixtures/historySortingFixtures.ts b/tests-ui/fixtures/historySortingFixtures.ts index 797aec243..a7b630667 100644 --- a/tests-ui/fixtures/historySortingFixtures.ts +++ b/tests-ui/fixtures/historySortingFixtures.ts @@ -197,3 +197,62 @@ export const historyV2FiveItemsSorting: HistoryResponseV2 = { } ] } + +export const historyV2MultipleNoTimestamp: HistoryResponseV2 = { + history: [ + { + prompt_id: 'item-no-timestamp-1', + prompt: { + priority: 0, + prompt_id: 'item-no-timestamp-1', + extra_data: { client_id: 'test-client' } + }, + outputs: { + '1': { + images: [{ filename: 'test1.png', type: 'output', subfolder: '' }] + } + }, + status: { + status_str: 'success', + completed: true, + messages: [] + } + }, + { + prompt_id: 'item-no-timestamp-2', + prompt: { + priority: 0, + prompt_id: 'item-no-timestamp-2', + extra_data: { client_id: 'test-client' } + }, + outputs: { + '2': { + images: [{ filename: 'test2.png', type: 'output', subfolder: '' }] + } + }, + status: { + status_str: 'success', + completed: true, + messages: [] + } + }, + { + prompt_id: 'item-no-timestamp-3', + prompt: { + priority: 0, + prompt_id: 'item-no-timestamp-3', + extra_data: { client_id: 'test-client' } + }, + outputs: { + '3': { + images: [{ filename: 'test3.png', type: 'output', subfolder: '' }] + } + }, + status: { + status_str: 'success', + completed: true, + messages: [] + } + } + ] +} diff --git a/tests-ui/tests/platform/remote/comfyui/history/adapters/v2ToV1Adapter.test.ts b/tests-ui/tests/platform/remote/comfyui/history/adapters/v2ToV1Adapter.test.ts index 0d6cef8c2..c047782a0 100644 --- a/tests-ui/tests/platform/remote/comfyui/history/adapters/v2ToV1Adapter.test.ts +++ b/tests-ui/tests/platform/remote/comfyui/history/adapters/v2ToV1Adapter.test.ts @@ -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) + }) }) }) diff --git a/tests-ui/tests/platform/remote/comfyui/history/reconciliation.test.ts b/tests-ui/tests/platform/remote/comfyui/history/reconciliation.test.ts new file mode 100644 index 000000000..cbfe30ba8 --- /dev/null +++ b/tests-ui/tests/platform/remote/comfyui/history/reconciliation.test.ts @@ -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) + }) + }) +}) diff --git a/tests-ui/tests/store/queueStore.test.ts b/tests-ui/tests/store/queueStore.test.ts index 72a398279..e01ec5afd 100644 --- a/tests-ui/tests/store/queueStore.test.ts +++ b/tests-ui/tests/store/queueStore.test.ts @@ -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