graph mutation service implementation

[jtydhr88]

Summary

GraphMutationService is the centralized service layer for all graph modification operations in ComfyUI Frontend. It provides a unified and validated API for graph mutations, serving as the single entry point for all graph modification operations.

Changes

• What:
  add IGraphMutationService.ts and implement graphMutationService.ts

in order to test it, I use changeNodeTitile as example

#4691

┆Issue is synchronized with this Notion page by Unito

[github-actions]

⚠️ Warnings

⚠️ Warning: E2E Test Coverage Missing

If this PR modifies behavior that can be covered by browser-based E2E tests, those tests are required. PRs lacking applicable test coverage may not be reviewed until added. Please add or update browser tests to ensure code quality and prevent regressions.

⚠️ Warning: Visual Documentation Missing

If this PR changes user-facing behavior, visual proof (screen recording or screenshot) is required. PRs without applicable visual documentation may not be reviewed until provided.
You can add it by:

• GitHub: Drag & drop media directly into the PR description

• YouTube: Include a link to a short demo

[snomiao]

test broken...

4. [chromium] › browser_tests/tests/widget.spec.ts:245:3 › Animated image widget › Can drag-and-drop animated webp image

Error: expect(received).toContain(expected) // indexOf

Expected substring: "animated_webp.webp"
Received string:    "animated_web.webp"

  261 |     const fileComboWidget = await loadAnimatedWebpNode.getWidget(0)
  262 |     const filename = await fileComboWidget.getValue()
> 263 |     expect(filename).toContain('animated_webp.webp')
      |                      ^
  264 |   })
  265 |
  266 |   test('Can preview saved animated webp image', async ({ comfyPage }) => {
    at 

[jtydhr88]

@snomiao it doesn’t likely caused by my changes, for the frontend, I only modified change node title

[snomiao]

let me click update branch to see if it's flaky test

[Myestery]

test broken...

4. [chromium] › browser_tests/tests/widget.spec.ts:245:3 › Animated image widget › Can drag-and-drop animated webp image

Error: expect(received).toContain(expected) // indexOf

Expected substring: "animated_webp.webp"
Received string:    "animated_web.webp"

  261 |     const fileComboWidget = await loadAnimatedWebpNode.getWidget(0)
  262 |     const filename = await fileComboWidget.getValue()
> 263 |     expect(filename).toContain('animated_webp.webp')
      |                      ^
  264 |   })
  265 |
  266 |   test('Can preview saved animated webp image', async ({ comfyPage }) => {
    at 

For headless tests, this widget.spec.ts has been flaky on my end but works on ui mode
I was able to fix it by adding a little timeout to line 259 of browser_tests/tests/widget.spec.ts

 await comfyPage.page.waitForTimeout(200)

[snomiao]

I think we have to add auto retry for flaky tests.

[christian-byrne]

This looks great! The centralized graph mutation service is exactly what we need.

To align with our new domain-driven refactoring (see #5071), could we move these files to:

src/core/graph/operations/
├── graphMutationService.ts
└── IGraphMutationService.ts

This places graph operations in the core/ domain where they belong (pure business logic), rather than in the services/ technical layer we're moving away from.

The implementation itself looks perfect - just needs the folder adjustment to match our new architecture!

[jtydhr88]

This looks great! The centralized graph mutation service is exactly what we need.

To align with our new domain-driven refactoring (see #5071), could we move these files to:

src/core/graph/operations/
├── graphMutationService.ts
└── IGraphMutationService.ts

This places graph operations in the core/ domain where they belong (pure business logic), rather than in the services/ technical layer we're moving away from.

The implementation itself looks perfect - just needs the folder adjustment to match our new architecture!

Done

[github-actions]

🎭 Playwright Test Results

✅ All tests passed!

⏰ Completed at: 09/19/2025, 07:41:23 PM UTC

📈 Summary

• Total Tests: 450
• Passed: 421 ✅
• Failed: 0
• Flaky: 0
• Skipped: 29 ⏭️

📊 Test Reports by Browser

• ✅ chromium: View Report • ✅ 414 / ❌ 0 / ⚠️ 0 / ⏭️ 29
• ✅ chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
• ✅ chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
• ✅ mobile-chrome: View Report • ✅ 4 / ❌ 0 / ⚠️ 0 / ⏭️ 0

---

🎉 Click on the links above to view detailed test results for each browser configuration.

[github-actions]

🎨 Storybook Build Status

✅ Build completed successfully!

⏰ Completed at: 08/21/2025, 03:24:29 AM UTC

📊 Build Summary

• Components: 13
• Stories: 52
• Visual changes: 0
• Errors: 0

🔗 Links

• 📸 View Chromatic Build
• 📖 Preview Storybook

---

🎉 Your Storybook is ready for review!

[christian-byrne]

This looks great. I left some possible suggestions. Also, as discussed offline, we will be integrating the command pattern to make it compatible with CRDT system like below pseudo code

interface GraphMutationCommand {
  type: string
  params: sometype
  timestamp: number
  origin: string
}

private createCommand(operation: string, params: sometype): GraphMutationCommand {
  return {
    type: operation,
    params,
    timestamp: Date.now(),
    origin: 'local'  // Will become peer ID in collaborative mode
  }
}

I also realized we might want to track direct access to nodes that circumvent the mutation service and add deprecation warnings.

class GraphMutationService {
  private warnDirectAccess(context: string) {
    console.warn(
      `Direct LiteGraph access detected in ${context}. ` +
      `Consider using GraphMutationService for better compatibility. ` +
      `Direct access will be deprecated in a future version`
    )
  }
}

[christian-byrne]

Some other edge cases we could cover:

  it('should handle canvas null scenarios gracefully', () => {
    // Test service behavior when canvas is not initialized
  })

  it('should handle subgraph context switching', () => {
    // Test mutations during subgraph navigation
    // Based on widget configuration patterns documented
  })

  it('should preserve link IDs across subgraph operations', () => {
    // Based on "Link ID Changes in Subgraphs" pattern
  })

[christian-byrne]

Do we want to keep this markdown doc?

[jtydhr88]

we can remove it from source, and move it to notion, but it is easy to update here, lets keep it in source for now, I will remove it until we merge

[christian-byrne]

See https://github.com/Comfy-Org/ComfyUI_frontend/blob/graphMutationService/src/lib/litegraph/src/LGraph.ts#L2130

[DrJKL]

I think I'd like to put this on pause for now. I think there's a lot that will change in how we're managing the state of the underlying graph(s) and crystallizing the current patterns will become more of a maintenance burden. WDYT @jtydhr88 ?

[snomiao]

@jtydhr88 lmk if you want a claude-auto-rebase XD