From 7f774542f73a1aeeb2f1f98edc75e0379290e45b Mon Sep 17 00:00:00 2001 From: Jacob Segal Date: Fri, 11 Jul 2025 19:04:48 -0700 Subject: [PATCH] Rename `HierarchicalNodeId` to `NodeExecutionId` --- src/stores/executionStore.ts | 21 +++-- src/stores/workflowStore.ts | 49 ++++++------ src/types/index.ts | 8 +- src/types/nodeIdentification.ts | 34 ++++----- tests-ui/tests/store/executionStore.test.ts | 24 +++--- tests-ui/tests/store/workflowStore.test.ts | 24 +++--- .../tests/types/nodeIdentification.test.ts | 76 +++++++++---------- 7 files changed, 114 insertions(+), 122 deletions(-) diff --git a/src/stores/executionStore.ts b/src/stores/executionStore.ts index 341bbae67..c615bad4e 100644 --- a/src/stores/executionStore.ts +++ b/src/stores/executionStore.ts @@ -56,7 +56,7 @@ export const useExecutionStore = defineStore('execution', () => { /** * Convert execution context node IDs to NodeLocatorIds - * @param nodeId The node ID from execution context (could be hierarchical) + * @param nodeId The node ID from execution context (could be execution ID) * @returns The NodeLocatorId */ const executionIdToNodeLocatorId = ( @@ -69,7 +69,7 @@ export const useExecutionStore = defineStore('execution', () => { return nodeIdStr as NodeLocatorId } - // It's a hierarchical node ID + // It's an execution node ID const parts = nodeIdStr.split(':') const localNodeId = parts[parts.length - 1] const subgraphs = getSubgraphsFromInstanceIds(app.graph, parts) @@ -77,7 +77,7 @@ export const useExecutionStore = defineStore('execution', () => { return nodeLocatorId } - const mergeHierarchicalProgressStates = ( + const mergeExecutionProgressStates = ( currentState: NodeProgressState | undefined, newState: NodeProgressState ): NodeProgressState => { @@ -119,7 +119,7 @@ export const useExecutionStore = defineStore('execution', () => { const locatorId = executionIdToNodeLocatorId(executionId) if (!locatorId) continue - result[locatorId] = mergeHierarchicalProgressStates( + result[locatorId] = mergeExecutionProgressStates( result[locatorId], state ) @@ -336,7 +336,7 @@ export const useExecutionStore = defineStore('execution', () => { const { nodeId, text } = e.detail if (!text || !nodeId) return - // Handle hierarchical node IDs for subgraphs + // Handle execution node IDs for subgraphs const currentId = getNodeIdIfExecuting(nodeId) const node = canvasStore.getCanvas().graph?.getNodeById(currentId) if (!node) return @@ -347,7 +347,7 @@ export const useExecutionStore = defineStore('execution', () => { function handleDisplayComponent(e: CustomEvent) { const { node_id: nodeId, component, props = {} } = e.detail - // Handle hierarchical node IDs for subgraphs + // Handle execution node IDs for subgraphs const currentId = getNodeIdIfExecuting(nodeId) const node = canvasStore.getCanvas().graph?.getNodeById(currentId) if (!node) return @@ -388,16 +388,15 @@ export const useExecutionStore = defineStore('execution', () => { } /** - * Convert a NodeLocatorId to an execution context ID (hierarchical ID) + * Convert a NodeLocatorId to an execution context ID * @param locatorId The NodeLocatorId - * @returns The hierarchical execution ID or null if conversion fails + * @returns The execution ID or null if conversion fails */ const nodeLocatorIdToExecutionId = ( locatorId: NodeLocatorId | string ): string | null => { - const hierarchicalId = - workflowStore.nodeLocatorIdToHierarchicalId(locatorId) - return hierarchicalId + const executionId = workflowStore.nodeLocatorIdToNodeExecutionId(locatorId) + return executionId } return { diff --git a/src/stores/workflowStore.ts b/src/stores/workflowStore.ts index a27f82644..3de9a2e95 100644 --- a/src/stores/workflowStore.ts +++ b/src/stores/workflowStore.ts @@ -9,14 +9,11 @@ import { api } from '@/scripts/api' import { app as comfyApp } from '@/scripts/app' import { ChangeTracker } from '@/scripts/changeTracker' import { defaultGraphJSON } from '@/scripts/defaultGraph' -import type { - HierarchicalNodeId, - NodeLocatorId -} from '@/types/nodeIdentification' +import type { NodeExecutionId, NodeLocatorId } from '@/types/nodeIdentification' import { - createHierarchicalNodeId, + createNodeExecutionId, createNodeLocatorId, - parseHierarchicalNodeId, + parseNodeExecutionId, parseNodeLocatorId } from '@/types/nodeIdentification' import { getPathDetails } from '@/utils/formatUtil' @@ -175,14 +172,14 @@ export interface WorkflowStore { updateActiveGraph: () => void executionIdToCurrentId: (id: string) => any nodeIdToNodeLocatorId: (nodeId: NodeId, subgraph?: Subgraph) => NodeLocatorId - hierarchicalIdToNodeLocatorId: ( - hierarchicalId: HierarchicalNodeId | string + nodeExecutionIdToNodeLocatorId: ( + nodeExecutionId: NodeExecutionId | string ) => NodeLocatorId | null nodeLocatorIdToNodeId: (locatorId: NodeLocatorId | string) => NodeId | null - nodeLocatorIdToHierarchicalId: ( + nodeLocatorIdToNodeExecutionId: ( locatorId: NodeLocatorId | string, targetSubgraph?: Subgraph - ) => HierarchicalNodeId | null + ) => NodeExecutionId | null } export const useWorkflowStore = defineStore('workflow', () => { @@ -493,7 +490,7 @@ export const useWorkflowStore = defineStore('workflow', () => { return } - // Parse the hierarchical ID (e.g., "123:456:789") + // Parse the execution ID (e.g., "123:456:789") const subgraphNodeIds = id.split(':') // Start from the root graph @@ -528,19 +525,19 @@ export const useWorkflowStore = defineStore('workflow', () => { } /** - * Convert a hierarchical ID to a NodeLocatorId - * @param hierarchicalId The hierarchical node ID (e.g., "123:456:789") + * Convert an execution ID to a NodeLocatorId + * @param nodeExecutionId The execution node ID (e.g., "123:456:789") * @returns The NodeLocatorId or null if conversion fails */ - const hierarchicalIdToNodeLocatorId = ( - hierarchicalId: HierarchicalNodeId | string + const nodeExecutionIdToNodeLocatorId = ( + nodeExecutionId: NodeExecutionId | string ): NodeLocatorId | null => { // Handle simple node IDs (root graph - no colons) - if (!hierarchicalId.includes(':')) { - return hierarchicalId as NodeLocatorId + if (!nodeExecutionId.includes(':')) { + return nodeExecutionId as NodeLocatorId } - const parts = parseHierarchicalNodeId(hierarchicalId) + const parts = parseNodeExecutionId(nodeExecutionId) if (!parts || parts.length === 0) return null const nodeId = parts[parts.length - 1] @@ -576,15 +573,15 @@ export const useWorkflowStore = defineStore('workflow', () => { } /** - * Convert a NodeLocatorId to a hierarchical ID for a specific context + * Convert a NodeLocatorId to an execution ID for a specific context * @param locatorId The NodeLocatorId * @param targetSubgraph The subgraph context (defaults to active subgraph) - * @returns The hierarchical ID or null if the node is not accessible from the target context + * @returns The execution ID or null if the node is not accessible from the target context */ - const nodeLocatorIdToHierarchicalId = ( + const nodeLocatorIdToNodeExecutionId = ( locatorId: NodeLocatorId | string, targetSubgraph?: Subgraph - ): HierarchicalNodeId | null => { + ): NodeExecutionId | null => { const parsed = parseNodeLocatorId(locatorId) if (!parsed) return null @@ -592,7 +589,7 @@ export const useWorkflowStore = defineStore('workflow', () => { // If no subgraph UUID, this is a root graph node if (!subgraphUuid) { - return String(localNodeId) as HierarchicalNodeId + return String(localNodeId) as NodeExecutionId } // Find the path from root to the subgraph with this UUID @@ -635,7 +632,7 @@ export const useWorkflowStore = defineStore('workflow', () => { return null } - return createHierarchicalNodeId([...path, localNodeId]) + return createNodeExecutionId([...path, localNodeId]) } return { @@ -666,9 +663,9 @@ export const useWorkflowStore = defineStore('workflow', () => { updateActiveGraph, executionIdToCurrentId, nodeIdToNodeLocatorId, - hierarchicalIdToNodeLocatorId, + nodeExecutionIdToNodeLocatorId, nodeLocatorIdToNodeId, - nodeLocatorIdToHierarchicalId + nodeLocatorIdToNodeExecutionId } }) satisfies () => WorkflowStore diff --git a/src/types/index.ts b/src/types/index.ts index 2371a434f..377581465 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -33,13 +33,13 @@ export type { ComfyNodeDef } from '@/schemas/nodeDefSchema' export type { InputSpec } from '@/schemas/nodeDefSchema' export type { NodeLocatorId, - HierarchicalNodeId, + NodeExecutionId, isNodeLocatorId, - isHierarchicalNodeId, + isNodeExecutionId, parseNodeLocatorId, createNodeLocatorId, - parseHierarchicalNodeId, - createHierarchicalNodeId + parseNodeExecutionId, + createNodeExecutionId } from './nodeIdentification' export type { EmbeddingsResponse, diff --git a/src/types/nodeIdentification.ts b/src/types/nodeIdentification.ts index a4edafd97..13d193c6e 100644 --- a/src/types/nodeIdentification.ts +++ b/src/types/nodeIdentification.ts @@ -12,19 +12,19 @@ import type { NodeId } from '@/schemas/comfyWorkflowSchema' * - "a1b2c3d4-e5f6-7890-abcd-ef1234567890:123" (node in subgraph) * - "456" (node in root graph) * - * Unlike hierarchical IDs which change based on the instance path, + * Unlike execution IDs which change based on the instance path, * NodeLocatorId remains the same for all instances of a particular node. */ export type NodeLocatorId = string /** - * A hierarchical identifier representing a node's position in nested subgraphs. + * An execution identifier representing a node's position in nested subgraphs. * Also known as ExecutionId in some contexts. * * Format: Colon-separated path of node IDs * Example: "123:456:789" (node 789 in subgraph 456 in subgraph 123) */ -export type HierarchicalNodeId = string +export type NodeExecutionId = string /** * Type guard to check if a value is a NodeLocatorId @@ -52,13 +52,11 @@ export function isNodeLocatorId(value: unknown): value is NodeLocatorId { } /** - * Type guard to check if a value is a HierarchicalNodeId + * Type guard to check if a value is a NodeExecutionId */ -export function isHierarchicalNodeId( - value: unknown -): value is HierarchicalNodeId { +export function isNodeExecutionId(value: unknown): value is NodeExecutionId { if (typeof value !== 'string') return false - // Must contain at least one colon to be hierarchical + // Must contain at least one colon to be an execution ID return value.includes(':') } @@ -103,12 +101,12 @@ export function createNodeLocatorId( } /** - * Parse a HierarchicalNodeId into its component node IDs - * @param id The HierarchicalNodeId to parse - * @returns Array of node IDs from root to target, or null if not hierarchical + * Parse a NodeExecutionId into its component node IDs + * @param id The NodeExecutionId to parse + * @returns Array of node IDs from root to target, or null if not an execution ID */ -export function parseHierarchicalNodeId(id: string): NodeId[] | null { - if (!isHierarchicalNodeId(id)) return null +export function parseNodeExecutionId(id: string): NodeId[] | null { + if (!isNodeExecutionId(id)) return null return id .split(':') @@ -116,12 +114,10 @@ export function parseHierarchicalNodeId(id: string): NodeId[] | null { } /** - * Create a HierarchicalNodeId from an array of node IDs + * Create a NodeExecutionId from an array of node IDs * @param nodeIds Array of node IDs from root to target - * @returns A properly formatted HierarchicalNodeId + * @returns A properly formatted NodeExecutionId */ -export function createHierarchicalNodeId( - nodeIds: NodeId[] -): HierarchicalNodeId { - return nodeIds.join(':') as HierarchicalNodeId +export function createNodeExecutionId(nodeIds: NodeId[]): NodeExecutionId { + return nodeIds.join(':') as NodeExecutionId } diff --git a/tests-ui/tests/store/executionStore.test.ts b/tests-ui/tests/store/executionStore.test.ts index 5c29a3fd8..d34bf4867 100644 --- a/tests-ui/tests/store/executionStore.test.ts +++ b/tests-ui/tests/store/executionStore.test.ts @@ -8,9 +8,9 @@ import { useWorkflowStore } from '@/stores/workflowStore' // Mock the workflowStore vi.mock('@/stores/workflowStore', () => ({ useWorkflowStore: vi.fn(() => ({ - hierarchicalIdToNodeLocatorId: vi.fn(), + nodeExecutionIdToNodeLocatorId: vi.fn(), nodeIdToNodeLocatorId: vi.fn(), - nodeLocatorIdToHierarchicalId: vi.fn() + nodeLocatorIdToNodeExecutionId: vi.fn() })) })) @@ -105,9 +105,9 @@ describe('useExecutionStore - NodeLocatorId conversions', () => { // Create the mock workflowStore instance const mockWorkflowStore = { - hierarchicalIdToNodeLocatorId: vi.fn(), + nodeExecutionIdToNodeLocatorId: vi.fn(), nodeIdToNodeLocatorId: vi.fn(), - nodeLocatorIdToHierarchicalId: vi.fn() + nodeLocatorIdToNodeExecutionId: vi.fn() } // Mock the useWorkflowStore function to return our mock @@ -119,7 +119,7 @@ describe('useExecutionStore - NodeLocatorId conversions', () => { }) describe('executionIdToNodeLocatorId', () => { - it('should convert hierarchical execution ID to NodeLocatorId', () => { + it('should convert execution ID to NodeLocatorId', () => { // Mock subgraph structure const mockSubgraph = { id: 'a1b2c3d4-e5f6-7890-abcd-ef1234567890', @@ -166,24 +166,24 @@ describe('useExecutionStore - NodeLocatorId conversions', () => { }) describe('nodeLocatorIdToExecutionId', () => { - it('should convert NodeLocatorId to hierarchical execution ID', () => { - const mockHierarchicalId = '123:456' - vi.spyOn(workflowStore, 'nodeLocatorIdToHierarchicalId').mockReturnValue( - mockHierarchicalId as any + it('should convert NodeLocatorId to execution ID', () => { + const mockExecutionId = '123:456' + vi.spyOn(workflowStore, 'nodeLocatorIdToNodeExecutionId').mockReturnValue( + mockExecutionId as any ) const result = store.nodeLocatorIdToExecutionId( 'a1b2c3d4-e5f6-7890-abcd-ef1234567890:456' ) - expect(workflowStore.nodeLocatorIdToHierarchicalId).toHaveBeenCalledWith( + expect(workflowStore.nodeLocatorIdToNodeExecutionId).toHaveBeenCalledWith( 'a1b2c3d4-e5f6-7890-abcd-ef1234567890:456' ) - expect(result).toBe(mockHierarchicalId) + expect(result).toBe(mockExecutionId) }) it('should return null when conversion fails', () => { - vi.spyOn(workflowStore, 'nodeLocatorIdToHierarchicalId').mockReturnValue( + vi.spyOn(workflowStore, 'nodeLocatorIdToNodeExecutionId').mockReturnValue( null ) diff --git a/tests-ui/tests/store/workflowStore.test.ts b/tests-ui/tests/store/workflowStore.test.ts index be6dcb5aa..579f330ff 100644 --- a/tests-ui/tests/store/workflowStore.test.ts +++ b/tests-ui/tests/store/workflowStore.test.ts @@ -641,19 +641,19 @@ describe('useWorkflowStore', () => { }) }) - describe('hierarchicalIdToNodeLocatorId', () => { - it('should convert hierarchical ID to NodeLocatorId', () => { - const result = store.hierarchicalIdToNodeLocatorId('123:456') + describe('nodeExecutionIdToNodeLocatorId', () => { + it('should convert execution ID to NodeLocatorId', () => { + const result = store.nodeExecutionIdToNodeLocatorId('123:456') expect(result).toBe('a1b2c3d4-e5f6-7890-abcd-ef1234567890:456') }) it('should return simple node ID for root level nodes', () => { - const result = store.hierarchicalIdToNodeLocatorId('123') + const result = store.nodeExecutionIdToNodeLocatorId('123') expect(result).toBe('123') }) - it('should return null for invalid hierarchical IDs', () => { - const result = store.hierarchicalIdToNodeLocatorId('999:456') + it('should return null for invalid execution IDs', () => { + const result = store.nodeExecutionIdToNodeLocatorId('999:456') expect(result).toBeNull() }) }) @@ -687,33 +687,33 @@ describe('useWorkflowStore', () => { }) }) - describe('nodeLocatorIdToHierarchicalId', () => { - it('should convert NodeLocatorId to hierarchical ID', () => { + describe('nodeLocatorIdToNodeExecutionId', () => { + it('should convert NodeLocatorId to execution ID', () => { // Need to mock isSubgraph to identify our mockSubgraph vi.mocked(isSubgraph).mockImplementation((obj): obj is Subgraph => { return obj === store.activeSubgraph }) - const result = store.nodeLocatorIdToHierarchicalId( + const result = store.nodeLocatorIdToNodeExecutionId( 'a1b2c3d4-e5f6-7890-abcd-ef1234567890:456' ) expect(result).toBe('123:456') }) it('should handle simple node IDs (root graph)', () => { - const result = store.nodeLocatorIdToHierarchicalId('123') + const result = store.nodeLocatorIdToNodeExecutionId('123') expect(result).toBe('123') }) it('should return null for unknown subgraph UUID', () => { - const result = store.nodeLocatorIdToHierarchicalId( + const result = store.nodeLocatorIdToNodeExecutionId( 'unknown-uuid-1234-5678-90ab-cdef12345678:456' ) expect(result).toBeNull() }) it('should return null for invalid NodeLocatorId', () => { - const result = store.nodeLocatorIdToHierarchicalId('invalid:format') + const result = store.nodeLocatorIdToNodeExecutionId('invalid:format') expect(result).toBeNull() }) }) diff --git a/tests-ui/tests/types/nodeIdentification.test.ts b/tests-ui/tests/types/nodeIdentification.test.ts index ae9f302ea..0d9aa647b 100644 --- a/tests-ui/tests/types/nodeIdentification.test.ts +++ b/tests-ui/tests/types/nodeIdentification.test.ts @@ -3,11 +3,11 @@ import { describe, expect, it } from 'vitest' import type { NodeId } from '@/schemas/comfyWorkflowSchema' import { type NodeLocatorId, - createHierarchicalNodeId, + createNodeExecutionId, createNodeLocatorId, - isHierarchicalNodeId, + isNodeExecutionId, isNodeLocatorId, - parseHierarchicalNodeId, + parseNodeExecutionId, parseNodeLocatorId } from '@/types/nodeIdentification' @@ -118,66 +118,66 @@ describe('nodeIdentification', () => { }) }) - describe('HierarchicalNodeId', () => { - describe('isHierarchicalNodeId', () => { - it('should return true for hierarchical IDs', () => { - expect(isHierarchicalNodeId('123:456')).toBe(true) - expect(isHierarchicalNodeId('123:456:789')).toBe(true) - expect(isHierarchicalNodeId('node_1:node_2')).toBe(true) + describe('NodeExecutionId', () => { + describe('isNodeExecutionId', () => { + it('should return true for execution IDs', () => { + expect(isNodeExecutionId('123:456')).toBe(true) + expect(isNodeExecutionId('123:456:789')).toBe(true) + expect(isNodeExecutionId('node_1:node_2')).toBe(true) }) - it('should return false for non-hierarchical IDs', () => { - expect(isHierarchicalNodeId('123')).toBe(false) - expect(isHierarchicalNodeId('node_1')).toBe(false) - expect(isHierarchicalNodeId('')).toBe(false) - expect(isHierarchicalNodeId(123)).toBe(false) - expect(isHierarchicalNodeId(null)).toBe(false) - expect(isHierarchicalNodeId(undefined)).toBe(false) + it('should return false for non-execution IDs', () => { + expect(isNodeExecutionId('123')).toBe(false) + expect(isNodeExecutionId('node_1')).toBe(false) + expect(isNodeExecutionId('')).toBe(false) + expect(isNodeExecutionId(123)).toBe(false) + expect(isNodeExecutionId(null)).toBe(false) + expect(isNodeExecutionId(undefined)).toBe(false) }) }) - describe('parseHierarchicalNodeId', () => { - it('should parse hierarchical IDs correctly', () => { - expect(parseHierarchicalNodeId('123:456')).toEqual([123, 456]) - expect(parseHierarchicalNodeId('123:456:789')).toEqual([123, 456, 789]) - expect(parseHierarchicalNodeId('node_1:node_2')).toEqual([ + describe('parseNodeExecutionId', () => { + it('should parse execution IDs correctly', () => { + expect(parseNodeExecutionId('123:456')).toEqual([123, 456]) + expect(parseNodeExecutionId('123:456:789')).toEqual([123, 456, 789]) + expect(parseNodeExecutionId('node_1:node_2')).toEqual([ 'node_1', 'node_2' ]) - expect(parseHierarchicalNodeId('123:node_2:456')).toEqual([ + expect(parseNodeExecutionId('123:node_2:456')).toEqual([ 123, 'node_2', 456 ]) }) - it('should return null for non-hierarchical IDs', () => { - expect(parseHierarchicalNodeId('123')).toBeNull() - expect(parseHierarchicalNodeId('')).toBeNull() + it('should return null for non-execution IDs', () => { + expect(parseNodeExecutionId('123')).toBeNull() + expect(parseNodeExecutionId('')).toBeNull() }) }) - describe('createHierarchicalNodeId', () => { - it('should create hierarchical IDs from node arrays', () => { - expect(createHierarchicalNodeId([123, 456])).toBe('123:456') - expect(createHierarchicalNodeId([123, 456, 789])).toBe('123:456:789') - expect(createHierarchicalNodeId(['node_1', 'node_2'])).toBe( + describe('createNodeExecutionId', () => { + it('should create execution IDs from node arrays', () => { + expect(createNodeExecutionId([123, 456])).toBe('123:456') + expect(createNodeExecutionId([123, 456, 789])).toBe('123:456:789') + expect(createNodeExecutionId(['node_1', 'node_2'])).toBe( 'node_1:node_2' ) - expect(createHierarchicalNodeId([123, 'node_2', 456])).toBe( + expect(createNodeExecutionId([123, 'node_2', 456])).toBe( '123:node_2:456' ) }) it('should handle single node ID', () => { - const result = createHierarchicalNodeId([123]) + const result = createNodeExecutionId([123]) expect(result).toBe('123') - // Single node IDs are not hierarchical - expect(isHierarchicalNodeId(result)).toBe(false) + // Single node IDs are not execution IDs + expect(isNodeExecutionId(result)).toBe(false) }) it('should handle empty array', () => { - expect(createHierarchicalNodeId([])).toBe('') + expect(createNodeExecutionId([])).toBe('') }) }) }) @@ -195,11 +195,11 @@ describe('nodeIdentification', () => { expect(parsed!.localNodeId).toBe(nodeId) }) - it('should round-trip HierarchicalNodeId correctly', () => { + it('should round-trip NodeExecutionId correctly', () => { const nodeIds: NodeId[] = [123, 'node_2', 456] - const hierarchicalId = createHierarchicalNodeId(nodeIds) - const parsed = parseHierarchicalNodeId(hierarchicalId) + const executionId = createNodeExecutionId(nodeIds) + const parsed = parseNodeExecutionId(executionId) expect(parsed).toEqual(nodeIds) })