mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-05 13:10:24 +00:00
feat(error-groups): sort execution error cards by node execution ID (#9334)
## Summary Sort execution error cards within each error group by their node execution ID in ascending numeric order, ensuring consistent and predictable display order. ## Changes - **What**: Added `compareExecutionId` utility to `src/types/nodeIdentification.ts` that splits node IDs on `:` and compares segments numerically left-to-right; applied it as a sort comparator when building `ErrorGroup.cards` in `useErrorGroups.ts` ## Review Focus - The comparison treats missing segments as `0`, so `"1"` sorts before `"1:20"` (subgraph nodes follow their parent); confirm this ordering matches user expectations - All comparisons are purely numeric — non-numeric segment values would sort as `NaN` (treated as `0`) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9334-feat-error-groups-sort-execution-error-cards-by-node-execution-ID-3176d73d365081e1b3e4e4fa8831fe16) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -323,6 +323,90 @@ describe('useErrorGroups', () => {
|
||||
)
|
||||
expect(promptGroup).toBeDefined()
|
||||
})
|
||||
|
||||
it('sorts cards within an execution group by nodeId numerically', async () => {
|
||||
const { store, groups } = createErrorGroups()
|
||||
store.lastNodeErrors = {
|
||||
'10': {
|
||||
class_type: 'KSampler',
|
||||
dependent_outputs: [],
|
||||
errors: [{ type: 'err', message: 'Error', details: '' }]
|
||||
},
|
||||
'2': {
|
||||
class_type: 'KSampler',
|
||||
dependent_outputs: [],
|
||||
errors: [{ type: 'err', message: 'Error', details: '' }]
|
||||
},
|
||||
'1': {
|
||||
class_type: 'KSampler',
|
||||
dependent_outputs: [],
|
||||
errors: [{ type: 'err', message: 'Error', details: '' }]
|
||||
}
|
||||
}
|
||||
await nextTick()
|
||||
|
||||
const execGroup = groups.allErrorGroups.value.find(
|
||||
(g) => g.type === 'execution'
|
||||
)
|
||||
const nodeIds = execGroup?.cards.map((c) => c.nodeId)
|
||||
expect(nodeIds).toEqual(['1', '2', '10'])
|
||||
})
|
||||
|
||||
it('sorts cards with subpath nodeIds before higher root IDs', async () => {
|
||||
const { store, groups } = createErrorGroups()
|
||||
store.lastNodeErrors = {
|
||||
'2': {
|
||||
class_type: 'KSampler',
|
||||
dependent_outputs: [],
|
||||
errors: [{ type: 'err', message: 'Error', details: '' }]
|
||||
},
|
||||
'1:20': {
|
||||
class_type: 'KSampler',
|
||||
dependent_outputs: [],
|
||||
errors: [{ type: 'err', message: 'Error', details: '' }]
|
||||
},
|
||||
'1': {
|
||||
class_type: 'KSampler',
|
||||
dependent_outputs: [],
|
||||
errors: [{ type: 'err', message: 'Error', details: '' }]
|
||||
}
|
||||
}
|
||||
await nextTick()
|
||||
|
||||
const execGroup = groups.allErrorGroups.value.find(
|
||||
(g) => g.type === 'execution'
|
||||
)
|
||||
const nodeIds = execGroup?.cards.map((c) => c.nodeId)
|
||||
expect(nodeIds).toEqual(['1', '1:20', '2'])
|
||||
})
|
||||
|
||||
it('sorts deeply nested nodeIds by each segment numerically', async () => {
|
||||
const { store, groups } = createErrorGroups()
|
||||
store.lastNodeErrors = {
|
||||
'10:11:99': {
|
||||
class_type: 'KSampler',
|
||||
dependent_outputs: [],
|
||||
errors: [{ type: 'err', message: 'Error', details: '' }]
|
||||
},
|
||||
'10:11:12': {
|
||||
class_type: 'KSampler',
|
||||
dependent_outputs: [],
|
||||
errors: [{ type: 'err', message: 'Error', details: '' }]
|
||||
},
|
||||
'10:2': {
|
||||
class_type: 'KSampler',
|
||||
dependent_outputs: [],
|
||||
errors: [{ type: 'err', message: 'Error', details: '' }]
|
||||
}
|
||||
}
|
||||
await nextTick()
|
||||
|
||||
const execGroup = groups.allErrorGroups.value.find(
|
||||
(g) => g.type === 'execution'
|
||||
)
|
||||
const nodeIds = execGroup?.cards.map((c) => c.nodeId)
|
||||
expect(nodeIds).toEqual(['10:2', '10:11:12', '10:11:99'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('filteredGroups', () => {
|
||||
|
||||
@@ -23,7 +23,10 @@ import { st } from '@/i18n'
|
||||
import type { MissingNodeType } from '@/types/comfy'
|
||||
import type { ErrorCardData, ErrorGroup, ErrorItem } from './types'
|
||||
import type { NodeExecutionId } from '@/types/nodeIdentification'
|
||||
import { isNodeExecutionId } from '@/types/nodeIdentification'
|
||||
import {
|
||||
isNodeExecutionId,
|
||||
compareExecutionId
|
||||
} from '@/types/nodeIdentification'
|
||||
|
||||
const PROMPT_CARD_ID = '__prompt__'
|
||||
const SINGLE_GROUP_KEY = '__single__'
|
||||
@@ -151,12 +154,16 @@ function addCardErrorToGroup(
|
||||
group.get(card.id)?.errors.push(error)
|
||||
}
|
||||
|
||||
function compareNodeId(a: ErrorCardData, b: ErrorCardData): number {
|
||||
return compareExecutionId(a.nodeId, b.nodeId)
|
||||
}
|
||||
|
||||
function toSortedGroups(groupsMap: Map<string, GroupEntry>): ErrorGroup[] {
|
||||
return Array.from(groupsMap.entries())
|
||||
.map(([title, groupData]) => ({
|
||||
type: 'execution' as const,
|
||||
title,
|
||||
cards: Array.from(groupData.cards.values()),
|
||||
cards: Array.from(groupData.cards.values()).sort(compareNodeId),
|
||||
priority: groupData.priority
|
||||
}))
|
||||
.sort((a, b) => {
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'
|
||||
|
||||
import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import {
|
||||
compareExecutionId,
|
||||
createNodeExecutionId,
|
||||
createNodeLocatorId,
|
||||
getAncestorExecutionIds,
|
||||
@@ -232,4 +233,45 @@ describe('nodeIdentification', () => {
|
||||
expect(getParentExecutionIds('65:70:63')).toEqual(['65', '65:70'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('compareExecutionId', () => {
|
||||
it('sorts simple numeric IDs in ascending order', () => {
|
||||
expect(compareExecutionId('1', '2')).toBeLessThan(0)
|
||||
expect(compareExecutionId('2', '1')).toBeGreaterThan(0)
|
||||
expect(compareExecutionId('5', '5')).toBe(0)
|
||||
})
|
||||
|
||||
it('compares nested IDs left-to-right by segment', () => {
|
||||
// "1" < "1:20" < "2" < "10:11:12" as documented
|
||||
expect(compareExecutionId('1', '1:20')).toBeLessThan(0)
|
||||
expect(compareExecutionId('1:20', '2')).toBeLessThan(0)
|
||||
expect(compareExecutionId('2', '10:11:12')).toBeLessThan(0)
|
||||
})
|
||||
|
||||
it('treats a shorter ID as having trailing segment 0 when comparing', () => {
|
||||
// "5" vs "5:0" → first segments equal, second: 0 vs 0 → equal
|
||||
expect(compareExecutionId('5', '5:0')).toBe(0)
|
||||
// "5" vs "5:1" → second segment 0 < 1
|
||||
expect(compareExecutionId('5', '5:1')).toBeLessThan(0)
|
||||
})
|
||||
|
||||
it('handles undefined inputs by treating them as empty string (segment 0)', () => {
|
||||
expect(compareExecutionId(undefined, '1')).toBeLessThan(0)
|
||||
expect(compareExecutionId('1', undefined)).toBeGreaterThan(0)
|
||||
expect(compareExecutionId(undefined, undefined)).toBe(0)
|
||||
})
|
||||
|
||||
it('handles empty string inputs', () => {
|
||||
expect(compareExecutionId('', '1')).toBeLessThan(0)
|
||||
expect(compareExecutionId('1', '')).toBeGreaterThan(0)
|
||||
expect(compareExecutionId('', '')).toBe(0)
|
||||
})
|
||||
|
||||
it('treats non-numeric segments as 0 via NaN guard', () => {
|
||||
// Number('foo') → NaN → treated as 0
|
||||
expect(compareExecutionId('foo', '1')).toBeLessThan(0)
|
||||
expect(compareExecutionId('foo', '0')).toBe(0)
|
||||
expect(compareExecutionId('1:foo', '1:0')).toBe(0)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -146,3 +146,26 @@ export function getParentExecutionIds(
|
||||
): NodeExecutionId[] {
|
||||
return getAncestorExecutionIds(executionId).slice(0, -1)
|
||||
}
|
||||
|
||||
/**
|
||||
* Compare two NodeExecutionIds for ascending numeric sort order.
|
||||
* Splits each ID by ":" and compares segments numerically left to right.
|
||||
*
|
||||
* Example order: "1" < "1:20" < "2" < "10:11:12"
|
||||
*/
|
||||
export function compareExecutionId(
|
||||
a: string | undefined,
|
||||
b: string | undefined
|
||||
): number {
|
||||
const parse = (id: string | undefined) => (id ?? '').split(':').map(Number)
|
||||
const idA = parse(a)
|
||||
const idB = parse(b)
|
||||
for (let i = 0; i < Math.max(idA.length, idB.length); i++) {
|
||||
const segA = idA[i] ?? 0
|
||||
const segB = idB[i] ?? 0
|
||||
const diff =
|
||||
(Number.isNaN(segA) ? 0 : segA) - (Number.isNaN(segB) ? 0 : segB)
|
||||
if (diff !== 0) return diff
|
||||
}
|
||||
return 0
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user