Skip to content

Commit 6feb202

Browse files
Add support for search aliases on subgraphs (#8608)
## Summary - add commands for setting search aliases and description when in subgraph - in future we can add these fields to the dialog when publishing a subgraph - map workflow extra metadata on save/load from from/to subgraph node to allow access via `canvas.subgraph.extra` ## Changes **What**: - new core commands for Comfy.Subgraph.SetSearchAliases & Comfy.Subgraph.SetDescription to be called when in a subgraph context - update Publish command to allow command metadata arg for name - update test executeCommand to allow passing metadata arg ## Review Focus - When saving a subgraph, the outer workflow "wrapper" is created at the point of publishing. So unlike a normal workflow `extra` property that is available at any point, for a subgraph this is not accessible. To workaround this, the `extra` property that exists on the inner subgraph node is copied to the top level on save, and restored on load so extra properties can be set via `canvas.subgraph.extra`. - I have kept the existing naming format matching `BlueprintDescription` for `BlueprintSearchAliases` but i'm not sure if the description was ever used before ## Screenshots https://github.com/user-attachments/assets/4d4df9c1-2281-4589-aa56-ab07cdecd353 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8608-Add-support-for-search-aliases-on-subgraphs-2fd6d73d365081d083caebd6befcacdd) by [Unito](https://www.unito.io) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Set subgraph search aliases (comma-separated) and descriptions; aliases enable discovery by alternative names. * Publish subgraphs using a provided name. * Node definitions now support search aliases so nodes can be found by alternate names. * UI strings added for entering descriptions and search aliases. * **Tests** * Added end-to-end and unit tests covering aliases, descriptions, search behavior, publishing, and persistence. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent b8287f6 commit 6feb202

File tree

12 files changed

+668
-25
lines changed

12 files changed

+668
-25
lines changed

browser_tests/fixtures/components/ComfyNodeSearchBox.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,11 @@ export class ComfyNodeSearchBox {
8080
async removeFilter(index: number) {
8181
await this.filterChips.nth(index).locator('.p-chip-remove-icon').click()
8282
}
83+
84+
/**
85+
* Returns a locator for a search result containing the specified text.
86+
*/
87+
findResult(text: string): Locator {
88+
return this.dropdown.locator('li').filter({ hasText: text })
89+
}
8390
}

browser_tests/fixtures/helpers/CommandHelper.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,18 @@ import type { KeyCombo } from '../../../src/platform/keybindings/types'
55
export class CommandHelper {
66
constructor(private readonly page: Page) {}
77

8-
async executeCommand(commandId: string): Promise<void> {
9-
await this.page.evaluate((id: string) => {
10-
return window.app!.extensionManager.command.execute(id)
11-
}, commandId)
8+
async executeCommand(
9+
commandId: string,
10+
metadata?: Record<string, unknown>
11+
): Promise<void> {
12+
await this.page.evaluate(
13+
({ commandId, metadata }) => {
14+
return window['app'].extensionManager.command.execute(commandId, {
15+
metadata
16+
})
17+
},
18+
{ commandId, metadata }
19+
)
1220
}
1321

1422
async registerCommand(
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import { expect } from '@playwright/test'
2+
3+
import type { ComfyPage } from '../fixtures/ComfyPage'
4+
import { comfyPageFixture as test } from '../fixtures/ComfyPage'
5+
6+
async function createSubgraphAndNavigateInto(comfyPage: ComfyPage) {
7+
await comfyPage.workflow.loadWorkflow('default')
8+
await comfyPage.nextFrame()
9+
10+
const ksampler = await comfyPage.nodeOps.getNodeRefById('3')
11+
await ksampler.click('title')
12+
await ksampler.convertToSubgraph()
13+
await comfyPage.nextFrame()
14+
15+
const subgraphNodes =
16+
await comfyPage.nodeOps.getNodeRefsByTitle('New Subgraph')
17+
expect(subgraphNodes.length).toBe(1)
18+
const subgraphNode = subgraphNodes[0]
19+
20+
await subgraphNode.navigateIntoSubgraph()
21+
return subgraphNode
22+
}
23+
24+
async function exitSubgraphAndPublish(
25+
comfyPage: ComfyPage,
26+
subgraphNode: Awaited<ReturnType<typeof createSubgraphAndNavigateInto>>,
27+
blueprintName: string
28+
) {
29+
await comfyPage.page.keyboard.press('Escape')
30+
await comfyPage.nextFrame()
31+
32+
await subgraphNode.click('title')
33+
await comfyPage.command.executeCommand('Comfy.PublishSubgraph', {
34+
name: blueprintName
35+
})
36+
37+
await expect(comfyPage.visibleToasts).toHaveCount(1, { timeout: 5000 })
38+
await comfyPage.toast.closeToasts(1)
39+
}
40+
41+
async function searchAndExpectResult(
42+
comfyPage: ComfyPage,
43+
searchTerm: string,
44+
expectedResult: string
45+
) {
46+
await comfyPage.command.executeCommand('Workspace.SearchBox.Toggle')
47+
await expect(comfyPage.searchBox.input).toHaveCount(1)
48+
await comfyPage.searchBox.input.fill(searchTerm)
49+
await expect(comfyPage.searchBox.findResult(expectedResult)).toBeVisible({
50+
timeout: 10000
51+
})
52+
}
53+
54+
test.describe('Subgraph Search Aliases', { tag: ['@subgraph'] }, () => {
55+
test.beforeEach(async ({ comfyPage }) => {
56+
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top')
57+
await comfyPage.settings.setSetting('Comfy.NodeSearchBoxImpl', 'default')
58+
})
59+
60+
test('Can set search aliases on subgraph and find via search', async ({
61+
comfyPage
62+
}) => {
63+
const subgraphNode = await createSubgraphAndNavigateInto(comfyPage)
64+
65+
await comfyPage.command.executeCommand('Comfy.Subgraph.SetSearchAliases', {
66+
aliases: 'qwerty,unicorn'
67+
})
68+
69+
const blueprintName = `test-aliases-${Date.now()}`
70+
await exitSubgraphAndPublish(comfyPage, subgraphNode, blueprintName)
71+
await searchAndExpectResult(comfyPage, 'unicorn', blueprintName)
72+
})
73+
74+
test('Can set description on subgraph', async ({ comfyPage }) => {
75+
await createSubgraphAndNavigateInto(comfyPage)
76+
77+
await comfyPage.command.executeCommand('Comfy.Subgraph.SetDescription', {
78+
description: 'This is a test description'
79+
})
80+
// Verify the description was set on the subgraph's extra
81+
const description = await comfyPage.page.evaluate(() => {
82+
const subgraph = window['app']!.canvas.subgraph
83+
return (subgraph?.extra as Record<string, unknown>)?.BlueprintDescription
84+
})
85+
expect(description).toBe('This is a test description')
86+
})
87+
88+
test('Search aliases persist after publish and reload', async ({
89+
comfyPage
90+
}) => {
91+
const subgraphNode = await createSubgraphAndNavigateInto(comfyPage)
92+
93+
await comfyPage.command.executeCommand('Comfy.Subgraph.SetSearchAliases', {
94+
aliases: 'dragon, fire breather'
95+
})
96+
97+
const blueprintName = `test-persist-${Date.now()}`
98+
await exitSubgraphAndPublish(comfyPage, subgraphNode, blueprintName)
99+
100+
// Reload the page to ensure aliases are persisted
101+
await comfyPage.page.reload()
102+
await comfyPage.page.waitForFunction(
103+
() => window['app'] && window['app'].extensionManager
104+
)
105+
await comfyPage.nextFrame()
106+
107+
await searchAndExpectResult(comfyPage, 'dragon', blueprintName)
108+
})
109+
})

src/composables/useCoreCommands.test.ts

Lines changed: 159 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,11 @@ vi.mock('@/platform/workflow/core/services/workflowService', () => ({
6868
useWorkflowService: vi.fn(() => ({}))
6969
}))
7070

71+
const mockDialogService = vi.hoisted(() => ({
72+
prompt: vi.fn()
73+
}))
7174
vi.mock('@/services/dialogService', () => ({
72-
useDialogService: vi.fn(() => ({}))
75+
useDialogService: vi.fn(() => mockDialogService)
7376
}))
7477

7578
vi.mock('@/services/litegraphService', () => ({
@@ -84,14 +87,31 @@ vi.mock('@/stores/toastStore', () => ({
8487
useToastStore: vi.fn(() => ({}))
8588
}))
8689

90+
const mockChangeTracker = vi.hoisted(() => ({
91+
checkState: vi.fn()
92+
}))
93+
const mockWorkflowStore = vi.hoisted(() => ({
94+
activeWorkflow: {
95+
changeTracker: mockChangeTracker
96+
}
97+
}))
8798
vi.mock('@/platform/workflow/management/stores/workflowStore', () => ({
88-
useWorkflowStore: vi.fn(() => ({}))
99+
useWorkflowStore: vi.fn(() => mockWorkflowStore)
89100
}))
90101

91102
vi.mock('@/stores/subgraphStore', () => ({
92103
useSubgraphStore: vi.fn(() => ({}))
93104
}))
94105

106+
vi.mock('@/renderer/core/canvas/canvasStore', () => ({
107+
useCanvasStore: vi.fn(() => ({
108+
getCanvas: () => app.canvas
109+
})),
110+
useTitleEditorStore: vi.fn(() => ({
111+
titleEditorTarget: null
112+
}))
113+
}))
114+
95115
vi.mock('@/stores/workspace/colorPaletteStore', () => ({
96116
useColorPaletteStore: vi.fn(() => ({}))
97117
}))
@@ -155,11 +175,12 @@ describe('useCoreCommands', () => {
155175
findNodeById: vi.fn(),
156176
getNodeById: vi.fn(),
157177
setDirtyCanvas: vi.fn(),
158-
sendActionToCanvas: vi.fn()
178+
sendActionToCanvas: vi.fn(),
179+
extra: {} as Record<string, unknown>
159180
} as Partial<typeof app.canvas.subgraph> as typeof app.canvas.subgraph
160181
}
161182

162-
const mockSubgraph = createMockSubgraph()
183+
const mockSubgraph = createMockSubgraph()!
163184

164185
function createMockSettingStore(
165186
getReturnValue: boolean
@@ -270,4 +291,138 @@ describe('useCoreCommands', () => {
270291
expect(api.dispatchCustomEvent).not.toHaveBeenCalled()
271292
})
272293
})
294+
295+
describe('Subgraph metadata commands', () => {
296+
beforeEach(() => {
297+
mockSubgraph.extra = {}
298+
vi.clearAllMocks()
299+
})
300+
301+
describe('SetDescription command', () => {
302+
it('should do nothing when not in subgraph', async () => {
303+
app.canvas.subgraph = undefined
304+
305+
const commands = useCoreCommands()
306+
const setDescCommand = commands.find(
307+
(cmd) => cmd.id === 'Comfy.Subgraph.SetDescription'
308+
)!
309+
310+
await setDescCommand.function()
311+
312+
expect(mockDialogService.prompt).not.toHaveBeenCalled()
313+
})
314+
315+
it('should set description on subgraph.extra', async () => {
316+
app.canvas.subgraph = mockSubgraph
317+
mockDialogService.prompt.mockResolvedValue('Test description')
318+
319+
const commands = useCoreCommands()
320+
const setDescCommand = commands.find(
321+
(cmd) => cmd.id === 'Comfy.Subgraph.SetDescription'
322+
)!
323+
324+
await setDescCommand.function()
325+
326+
expect(mockDialogService.prompt).toHaveBeenCalled()
327+
expect(mockSubgraph.extra.BlueprintDescription).toBe('Test description')
328+
expect(mockChangeTracker.checkState).toHaveBeenCalled()
329+
})
330+
331+
it('should not set description when user cancels', async () => {
332+
app.canvas.subgraph = mockSubgraph
333+
mockDialogService.prompt.mockResolvedValue(null)
334+
335+
const commands = useCoreCommands()
336+
const setDescCommand = commands.find(
337+
(cmd) => cmd.id === 'Comfy.Subgraph.SetDescription'
338+
)!
339+
340+
await setDescCommand.function()
341+
342+
expect(mockSubgraph.extra.BlueprintDescription).toBeUndefined()
343+
expect(mockChangeTracker.checkState).not.toHaveBeenCalled()
344+
})
345+
})
346+
347+
describe('SetSearchAliases command', () => {
348+
it('should do nothing when not in subgraph', async () => {
349+
app.canvas.subgraph = undefined
350+
351+
const commands = useCoreCommands()
352+
const setAliasesCommand = commands.find(
353+
(cmd) => cmd.id === 'Comfy.Subgraph.SetSearchAliases'
354+
)!
355+
356+
await setAliasesCommand.function()
357+
358+
expect(mockDialogService.prompt).not.toHaveBeenCalled()
359+
})
360+
361+
it('should set search aliases on subgraph.extra', async () => {
362+
app.canvas.subgraph = mockSubgraph
363+
mockDialogService.prompt.mockResolvedValue('alias1, alias2, alias3')
364+
365+
const commands = useCoreCommands()
366+
const setAliasesCommand = commands.find(
367+
(cmd) => cmd.id === 'Comfy.Subgraph.SetSearchAliases'
368+
)!
369+
370+
await setAliasesCommand.function()
371+
372+
expect(mockDialogService.prompt).toHaveBeenCalled()
373+
expect(mockSubgraph.extra.BlueprintSearchAliases).toEqual([
374+
'alias1',
375+
'alias2',
376+
'alias3'
377+
])
378+
expect(mockChangeTracker.checkState).toHaveBeenCalled()
379+
})
380+
381+
it('should trim whitespace and filter empty strings', async () => {
382+
app.canvas.subgraph = mockSubgraph
383+
mockDialogService.prompt.mockResolvedValue(' alias1 , , alias2 , ')
384+
385+
const commands = useCoreCommands()
386+
const setAliasesCommand = commands.find(
387+
(cmd) => cmd.id === 'Comfy.Subgraph.SetSearchAliases'
388+
)!
389+
390+
await setAliasesCommand.function()
391+
392+
expect(mockSubgraph.extra.BlueprintSearchAliases).toEqual([
393+
'alias1',
394+
'alias2'
395+
])
396+
})
397+
398+
it('should set undefined when empty input', async () => {
399+
app.canvas.subgraph = mockSubgraph
400+
mockDialogService.prompt.mockResolvedValue('')
401+
402+
const commands = useCoreCommands()
403+
const setAliasesCommand = commands.find(
404+
(cmd) => cmd.id === 'Comfy.Subgraph.SetSearchAliases'
405+
)!
406+
407+
await setAliasesCommand.function()
408+
409+
expect(mockSubgraph.extra.BlueprintSearchAliases).toBeUndefined()
410+
})
411+
412+
it('should not set aliases when user cancels', async () => {
413+
app.canvas.subgraph = mockSubgraph
414+
mockDialogService.prompt.mockResolvedValue(null)
415+
416+
const commands = useCoreCommands()
417+
const setAliasesCommand = commands.find(
418+
(cmd) => cmd.id === 'Comfy.Subgraph.SetSearchAliases'
419+
)!
420+
421+
await setAliasesCommand.function()
422+
423+
expect(mockSubgraph.extra.BlueprintSearchAliases).toBeUndefined()
424+
expect(mockChangeTracker.checkState).not.toHaveBeenCalled()
425+
})
426+
})
427+
})
273428
})

0 commit comments

Comments
 (0)