code improve

src/core/graph/operations/IGraphMutationService.ts

@@ -9,9 +9,11 @@ import type {
   CreateGroupParams,
   CreateNodeParams,
   CreateSubgraphParams,
+  CreateSubgraphResult,
   DisconnectParams,
   GraphMutationOperation,
   NodeInputSlotParams,
+  OperationResultType,
   Result,
   SubgraphIndexParams,
   SubgraphNameTypeParams,
@@ -26,15 +28,15 @@ import type { RerouteId } from '@/lib/litegraph/src/Reroute'
 import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSchema'
 
 export interface IGraphMutationService {
-  applyOperation(
-    operation: GraphMutationOperation
-  ): Promise<Result<any, GraphMutationError>>
+  applyOperation<T extends GraphMutationOperation>(
+    operation: T
+  ): Promise<Result<OperationResultType<T>, GraphMutationError>>
 
   createNode(
     params: CreateNodeParams
   ): Promise<Result<NodeId, GraphMutationError>>
 
-  getNodeById(nodeId: NodeId): LGraphNode
+  getNodeById(nodeId: NodeId): Promise<Result<LGraphNode, GraphMutationError>>
 
   removeNode(nodeId: NodeId): Promise<Result<void, GraphMutationError>>
 
@@ -106,15 +108,9 @@ export interface IGraphMutationService {
     params: NodeInputSlotParams
   ): Promise<Result<void, GraphMutationError>>
 
-  createSubgraph(params: CreateSubgraphParams): Promise<
-    Result<
-      {
-        subgraph: any
-        node: any
-      },
-      GraphMutationError
-    >
-  >
+  createSubgraph(
+    params: CreateSubgraphParams
+  ): Promise<Result<CreateSubgraphResult, GraphMutationError>>
 
   unpackSubgraph(
     subgraphNodeId: NodeId

src/core/graph/operations/graphMutationService.ts

@@ -10,9 +10,11 @@ import type {
   CreateGroupParams,
   CreateNodeParams,
   CreateSubgraphParams,
+  CreateSubgraphResult,
   DisconnectParams,
   GraphMutationOperation,
   NodeInputSlotParams,
+  OperationResultType,
   Result,
   SubgraphIndexParams,
   SubgraphNameTypeParams,
@@ -50,13 +52,27 @@ export class GraphMutationService implements IGraphMutationService {
     return app.graph
   }
 
+  private getCanvas() {
+    return app.canvas
+  }
+
   private getChangeTracker() {
     return this.workflowStore.activeWorkflow?.changeTracker
   }
 
-  async applyOperation(
+  async applyOperation<T extends GraphMutationOperation>(
+    operation: T
+  ): Promise<Result<OperationResultType<T>, GraphMutationError>> {
+    const result = await this._executeOperation(operation)
+
+    return result as Result<OperationResultType<T>, GraphMutationError>
+  }
+
+  private async _executeOperation(
     operation: GraphMutationOperation
-  ): Promise<Result<any, GraphMutationError>> {
+  ): Promise<
+    Result<OperationResultType<typeof operation>, GraphMutationError>
+  > {
     switch (operation.type) {
       case 'createNode':
         return await this.createNode(operation.params)
@@ -127,7 +143,7 @@ export class GraphMutationService implements IGraphMutationService {
       case 'redo':
         return await this.redo()
       default: {
-        const unknownOp = operation as any
+        const unknownOp = operation as { type: string }
         console.warn('Unknown operation type:', unknownOp)
         return {
           success: false,
@@ -146,24 +162,15 @@ export class GraphMutationService implements IGraphMutationService {
     params: CreateNodeParams
   ): Promise<Result<NodeId, GraphMutationError>> {
     try {
-      const { type, properties, title, id } = params
+      const { type, properties, title } = params
       const graph = this.getGraph()
 
-      const node = LiteGraph.createNode(type)
+      const node = LiteGraph.createNode(type, title)
 
       if (!node) {
         throw new Error(`Failed to create node of type: ${type}`)
       }
 
-      // Set custom ID if provided (for loading workflows)
-      if (id !== undefined) {
-        node.id = id
-      }
-
-      if (title) {
-        node.title = title
-      }
-
       if (properties) {
         Object.assign(node.properties || {}, properties)
       }
@@ -190,15 +197,26 @@ export class GraphMutationService implements IGraphMutationService {
     }
   }
 
-  getNodeById(nodeId: NodeId): LGraphNode {
-    const graph = this.getGraph()
-    const node = graph.getNodeById(nodeId)
+  getNodeById(nodeId: NodeId): Promise<Result<LGraphNode, GraphMutationError>> {
+    try {
+      const graph = this.getGraph()
+      const node = graph.getNodeById(nodeId)
 
-    if (!node) {
-      throw new Error(`Node with id ${nodeId} not found`)
+      if (!node) {
+        throw new Error(`Node with id ${nodeId} not found`)
+      }
+
+      return Promise.resolve({ success: true, data: node })
+    } catch (error) {
+      return Promise.resolve({
+        success: false,
+        error: new GraphMutationError('Failed to get node by id', {
+          operation: 'getNodeById',
+          params: nodeId,
+          cause: error
+        })
+      })
     }
-
-    return node
   }
 
   async removeNode(nodeId: NodeId): Promise<Result<void, GraphMutationError>> {
@@ -1004,15 +1022,9 @@ export class GraphMutationService implements IGraphMutationService {
     }
   }
 
-  async createSubgraph(params: CreateSubgraphParams): Promise<
-    Result<
-      {
-        subgraph: any
-        node: any
-      },
-      GraphMutationError
-    >
-  > {
+  async createSubgraph(
+    params: CreateSubgraphParams
+  ): Promise<Result<CreateSubgraphResult, GraphMutationError>> {
     try {
       const graph = this.getGraph()
 
@@ -1197,12 +1209,15 @@ export class GraphMutationService implements IGraphMutationService {
 
   async clearGraph(): Promise<Result<void, GraphMutationError>> {
     try {
-      // No params to validate for clear operation
       const graph = this.getGraph()
+      const canvas = this.getCanvas()
 
-      graph.beforeChange()
-      graph.clear()
-      graph.afterChange()
+      //TODO same behavior as app.clear() to skip if it's a subgraph
+      if (graph && !canvas.subgraph) {
+        graph.beforeChange()
+        graph.clear()
+        graph.afterChange()
+      }
       return { success: true, data: undefined }
     } catch (error) {
       return {

src/core/graph/operations/types.ts

@@ -4,10 +4,15 @@
  * Defines command types for graph mutation operations with CRDT support.
  * Each command represents an atomic operation that can be applied, undone, and synchronized.
  */
-import type { GroupId } from '@/lib/litegraph/src/LGraphGroup'
+import type { Subgraph } from '@/lib/litegraph/src/LGraph'
+import type { GroupId, LGraphGroup } from '@/lib/litegraph/src/LGraphGroup'
+import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
 import type { LinkId } from '@/lib/litegraph/src/LLink'
 import type { RerouteId } from '@/lib/litegraph/src/Reroute'
-import type { SubgraphId } from '@/lib/litegraph/src/subgraph/SubgraphNode'
+import type {
+  SubgraphId,
+  SubgraphNode
+} from '@/lib/litegraph/src/subgraph/SubgraphNode'
 import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSchema'
 
 export type Result<T, E> =
@@ -16,15 +21,14 @@ export type Result<T, E> =
 
 export interface CreateNodeParams {
   type: string
-  properties?: Record<string, any>
+  properties?: Record<string, string | number | boolean | object>
   title?: string
-  id?: NodeId // Support custom ID for loading workflows
 }
 
 export interface UpdateNodePropertyParams {
   nodeId: NodeId
   property: string
-  value: any
+  value: string | number | boolean | object | string[] | undefined
 }
 
 export interface UpdateNodeTitleParams {
@@ -78,18 +82,18 @@ export interface AddNodeInputParams {
   nodeId: NodeId
   name: string
   type: string
-  extra_info?: Record<string, any>
+  extra_info?: Record<string, unknown>
 }
 
 export interface AddNodeOutputParams {
   nodeId: NodeId
   name: string
   type: string
-  extra_info?: Record<string, any>
+  extra_info?: Record<string, unknown>
 }
 
 export interface CreateSubgraphParams {
-  selectedItems: Set<any>
+  selectedItems: Set<LGraphNode | LGraphGroup>
 }
 
 export interface NodeInputSlotParams {
@@ -113,7 +117,7 @@ export enum CommandOrigin {
 }
 
 export type GraphMutationOperation =
-  | createNodeCommand
+  | CreateNodeCommand
   | RemoveNodeCommand
   | UpdateNodePropertyCommand
   | UpdateNodeTitleCommand
@@ -145,10 +149,8 @@ export type GraphMutationOperation =
   | RemoveSubgraphInputCommand
   | RemoveSubgraphOutputCommand
   | ClearGraphCommand
-  | bypassNodeCommand
-  | unbypassNodeCommand
-  | undoCommand
-  | redoCommand
+  | UndoCommand
+  | RedoCommand
 
 interface GraphOpBase {
   /** Timestamp for ordering commands */
@@ -157,7 +159,7 @@ interface GraphOpBase {
   origin: CommandOrigin
 }
 
-export interface createNodeCommand extends GraphOpBase {
+export interface CreateNodeCommand extends GraphOpBase {
   type: 'createNode'
   params: CreateNodeParams
 }
@@ -316,20 +318,50 @@ export interface ClearGraphCommand extends GraphOpBase {
   type: 'clearGraph'
 }
 
-export interface bypassNodeCommand extends GraphOpBase {
-  type: 'bypassNode'
-  params: NodeId
-}
-
-export interface unbypassNodeCommand extends GraphOpBase {
-  type: 'unbypassNode'
-  params: NodeId
-}
-
-export interface undoCommand extends GraphOpBase {
+export interface UndoCommand extends GraphOpBase {
   type: 'undo'
 }
 
-export interface redoCommand extends GraphOpBase {
+export interface RedoCommand extends GraphOpBase {
   type: 'redo'
 }
+
+export type NodeIdReturnOperations = CreateNodeCommand | CloneNodeCommand
+
+export type LinkIdReturnOperations = ConnectCommand
+
+export type BooleanReturnOperations = DisconnectCommand
+
+export type GroupIdReturnOperations = CreateGroupCommand
+
+export type RerouteIdReturnOperations = AddRerouteCommand
+
+export type NodeIdArrayReturnOperations = PasteNodesCommand
+
+export type NumberReturnOperations =
+  | AddSubgraphNodeInputCommand
+  | AddSubgraphNodeOutputCommand
+
+export interface CreateSubgraphResult {
+  subgraph: Subgraph
+  node: SubgraphNode
+}
+
+export type OperationResultType<T extends GraphMutationOperation> =
+  T extends NodeIdReturnOperations
+    ? NodeId
+    : T extends LinkIdReturnOperations
+      ? LinkId
+      : T extends BooleanReturnOperations
+        ? boolean
+        : T extends GroupIdReturnOperations
+          ? GroupId
+          : T extends RerouteIdReturnOperations
+            ? RerouteId
+            : T extends NodeIdArrayReturnOperations
+              ? NodeId[]
+              : T extends NumberReturnOperations
+                ? number
+                : T extends CreateSubgraphCommand
+                  ? CreateSubgraphResult
+                  : void

tests-ui/tests/services/graphMutationService.test.ts

@@ -10,6 +10,8 @@ import {
   CommandOrigin,
   type GraphMutationOperation
 } from '@/core/graph/operations/types'
+import type { LGraphGroup } from '@/lib/litegraph/src/LGraphGroup'
+import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
 
 const mockGraph = vi.hoisted(() => ({
   beforeChange: vi.fn(),
@@ -21,8 +23,8 @@ const mockGraph = vi.hoisted(() => ({
   clear: vi.fn(),
   setDirtyCanvas: vi.fn(),
   _links: new Map(),
-  _groups: [] as any[],
-  _nodes: [] as any[],
+  _groups: [] as LGraphGroup[],
+  _nodes: [] as LGraphNode[],
   reroutes: new Map(),
   createReroute: vi.fn(),
   removeReroute: vi.fn(),
@@ -34,7 +36,8 @@ const mockGraph = vi.hoisted(() => ({
 
 const mockApp = vi.hoisted(() => ({
   graph: null as any,
-  changeTracker: null as any
+  changeTracker: null as any,
+  canvas: null as any
 }))
 
 Object.defineProperty(mockApp, 'graph', {
@@ -45,6 +48,10 @@ Object.defineProperty(mockApp, 'changeTracker', {
   writable: true,
   value: null
 })
+Object.defineProperty(mockApp, 'canvas', {
+  writable: true,
+  value: null
+})
 
 const mockWorkflowStore = vi.hoisted(() => ({
   activeWorkflow: {
@@ -103,6 +110,7 @@ vi.mock('@/scripts/app', () => ({
 
 mockApp.graph = mockGraph
 mockApp.changeTracker = mockWorkflowStore.activeWorkflow.changeTracker
+mockApp.canvas = { subgraph: null }
 
 vi.mock('@/platform/workflow/management/stores/workflowStore', () => ({
   useWorkflowStore: vi.fn(() => mockWorkflowStore)
@@ -240,7 +248,10 @@ describe('GraphMutationService', () => {
         if (result.success) {
           expect(result.data).toBe('node-1')
         }
-        expect(mockLiteGraph.createNode).toHaveBeenCalledWith('LoadImage')
+        expect(mockLiteGraph.createNode).toHaveBeenCalledWith(
+          'LoadImage',
+          'My Image Loader'
+        )
         expect(mockGraph.beforeChange).toHaveBeenCalled()
         expect(mockGraph.add).toHaveBeenCalledWith(mockNode)
         expect(mockGraph.afterChange).toHaveBeenCalled()