V2 Node Search (+ hidden Node Library changes)#8987
Merged
christian-byrne merged 48 commits intomainfrom Feb 20, 2026
Merged
Conversation
pythongosssss
requested review from
a team,
bigcat88 and
jojodecayz
as code owners
dosubot
bot
added
the
size:L
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a V2 Node Search UI and Node Library redesign (components, tests), node preview, pricing utilities, drag-to-canvas composables, litegraph API/type extensions, schema/type updates for essentials, many new Vue components/tests, Playwright fixtures for SearchBoxV2, and multiple tests switched to legacy search implementations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as NodeSearchBoxV2
participant Store as NodeDefStore
participant Content as NodeSearchContent
participant Preview as NodePreviewCard
participant Canvas as LiteGraph/Canvas
User->>UI: type query / navigate
UI->>Store: debounce query
Store-->>UI: results
UI->>Content: render filtered results
User->>Content: hover item
Content->>Preview: set hovered node
Preview-->>User: show preview
User->>Content: press Enter / click
Content->>Canvas: emit addNode(nodeDef, dragEvent?)
Canvas-->>Content: add node on graph (ghost/placement)
sequenceDiagram
participant NodeCard
participant Drag as useNodeDragToCanvas
participant Global as BrowserEvents
participant Canvas as LiteGraph/Canvas
User->>NodeCard: mousedown (start drag)
NodeCard->>Drag: startDrag(nodeDef, mode:'native')
Drag->>Global: install pointermove/pointerup listeners
Global->>Drag: pointermove -> update cursorPosition
User->>Canvas: pointerup/drop over canvas
Global->>Drag: pointerup triggers endDrag
Drag->>Canvas: addNodeOnGraph(nodeDef, addOptions)
Canvas-->>Drag: finalize placement and selection change
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/20/2026, 08:46:39 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
|
Playwright: ✅ 527 passed, 0 failed · 3 flaky 📊 Browser Reports
|
📦 Bundle: 4.32 MB gzip 🔴 +50.8 kBDetailsSummary
Category Glance App Entry Points — 21.4 kB (baseline 21.4 kB) • 🔴 +1 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 936 kB (baseline 914 kB) • 🔴 +22.1 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.6 kB (baseline 68.6 kB) • 🟢 -1 BTop-level views, pages, and routed surfaces
Status: 11 added / 11 removed Panels & Settings — 430 kB (baseline 430 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed Editors & Dialogs — 706 B (baseline 706 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 42.3 kB (baseline 42.3 kB) • ⚪ 0 BReusable component library chunks
Status: 11 added / 11 removed Data & Services — 2.47 MB (baseline 2.4 MB) • 🔴 +68.8 kBStores, services, APIs, and repositories
Status: 14 added / 14 removed Utilities & Hooks — 57.6 kB (baseline 57.6 kB) • 🔴 +1 BHelpers, composables, and utility bundles
Status: 15 added / 15 removed Vendor & Third-Party — 8.86 MB (baseline 8.7 MB) • 🔴 +161 kBExternal libraries and shared vendor chunks
Status: 7 added / 7 removed Other — 7.38 MB (baseline 7.38 MB) • 🔴 +443 BBundles that do not match a named category
Status: 74 added / 74 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lib/litegraph/src/LGraphCanvas.ts (1)
3612-3626:⚠️ Potential issue | 🟡 MinorFire selection-change callback on ghost placement cancellation too.
The callback is only invoked on successful placement (Line 3616-3625). When cancellation removes the node, selection changes but no
onSelectionChangefires, so downstream selection consumers may stay stale. Consider moving the callback outside the conditional or mirroring it in the cancel path.Proposed fix
if (cancelled) { this.deselect(node) this.graph?.remove(node) } else { delete node.flags.ghost this.graph?.trigger('node:property:changed', { nodeId: node.id, property: 'flags.ghost', oldValue: true, newValue: false }) - this.state.selectionChanged = true - this.onSelectionChange?.(this.selected_nodes) } + + this.state.selectionChanged = true + this.onSelectionChange?.(this.selected_nodes)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/litegraph/src/LGraphCanvas.ts` around lines 3612 - 3626, The selection-change callback isn't called when placement is cancelled: in the cancel branch where deselect(node) and this.graph?.remove(node) are executed you must also mark selection changed and invoke the callback so consumers are updated; update the logic around the conditional handling of node.flags.ghost (in the same block that currently sets this.state.selectionChanged = true and calls this.onSelectionChange?.(this.selected_nodes)) so that either the onSelectionChange call is moved after the if/else or duplicated into the cancelled path, ensuring deselect(node) / this.graph?.remove(node) are followed by this.state.selectionChanged = true and this.onSelectionChange?.(this.selected_nodes).browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts (1)
992-1047:⚠️ Potential issue | 🟠 MajorBoth tests will likely break once the default
Comfy.NodeSearchBoxImplchanges to'v2'.
comfyPage.searchBoxis the v1 fixture. These two tests don't pinComfy.NodeSearchBoxImplto'v1 (legacy)', unlike the sibling test at line 923. When the default flips to'v2', shift-dropping will open the v2 UI and the subgraph workflow toggles the v2 search box, both of whichcomfyPage.searchBox.*selectors won't match.🐛 Proposed fix — add the missing setting to both tests
test('Search box opens on Shift-drop and connects after selection', async ({ comfyPage, comfyMouse }) => { await comfyPage.settings.setSetting( 'Comfy.LinkRelease.ActionShift', 'search box' ) + await comfyPage.settings.setSetting( + 'Comfy.NodeSearchBoxImpl', + 'v1 (legacy)' + )test('Dragging from subgraph input connects to correct slot', async ({ comfyPage, comfyMouse }) => { + await comfyPage.settings.setSetting( + 'Comfy.NodeSearchBoxImpl', + 'v1 (legacy)' + ) // Setup workflow with a KSampler nodeAlso applies to: 1050-1059
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts` around lines 992 - 1047, The test "Search box opens on Shift-drop and connects after selection" uses the v1 searchBox fixture (comfyPage.searchBox) but doesn't pin the implementation, so when Comfy.NodeSearchBoxImpl defaults to 'v2' the selectors will break; fix by setting the node search box implementation at the start of this test (and the sibling test around lines 1050-1059) via comfyPage.settings.setSetting('Comfy.NodeSearchBoxImpl', 'v1 (legacy)') so the test always opens the v1 UI that comfyPage.searchBox.* expects.src/platform/settings/constants/coreSettings.ts (1)
33-40:⚠️ Potential issue | 🟠 MajorMissing
migrateDeprecatedValuefor the removed'simple'option.Users who explicitly chose the old
'simple'(v1) search box have'simple'persisted in their settings. Removing it without a migration will silently revert them to'default'(the new v2 implementation), which is the opposite of their stated preference. The Zod schema inapiSchema.tswill also reject the stored'simple'value.The same migration pattern already exists for
Comfy.UseNewMenu(lines 583-592) andComfy.Workflow.WorkflowTabsPosition(lines 600-606).🔧 Proposed fix
{ id: 'Comfy.NodeSearchBoxImpl', category: ['Comfy', 'Node Search Box', 'Implementation'], experimental: true, name: 'Node search box implementation', type: 'combo', options: ['default', 'v1 (legacy)', 'litegraph (legacy)'], - defaultValue: 'default' + defaultValue: 'default', + migrateDeprecatedValue: (val: unknown) => { + if (val === 'simple') return 'v1 (legacy)' + return val + } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/settings/constants/coreSettings.ts` around lines 33 - 40, The setting with id 'Comfy.NodeSearchBoxImpl' is missing a migrateDeprecatedValue mapping for the removed 'simple' option; add a migrateDeprecatedValue handler on that setting to translate the old stored value 'simple' to the retained option name 'v1 (legacy)' so users who previously chose the v1/simple search box keep their preference and the Zod schema in apiSchema.ts accepts it; mirror the same migration pattern used by Comfy.UseNewMenu and Comfy.Workflow.WorkflowTabsPosition when implementing this.
🧹 Nitpick comments (30)
packages/design-system/src/css/style.css (1)
15-19:load-3-dnaming is correct; consider automating icon safelist updates.The icon file
load-3-d.svgexists and matches the safelist entry exactly, so no renaming is needed. However, the hardcoded lists will require manual updates whenever new essential node icons or category folder icons are added. Consider generating these lists from the icon directory (e.g., a small build script that reads./packages/design-system/src/icons/and emits the@source inlinedirectives) to reduce maintenance burden and prevent drift when new icons are integrated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/design-system/src/css/style.css` around lines 15 - 19, The safelist `@source` inline directives (the two "icon-[comfy--{...}]" lines including the "load-3-d" entry) are being maintained manually which will drift as icons are added; replace the hardcoded lists with a generated output: add a small build script that scans packages/design-system/src/icons/ for filenames, converts them to the kebab-case/comfy-prefixed tokens used in the `@source` inline patterns (e.g., "load-3-d" and other icon names), and writes or injects the updated `@source` inline directives into packages/design-system/src/css/style.css during the build step so the icon safelist is kept in sync automatically.src/stores/workspace/searchBoxStore.test.ts (1)
43-64:useSearchBoxV2is now public but has no test coverage.
useSearchBoxV2is returned from the store and consumed by components to route to the v2 search box. The existing'when user has new search box enabled'suite only assertsnewSearchBoxEnabled; a case foruseSearchBoxV2beingtruewhen the setting is'default'andfalsefor'v1 (legacy)'would close that gap.✅ Suggested additional test
describe('when user has new search box enabled', () => { beforeEach(() => { vi.mocked(mockSettingStore.get).mockReturnValue('default') }) it('should show new search box is enabled', () => { const store = useSearchBoxStore() expect(store.newSearchBoxEnabled).toBe(true) }) + it('should use the v2 search box', () => { + const store = useSearchBoxStore() + expect(store.useSearchBoxV2).toBe(true) + }) }) + describe('when user has v1 legacy Vue search box enabled', () => { + beforeEach(() => { + vi.mocked(mockSettingStore.get).mockReturnValue('v1 (legacy)') + }) + + it('should not use the v2 search box but keep new search box enabled', () => { + const store = useSearchBoxStore() + expect(store.useSearchBoxV2).toBe(false) + expect(store.newSearchBoxEnabled).toBe(true) + }) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/workspace/searchBoxStore.test.ts` around lines 43 - 64, Add tests in the existing "when user has new search box enabled" suite to cover the exported useSearchBoxV2 value from useSearchBoxStore: mock mockSettingStore.get to return 'default' and assert store.useSearchBoxV2 (or useSearchBoxV2) is true, then add a separate case where mockSettingStore.get returns 'v1 (legacy)' and assert store.useSearchBoxV2 is false; ensure you call useSearchBoxStore() inside each test so the store reads the mocked setting and keep the existing visibility/toggle tests unchanged.src/components/sidebar/tabs/nodeLibrary/EssentialNodeCard.test.ts (1)
154-183: Drag/drop and hover tests do not assert any behavior — they will pass even if the handlers are deleted.Both describe blocks only trigger events and make no assertions. These don't protect against regressions per the project testing guideline: "Do not write tests that just test the mocks; ensure tests fail when code behaves unexpectedly."
The mock for
useNodeDragToCanvasalready provides trackedstartDrag/handleNativeDropfns — add spy assertions to make these tests meaningful.✅ Suggested minimal assertions
describe('drag and drop', () => { it('should handle dragstart event', async () => { const wrapper = mountComponent() const card = wrapper.find('div') + const { startDrag } = vi.mocked( + (await import('@/composables/node/useNodeDragToCanvas')).useNodeDragToCanvas() + ) await card.trigger('dragstart') + expect(startDrag).toHaveBeenCalled() }) it('should handle dragend event', async () => { const wrapper = mountComponent() const card = wrapper.find('div') await card.trigger('dragend') + // Verify no errors and draggable state is cleared + expect(wrapper.find('[draggable]').attributes('draggable')).toBe('true') }) }) describe('hover preview', () => { it('should handle mouseenter event', async () => { const wrapper = mountComponent() const card = wrapper.find('div') await card.trigger('mouseenter') + // NodePreviewCard should be conditionally rendered after mouseenter + expect(wrapper.findComponent({ name: 'NodePreviewCard' }).exists()).toBe(true) }) it('should handle mouseleave event', async () => { const wrapper = mountComponent() const card = wrapper.find('div') await card.trigger('mouseenter') await card.trigger('mouseleave') + expect(wrapper.findComponent({ name: 'NodePreviewCard' }).exists()).toBe(false) }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/nodeLibrary/EssentialNodeCard.test.ts` around lines 154 - 183, The tests currently only trigger events without asserting behavior; add assertions that verify the mocked drag/hover handlers are invoked: spy on the functions provided by the useNodeDragToCanvas mock (startDrag and handleNativeDrop) and assert startDrag is called when triggering 'dragstart' and handleNativeDrop is called on 'dragend'; for the hover preview tests locate the component's hover handler (the function invoked on mouseenter/mouseleave in mountComponent or the preview setter used by the component) and add spies/assertions that those handlers are called when you trigger 'mouseenter' and 'mouseleave' so the tests fail if the handlers are removed.src/services/nodeOrganizationService.ts (3)
119-127:categoryPathExtractorduplicates the'category'grouping strategy defined above.The lambda at Lines 123–127 is identical to the
getNodePathin the'category'grouping strategy (Lines 28–32). Consider reusing the strategy'sgetNodePathdirectly.♻️ Possible approach
organizeNodesByTab( nodes: ComfyNodeDefImpl[], tabId: TabId = DEFAULT_TAB_ID ): NodeSection[] { - const categoryPathExtractor = (nodeDef: ComfyNodeDefImpl) => { - const category = nodeDef.category || '' - const categoryParts = category ? category.split('/') : [] - return [...categoryParts, nodeDef.name] - } + const categoryPathExtractor = + this.getGroupingStrategy('category')!.getNodePath🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/nodeOrganizationService.ts` around lines 119 - 127, The local lambda categoryPathExtractor inside organizeNodesByTab duplicates the existing 'category' grouping strategy's getNodePath; replace the lambda by invoking or referencing that strategy's getNodePath (e.g., use the groupingStrategies['category'].getNodePath or the symbol getNodePath used by the 'category' strategy) so organizeNodesByTab reuses the existing logic rather than duplicating it, and remove the redundant categoryPathExtractor declaration.
141-163: HardcodedfolderOrdermirrors the mock categories innodeSource.ts.If the essentials categories evolve (e.g., new categories added backend-side), this ordering list will need a manual update. Worth a brief comment noting the coupling, or consider co-locating the canonical category list.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/nodeOrganizationService.ts` around lines 141 - 163, The hardcoded folderOrder array in nodeOrganizationService.ts duplicates the canonical categories from nodeSource.ts and creates a fragile coupling; replace the inline folderOrder with a single shared constant (e.g., export a CATEGORY_ORDER or DEFAULT_FOLDER_ORDER from nodeSource.ts and import it into nodeOrganizationService) and use that imported list in the existing sort logic (keeping originalIndex and the fallback behavior for unknown labels). If you prefer not to refactor now, at minimum add a brief comment above folderOrder noting it mirrors nodeSource.ts and must be updated when backend categories change.
166-208:'custom'and'all'branches share near-identical grouping logic.Both branches group nodes by
main_category, build aMap, and map entries throughbuildNodeDefTree. The only differences are the pre-filter (CustomNodes only vs. all) and the fallback category name. Consider extracting the shared logic.♻️ Sketch
+ private groupByMainCategory( + nodes: ComfyNodeDefImpl[], + fallbackCategory: string, + pathExtractor: (n: ComfyNodeDefImpl) => string[] + ): NodeSection[] { + const grouped = new Map<string, ComfyNodeDefImpl[]>() + for (const node of nodes) { + const cat = node.main_category ?? fallbackCategory + if (!grouped.has(cat)) grouped.set(cat, []) + grouped.get(cat)!.push(node) + } + return Array.from(grouped.entries()).map(([cat, catNodes]) => ({ + title: upperCase(cat), + tree: buildNodeDefTree(catNodes, { pathExtractor }) + })) + }Then in
organizeNodesByTab:case 'custom': { const customNodes = nodes.filter( (nodeDef) => nodeDef.nodeSource.type === NodeSourceType.CustomNodes ) - // ...duplicated grouping code + return this.groupByMainCategory(customNodes, 'custom_extensions', categoryPathExtractor) } case 'all': default: { - // ...duplicated grouping code + return this.groupByMainCategory(nodes, 'basics', categoryPathExtractor) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/nodeOrganizationService.ts` around lines 166 - 208, Refactor the duplicated grouping/mapping logic inside the 'custom' and 'all' cases by extracting a helper (e.g., groupAndBuildTrees) that accepts an array of nodes and a fallback mainCategory string; inside organizeNodesByTab call this helper for the filtered customNodes with fallback 'custom_extensions' and for all nodes with fallback 'basics'. The helper should create the Map keyed by node.main_category ?? fallback, push nodes into buckets, then return Array.from(map.entries()).map(([mainCategory, categoryNodes]) => ({ title: upperCase(mainCategory), tree: buildNodeDefTree(categoryNodes, { pathExtractor: categoryPathExtractor }) })). Use NodeSourceType.CustomNodes only for filtering in the caller and keep buildNodeDefTree and categoryPathExtractor usage unchanged.src/utils/categoryUtil.ts (1)
102-125: Consider extracting the sharediconKeyderivation.Both
getProviderIconandgetProviderBorderStyleindependently compute the sameiconKeyfromproviderName. A small helper would reduce duplication and keep the normalization logic in one place.♻️ Proposed refactor
+function toProviderKey(providerName: string): string { + return providerName.toLowerCase().replaceAll(/\s+/g, '-') +} + export function getProviderIcon(providerName: string): string { - const iconKey = providerName.toLowerCase().replaceAll(/\s+/g, '-') - return `icon-[comfy--${iconKey}]` + return `icon-[comfy--${toProviderKey(providerName)}]` } export function getProviderBorderStyle(providerName: string): string { - const iconKey = providerName.toLowerCase().replaceAll(/\s+/g, '-') - const colors = PROVIDER_COLORS[iconKey] + const colors = PROVIDER_COLORS[toProviderKey(providerName)]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/categoryUtil.ts` around lines 102 - 125, Extract the shared provider-name normalization into a small helper (e.g., normalizeProviderName or getIconKey) and use it from both getProviderIcon and getProviderBorderStyle so the logic isn't duplicated; move the toLowerCase() and whitespace-to-dash logic into that helper and replace both current iconKey derivations with a call to the new function, leaving the rest of each function unchanged (including the PROVIDER_COLORS lookup and return behavior).src/components/sidebar/tabs/NodeLibrarySidebarTabV2.test.ts (1)
90-90: Prefer the component reference over a string indescribe().Per the
vitest/prefer-describe-function-titlerule:describe(NodeLibrarySidebarTabV2, () => {...}).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.test.ts` at line 90, Replace the string title in the test suite with the actual component reference: update the describe call to use NodeLibrarySidebarTabV2 (the component identifier) instead of the string 'NodeLibrarySidebarTabV2' so it follows the vitest/prefer-describe-function-title rule; locate the describe invocation in NodeLibrarySidebarTabV2.test.ts and pass the NodeLibrarySidebarTabV2 symbol directly as the first argument.src/components/common/SearchBoxV2.test.ts (2)
25-48: Type thepropsparameter ofmountComponent.
props = {}is implicitlyany, which defeats type safety and won't catch invalid prop names or types.♻️ Proposed fix
+import type { ComponentProps } from 'vue-component-type-helpers' + - function mountComponent(props = {}) { + function mountComponent(props: Partial<ComponentProps<typeof SearchBoxV2>> = {}) {Based on learnings: "for test helpers like
mountComponent, type the props parameter asPartial<ComponentProps<typeof Component>>."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/SearchBoxV2.test.ts` around lines 25 - 48, The mountComponent helper currently accepts an untyped props parameter which is implicitly any; update its signature to accept props: Partial<ComponentProps<typeof SearchBoxV2>> to get proper prop typing and autocomplete for SearchBoxV2, and add the necessary import for ComponentProps from 'vue' at the top of the test file; keep the function body unchanged and ensure existing calls to mountComponent still work with the new typed parameter.
81-89: Class-name presence checks are brittle.
wrapper.html().toContain('size-5')andwrapper.html().toContain('size-4')couple tests to internal Tailwind utility names. A straightforward refactor of the icon sizing (e.g., renaming a class or switching tow-5 h-5) breaks these tests without any behaviour change.Consider asserting on a more stable property — for example, querying the icon element and checking a
data-sizeattribute, an aria attribute, or an accessible name that reflects the intent of the size prop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/SearchBoxV2.test.ts` around lines 81 - 89, Replace brittle HTML string checks in SearchBoxV2 tests by querying the icon element returned by mountComponent and asserting a stable attribute that reflects the size prop (e.g., check for data-size or an aria-label/class meant for accessibility) instead of matching Tailwind utility class names; locate the two tests that call mountComponent({ size: 'lg' }) and mountComponent({ size: 'md' }), find the rendered icon element (by role, test id, or selector used in the component) and assert its data-size or aria attribute equals 'lg'/'md' (or another stable marker exposed by the component) so the tests verify behavior rather than internal utility class names.src/utils/categoryUtil.test.ts (1)
10-87: Prefer function references over string literals indescribe()calls.All four
describeblocks use string literals. Per thevitest/prefer-describe-function-titlerule enforced in this repo, pass the actual function reference instead:♻️ Proposed fix
-describe('getCategoryIcon', () => { +describe(getCategoryIcon, () => { ... -describe('getProviderIcon', () => { +describe(getProviderIcon, () => { ... -describe('getProviderBorderStyle', () => { +describe(getProviderBorderStyle, () => { ... -describe('generateCategoryId', () => { +describe(generateCategoryId, () => {Based on learnings: "In test files under
src/**/*.test.ts, follow thevitest/prefer-describe-function-titlerule by usingdescribe(ComponentOrFunction, ...)instead of a string literal description."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/categoryUtil.test.ts` around lines 10 - 87, Replace string literal titles in the four describe blocks with the actual function references to satisfy vitest/prefer-describe-function-title: change describe('getCategoryIcon', ...) to describe(getCategoryIcon, ...), describe('getProviderIcon', ...) to describe(getProviderIcon, ...), describe('getProviderBorderStyle', ...) to describe(getProviderBorderStyle, ...), and describe('generateCategoryId', ...) to describe(generateCategoryId, ...); ensure the corresponding functions (getCategoryIcon, getProviderIcon, getProviderBorderStyle, generateCategoryId) are in scope/imported in the test file so the describe calls reference the actual functions.src/components/searchbox/v2/NodeSearchInput.test.ts (2)
56-74: TypecreateWrapper's props withComponentProps<typeof NodeSearchInput>instead of duplicating the type manually.Duplicating the prop type inline risks type drift if
NodeSearchInput's props change.♻️ Proposed refactor
+ import type { ComponentProps } from 'vue-component-type-helpers' ... - function createWrapper( - props: Partial<{ - filters: FuseFilterWithValue<ComfyNodeDefImpl, string>[] - activeFilter: FilterChip | null - searchQuery: string - filterQuery: string - }> = {} - ) { + function createWrapper( + props: Partial<ComponentProps<typeof NodeSearchInput>> = {} + ) {Based on learnings: "type the props parameter as
Partial<ComponentProps<typeof Component>>" for test helpers insrc/**/*.test.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchInput.test.ts` around lines 56 - 74, The createWrapper test helper currently types its props inline; replace that manual type with Partial<ComponentProps<typeof NodeSearchInput>> to avoid duplication and drift — update the createWrapper signature to accept props: Partial<ComponentProps<typeof NodeSearchInput>> and keep the rest of the function body (default props merge and mount call) unchanged so tests continue to mount NodeSearchInput with correct typed props.
50-50: Usedescribe(NodeSearchInput, ...)instead of a string literal.Per the
vitest/prefer-describe-function-titlerule enforced in this repo, the first argument todescribeshould be the component reference, not a string.♻️ Proposed fix
- describe('NodeSearchInput', () => { + describe(NodeSearchInput, () => {Based on learnings from
vitest/prefer-describe-function-title— preferdescribe(ComponentOrFunction, ...)over a string literal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchInput.test.ts` at line 50, Replace the string literal describe('NodeSearchInput', ...) with a reference to the component by using describe(NodeSearchInput, ...) so the test file uses the component identifier; locate the describe call in NodeSearchInput.test.ts and change its first argument from the quoted name to the NodeSearchInput symbol (ensure NodeSearchInput is imported/available in the test file).src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue (2)
297-309: Merge the twoifblocks intoif/else if— conditions are mutually exclusive.
node.typeis either'node'or'folder'; evaluating bothifs unconditionally is a minor inefficiency and slightly obscures the intent.♻️ Proposed fix
function handleNodeClick(node: RenderedTreeExplorerNode<ComfyNodeDefImpl>) { if (node.type === 'node' && node.data) { startDrag(node.data) - } - if (node.type === 'folder') { + } else if (node.type === 'folder') { const index = expandedKeys.value.indexOf(node.key) if (index === -1) { expandedKeys.value = [...expandedKeys.value, node.key] } else { expandedKeys.value = expandedKeys.value.filter((k) => k !== node.key) } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue` around lines 297 - 309, The two separate conditionals in handleNodeClick cause mutually exclusive branches to be evaluated sequentially; change them into an if / else if so only one branch runs: keep the existing node.type === 'node' branch calling startDrag(node.data) and convert the subsequent node.type === 'folder' block into an else if that toggles expandedKeys (using the index check and filter logic as currently written). Ensure you preserve the startDrag(node.data) call, the expandedKeys.value update logic, and the function signature RenderedTreeExplorerNode<ComfyNodeDefImpl>.
14-19: Replace raw<button>with the sharedButtoncomponent.Per project convention, raw
<button>elements should be replaced with theButtoncomponent from@/components/ui/button/Button.vuefor consistent design-system styling.♻️ Proposed fix
+ import Button from '@/components/ui/button/Button.vue' ... - <button - :aria-label="$t('g.sort')" - class="flex size-10 shrink-0 cursor-pointer items-center justify-center rounded-lg bg-comfy-input hover:bg-comfy-input-hover border-none" - > - <i class="icon-[lucide--arrow-up-down] size-4" /> - </button> + <Button + :aria-label="$t('g.sort')" + variant="ghost" + size="icon" + class="size-10 shrink-0" + > + <i class="icon-[lucide--arrow-up-down] size-4" /> + </Button>Based on learnings: "replace raw
<button>HTML elements with the shared Button component located atsrc/components/ui/button/Button.vue."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue` around lines 14 - 19, Replace the raw <button> with the shared Button component: import and register Button from "@/components/ui/button/Button.vue" (component name Button), then replace the element in the template with <Button ...> preserving the aria-label="$t('g.sort')" and the existing class string (so styling and size classes remain) and keep the icon child (<i class="icon-[lucide--arrow-up-down] size-4" />) inside the Button; ensure any event bindings or attributes on the original <button> are moved to the Button usage so behavior is unchanged.src/composables/node/useNodePreviewAndDrag.ts (1)
8-8:PREVIEW_WIDTH = 200must match the actual rendered width ofNodePreviewCard.The left/right placement logic in
calculatePreviewPositionuses this constant to avoid viewport overflow. IfNodePreviewCardrenders wider (e.g., due to content or responsive styles), the preview will clip or overflow. Consider reading the actual width frompreviewRefafter the first render — therequestAnimationFramecallback inhandleMouseEnteralready has access topreviewRef.valueand could update the horizontal position at the same time it corrects the vertical one.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodePreviewAndDrag.ts` at line 8, PREVIEW_WIDTH is a hardcoded width that can diverge from the actual rendered NodePreviewCard causing overflow; update calculatePreviewPosition to use the real DOM width by reading previewRef.value.getBoundingClientRect().width (or offsetWidth) instead of PREVIEW_WIDTH, and in handleMouseEnter's requestAnimationFrame where you already access previewRef, compute and store the measured width (fallback to PREVIEW_WIDTH if unavailable) before running the horizontal placement logic so positioning uses the actual rendered size of NodePreviewCard.src/composables/node/useNodePreviewAndDrag.test.ts (1)
24-24: Usedescribe(useNodePreviewAndDrag, ...)instead of a string literal.♻️ Proposed fix
- describe('useNodePreviewAndDrag', () => { + describe(useNodePreviewAndDrag, () => {Based on learnings from
vitest/prefer-describe-function-title— preferdescribe(ComponentOrFunction, ...)over a string literal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodePreviewAndDrag.test.ts` at line 24, Replace the string literal test suite title with the actual function reference: change the describe call in the test that currently reads describe('useNodePreviewAndDrag', ...) to describe(useNodePreviewAndDrag, ...). Update any imports if needed so the symbol useNodePreviewAndDrag is in scope for the test file and ensure the test harness recognizes the function reference instead of the string.src/components/searchbox/v2/NodeSearchFilterPanel.vue (1)
60-66: Options capped at 64 results — consider documenting or extracting the limit.The
.slice(0, 64)caps search results for performance, but the raw sorted dataset (no-query path) is unbounded. IffuseSearch.datais large, the no-query path could render many items. Consider applying a consistent cap to both paths.♻️ Proposed change
+const MAX_OPTIONS = 64 + const options = computed(() => { const { fuseSearch } = chip.filter if (query.value) { - return fuseSearch.search(query.value).slice(0, 64) + return fuseSearch.search(query.value).slice(0, MAX_OPTIONS) } - return fuseSearch.data.slice().sort() + return fuseSearch.data.slice().sort().slice(0, MAX_OPTIONS) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchFilterPanel.vue` around lines 60 - 66, The options computed currently caps search results with fuseSearch.search(query.value).slice(0, 64) but returns the unbounded fuseSearch.data on the no-query path; make the behavior consistent by extracting the limit into a named constant (e.g., RESULT_LIMIT) and applying it to both branches (use slice(0, RESULT_LIMIT) for the search results and on fuseSearch.data before sort), or alternatively document the limit if intentional; update the options computed to reference the constant and ensure both branches are capped.src/components/searchbox/NodeSearchBoxPopover.vue (1)
94-99: Magic number1320for the preview breakpoint.The
1320px threshold forenableNodePreviewis undocumented. Consider extracting this as a named constant for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/NodeSearchBoxPopover.vue` around lines 94 - 99, The magic number 1320 used in the computed enableNodePreview should be extracted to a named constant for clarity and reuse: define a constant (e.g. NODE_PREVIEW_BREAKPOINT = 1320) near the top of NodeSearchBoxPopover.vue and replace the literal in the enableNodePreview computed (which references useSearchBoxV2, settingStore.get('Comfy.NodeSearchBoxImpl.NodePreview'), and windowWidth) with that constant so the breakpoint is documented and easy to change.src/components/common/TreeExplorerV2Node.test.ts (1)
70-100: Consider stronger prop typing inmountComponent.Using
Record<string, unknown>for props loses type safety. Per project conventions, preferPartial<ComponentProps<typeof TreeExplorerV2Node>>and cast when passing tomount.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/TreeExplorerV2Node.test.ts` around lines 70 - 100, Replace the loose props typing in mountComponent with a stronger type: change the first parameter from Record<string, unknown> to Partial<ComponentProps<typeof TreeExplorerV2Node>> (import ComponentProps from Vue/types if needed), and when spreading into mount's props, cast the value to ComponentProps<typeof TreeExplorerV2Node> so the mount call receives the correct typed props; keep options.treeItemStub and provide typing as-is.src/components/searchbox/NodeSearchBox.vue (1)
158-164: Prefer a function declaration over a function expression.Per coding guidelines, function declarations are preferred over function expressions when possible.
♻️ Suggested change
-const onAddNode = (nodeDef: ComfyNodeDefImpl, event?: MouseEvent) => { +function onAddNode(nodeDef: ComfyNodeDefImpl, event?: MouseEvent) { telemetry?.trackNodeSearchResultSelected({ node_type: nodeDef.name, last_query: currentQuery.value }) emit('addNode', nodeDef, event) -} +}As per coding guidelines: "Do not use function expressions if it's possible to use function declarations instead."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/NodeSearchBox.vue` around lines 158 - 164, Replace the onAddNode function expression with a function declaration: define function onAddNode(nodeDef: ComfyNodeDefImpl, event?: MouseEvent) { ... } (keeping the existing body that calls telemetry?.trackNodeSearchResultSelected and emit('addNode', nodeDef, event)); update any references if needed so the declared function name is used in place of the previous const onAddNode, ensuring TypeScript parameter types and optional event remain unchanged.src/components/node/NodePreviewCard.vue (2)
19-24:text-neutral-400is a hardcoded color; prefer a semantic token.
text-neutral-400won't adapt to theme changes. Considertext-muted-foreground(used elsewhere in this component for similar metadata text).♻️ Suggested change
- class="text-xs text-neutral-400 -mt-1" + class="text-xs text-muted-foreground -mt-1"As per coding guidelines: "use semantic values from
style.csstheme instead."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/node/NodePreviewCard.vue` around lines 19 - 24, Replace the hardcoded color class on the category path element with the semantic token: in NodePreviewCard.vue, locate the <p> that is conditional on showCategoryPath and nodeDef.category (the nodeDef.category.replaceAll(...) line) and change the class from text-neutral-400 to the semantic token used elsewhere (text-muted-foreground) so the metadata text follows the theme tokens.
117-123: Magic number24in the ResizeObserver callback should be a named constant.The
24representsp-3top + bottom padding (12 px × 2). If the padding class changes, this number silently diverges. Extracting it keepsSCALE_FACTORand the padding in one place.♻️ Suggested change
const SCALE_FACTOR = 0.5 +const PREVIEW_CONTAINER_PADDING_PX = 24 // p-3 top + bottom useResizeObserver(previewWrapperRef, (entries) => { const entry = entries[0] if (entry && previewContainerRef.value) { const scaledHeight = entry.contentRect.height * SCALE_FACTOR - previewContainerRef.value.style.height = `${scaledHeight + 24}px` + previewContainerRef.value.style.height = `${scaledHeight + PREVIEW_CONTAINER_PADDING_PX}px` } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/node/NodePreviewCard.vue` around lines 117 - 123, The ResizeObserver callback in useResizeObserver mutates previewContainerRef.value.style.height using a magic number 24 for padding; extract that magic number into a named constant (e.g. PREVIEW_PADDING_PX or PREVIEW_WRAPPER_PADDING = 24) alongside SCALE_FACTOR and use that constant when computing height: const scaledHeight = entry.contentRect.height * SCALE_FACTOR; previewContainerRef.value.style.height = `${scaledHeight + PREVIEW_PADDING_PX}px` so padding and scale are declared together and easy to update; update references to previewWrapperRef/previewContainerRef and SCALE_FACTOR accordingly.src/components/searchbox/v2/NodeSearchFilterBar.vue (1)
3-19: Consider using the sharedButtoncomponent instead of raw<button>elements.These chip/toggle buttons could leverage the
Buttoncomponent fromsrc/components/ui/button/Button.vuefor design-system consistency.Based on learnings: "In the ComfyUI_frontend Vue codebase, replace raw
<button>HTML elements with the shared Button component located atsrc/components/ui/button/Button.vue."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchFilterBar.vue` around lines 3 - 19, Replace the raw <button> loop with the shared Button component (Button.vue) to ensure design-system consistency: import and register the Button component in the NodeSearchFilterBar component, render <Button> for each item in chips instead of the raw <button>, map the dynamic attributes (type, aria-pressed using activeChipKey === chip.key, the class variants currently computed via cn, and the click handler emit('selectChip', chip)) to the Button's props/attributes or slots so visual states (active/inactive/hover) match the original behavior, and keep the text content as chip.label; ensure the key remains :key="chip.key" and that the Button receives the same accessibility attribute (aria-pressed) and click behavior.src/components/searchbox/v2/NodeSearchListItem.vue (1)
127-135:<style scoped>block violates the "avoid<style>blocks" guideline.The
:deep(.highlight)selector is necessary because the class is injected viav-htmland Tailwind utilities can't target that content. If thehighlightQueryXSS concern above is resolved by moving to a DOM-based approach (e.g., splitting the string and using template interpolation instead ofv-html), the<style>block could be eliminated entirely and the highlight styling inlined with Tailwind.As per coding guidelines: "Use Tailwind 4 for styling in Vue components; avoid
<style>blocks."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchListItem.vue` around lines 127 - 135, The component currently uses a scoped <style> with :deep(.highlight) because highlighted HTML is injected via v-html; remove the <style scoped> block and eliminate v-html by changing the rendering in NodeSearchListItem.vue to a DOM-based approach (e.g., update the logic around highlightQuery to split the string into plain and highlighted segments and render them with template interpolation), wrap highlighted segments in a span with Tailwind utility classes (e.g., bg/opacity, font-semibold, rounded, px) instead of the .highlight CSS, and ensure all places referencing :deep(.highlight) or v-html are updated so the custom style is no longer needed.src/composables/node/useNodePricing.ts (1)
489-519: Errors inscheduleEvaluationare silently swallowed.Line 499's
.catch(() => { ... })no longer logs the error. While this avoids console spam from repeated evaluation cycles, it can make debugging pricing issues harder. Aconsole.debugor conditional log at a lower verbosity level could help.Suggested change
- .catch(() => { + .catch((e) => { + console.debug('[pricing/jsonata] evaluation error:', e) // Cache empty to avoid retry-spam for same signature🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodePricing.ts` around lines 489 - 519, The catch block inside scheduleEvaluation (the Promise handling for rule._compiled.evaluate) currently swallows errors; modify the .catch handler to log the caught error at a low verbosity (e.g., console.debug or a conditional debug logger) while keeping the existing behavior of caching an empty label for the matching signature; keep references to the same symbols (rule._compiled.evaluate, formatPricingResult, desiredSig, cache, inflight) and ensure the error is included in the log so debugging pricing evaluation failures is possible without producing noisy console spam.src/composables/node/useNodePricing.test.ts (1)
130-141: Timing-based async assertions usesetTimeout(r, 50)throughout.The
await new Promise((r) => setTimeout(r, 50))pattern for waiting on async JSONata evaluation is fragile under load. This is pre-existing across the file so not a blocker for this PR, but consider eventually switching to a utility likevi.waitFororflushPromisesfor deterministic async control.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodePricing.test.ts` around lines 130 - 141, The test uses a fragile fixed delay to wait for async JSONata evaluation—replace the await new Promise((r) => setTimeout(r, 50)) in this test (and similar tests) with a deterministic wait such as vi.waitFor or a flushPromises helper so getNodeDisplayPrice (from useNodePricing) has completed before asserting; update the test to call getNodeDisplayPrice(node) then await vi.waitFor(() => { expect(getNodeDisplayPrice(node)).toBe(creditsLabel(0.05)) }) or use flushPromises followed by the expect to avoid timing-based flakiness.src/components/common/SearchBoxV2.vue (1)
59-75: Default icon uses PrimeIcons while the clear button uses Lucide.The default
iconprop is'pi pi-search'(PrimeIcons), but the clear button on line 28 usesicon-[lucide--x]. Consider using'icon-[lucide--search]'as the default to maintain consistent icon sourcing across this component, especially given the guideline to avoid new PrimeVue component usage.Suggested change
- icon = 'pi pi-search', + icon = 'icon-[lucide--search]',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/SearchBoxV2.vue` around lines 59 - 75, The default icon prop for SearchBoxV2 is using PrimeIcons ('pi pi-search') while the clear button uses Lucide ('icon-[lucide--x]'); update the default value of the icon prop in the defineProps block (icon = 'pi pi-search') to use the Lucide search class (e.g., icon = 'icon-[lucide--search]') so the component consistently uses Lucide icons across the icon prop, the clear button, and any places referencing the icon prop.src/composables/node/useNodeDragToCanvas.test.ts (2)
6-13: Consider usingvi.hoisted()for the module-level mock variables.The coding guideline states: "Keep module mocks contained; do not use global mutable state within test files; use
vi.hoisted()if necessary for per-test Arrange phase manipulation." The module-levelvi.fn()objects carry mutable state that is shared across all tests.Vitest does allow
mock-prefixed variable names to be referenced insidevi.mock()factory functions withoutvi.hoisted()as a documented shortcut, so the current pattern is functional. However, usingvi.hoisted()makes the intent explicit and fully aligns with the project guideline.♻️ Proposed refactor with `vi.hoisted()`
+const { + mockAddNodeOnGraph, + mockConvertEventToCanvasOffset, + mockCanvas +} = vi.hoisted(() => { + const mockConvertEventToCanvasOffset = vi.fn() + return { + mockAddNodeOnGraph: vi.fn(), + mockConvertEventToCanvasOffset, + mockCanvas: { + canvas: { getBoundingClientRect: vi.fn() }, + convertEventToCanvasOffset: mockConvertEventToCanvasOffset + } + } +}) -const mockAddNodeOnGraph = vi.fn() -const mockConvertEventToCanvasOffset = vi.fn() -const mockCanvas = { - canvas: { - getBoundingClientRect: vi.fn() - }, - convertEventToCanvasOffset: mockConvertEventToCanvasOffset -}Based on learnings: "Keep module mocks contained; do not use global mutable state within test files; use
vi.hoisted()if necessary for per-test Arrange phase manipulation."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodeDragToCanvas.test.ts` around lines 6 - 13, The module-level mocks (mockAddNodeOnGraph, mockConvertEventToCanvasOffset, mockCanvas) are mutable across tests; replace their vi.fn() initializations with vi.hoisted(() => ({ ... })) or vi.hoisted(() => vi.fn()) so each test gets isolated instances: hoist mockAddNodeOnGraph and mockConvertEventToCanvasOffset via vi.hoisted(() => vi.fn()), and hoist mockCanvas to return an object whose canvas.getBoundingClientRect and convertEventToCanvasOffset are created inside the vi.hoisted factory; update any vi.mock factories or test code that referenced these variables to use the hoisted versions.
36-37: Replacevi.clearAllMocks()withvi.resetAllMocks()for true per-test isolation.
vi.clearAllMocks()only clears call history (.mockClear()); it does not resetmockReturnValue/mockImplementationstate. Consequently,mockReturnValue([150, 150])set in "should add node when pointer is over canvas" (line 164) andmockReturnValue([200, 200])set in thehandleNativeDropsuite (line 271/307/324) persist into subsequent tests that never call.mockReturnValue()themselves.The tests currently survive because the composable performs a bounds check before calling
convertEventToCanvasOffset, so the stale return value is never consumed in "outside canvas" scenarios. If the composable's implementation order ever changes, these tests could silently produce wrong results.vi.resetAllMocks()(.mockReset()) resets both call history and implementations, giving genuine isolation. The switch is safe here because everymockReturnValuecall is inside a specificitbody, not inside avi.mock()factory.♻️ Proposed fix
beforeEach(async () => { vi.resetModules() - vi.clearAllMocks() + vi.resetAllMocks() const module = await import('./useNodeDragToCanvas') useNodeDragToCanvas = module.useNodeDragToCanvas })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodeDragToCanvas.test.ts` around lines 36 - 37, Replace the vi.clearAllMocks() call with vi.resetAllMocks() in the test setup (currently calling vi.resetModules() then vi.clearAllMocks()) so each test truly resets mock implementations; specifically update the setup that invokes vi.clearAllMocks() to use vi.resetAllMocks() instead, ensuring mocks used in tests like "should add node when pointer is over canvas" and the handleNativeDrop suite do not leak implementations between tests.
Comment on lines
+44
to
+50
| if (filled) { | ||
| return { | ||
| borderColor: borderStyle, | ||
| backgroundColor: `${borderStyle}33`, | ||
| color: borderStyle | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
${borderStyle}33 opacity trick only works for 6-digit hex colors.
Appending '33' works only when borderStyle is exactly a 6-digit hex (e.g., #ff6b00 → #ff6b0033). It produces invalid CSS for:
- 3-digit hex:
#f60→#f6033 - RGB/RGBA:
rgb(255, 107, 0)→rgb(255, 107, 0)33 - Named colors:
red→red33 - CSS variables:
var(--c)→var(--c)33
color-mix() is broadly supported and handles any valid color:
🐛 Proposed fix using `color-mix()`
if (filled) {
return {
borderColor: borderStyle,
- backgroundColor: `${borderStyle}33`,
+ backgroundColor: `color-mix(in srgb, ${borderStyle} 20%, transparent)`,
color: borderStyle
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/common/BadgePill.vue` around lines 44 - 50, The current
filled-branch appends "33" to borderStyle which only works for 6-digit hex;
change it to produce a robust translucent background using color-mix and fall
back to the hex33 trick for legacy cases: detect if borderStyle matches a
6-digit hex (e.g., /^#([0-9a-f]{6})$/i) and use `${borderStyle}33`, otherwise
set backgroundColor via `color-mix(in srgb, ${borderStyle} <alpha> ,
transparent)` (choose an appropriate alpha like 20% or 0.2); update the returned
object in the filled branch (the block referencing filled and borderStyle in
BadgePill.vue) to use this conditional logic so named colors, rgb/rgba, CSS
vars, and functions work correctly.
Comment on lines
+76
to
+83
| function cleanupGlobalListeners() { | ||
| if (!listenersSetup) return | ||
| listenersSetup = false | ||
|
|
||
| document.removeEventListener('pointermove', updatePosition) | ||
| document.removeEventListener('pointerup', endDrag, true) | ||
| document.removeEventListener('keydown', handleKeydown) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find where cleanupGlobalListeners is called
rg -n "cleanupGlobalListeners" --type=ts --type=vue -A3 -B3Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Find and inspect NodeDragPreview.vue
fd "NodeDragPreview.vue" -x cat {}Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1830
🏁 Script executed:
#!/bin/bash
# Get full context of useNodeDragToCanvas.ts to understand the complete flow
fd "useNodeDragToCanvas.ts" -x cat {}Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2939
cleanupGlobalListeners doesn't call cancelDrag, leaving stale drag state if the component unmounts during a click-mode drag.
Scenario:
- Click-mode drag starts →
isDragging = true,draggedNodeis set. NodeDragPreviewunmounts (sidebar collapses) →cleanupGlobalListenersremoves thepointeruplistener.- User releases the mouse button →
endDrag/cancelDragis never called. isDragginganddraggedNoderemain set.- Sidebar reopens →
NodeDragPreviewremounts, callssetupGlobalListeners. - The next
pointerupfiresendDrag, which callsaddNodeAtPositionwith the staledraggedNode.
🐛 Proposed fix
function cleanupGlobalListeners() {
if (!listenersSetup) return
listenersSetup = false
document.removeEventListener('pointermove', updatePosition)
document.removeEventListener('pointerup', endDrag, true)
document.removeEventListener('keydown', handleKeydown)
+
+ // If a click-mode drag was in progress when listeners were torn down, cancel it
+ if (isDragging.value && dragMode.value === 'click') {
+ cancelDrag()
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/composables/node/useNodeDragToCanvas.ts` around lines 76 - 83,
cleanupGlobalListeners currently only removes DOM listeners and can leave
click-mode drag state (isDragging, draggedNode) stale if the component unmounts;
update cleanupGlobalListeners to call cancelDrag() (the same cleanup path used
by endDrag) whenever listenersSetup is true before removing event listeners so
drag state is reset if NodeDragPreview unmounts; reference
cleanupGlobalListeners, cancelDrag, endDrag, isDragging, draggedNode and ensure
this runs when NodeDragPreview unmounts or before setupGlobalListeners can be
called again.
Comment on lines
+310
to
+314
| 'Comfy.NodeSearchBoxImpl': z.enum([ | ||
| 'default', | ||
| 'v1 (legacy)', | ||
| 'litegraph (legacy)' | ||
| ]), |
Contributor
There was a problem hiding this comment.
Schema enum change is consistent — but see migration concern in coreSettings.ts.
The enum update is correct in isolation. However, because 'simple' is no longer a valid enum member, any persisted 'simple' value will fail Zod validation at runtime. The fix (a migrateDeprecatedValue entry) belongs in coreSettings.ts — see that file's review comment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/schemas/apiSchema.ts` around lines 310 - 314, Update coreSettings.ts to
migrate persisted deprecated enum values for the schema change: add a
migrateDeprecatedValue entry targeting the setting key "Comfy.NodeSearchBoxImpl"
(the enum you changed in apiSchema.ts) that converts the old value "simple" to
the new valid value (e.g., "default"). Ensure the migration is applied during
settings load by using the same migrateDeprecatedValue mechanism already used in
coreSettings.ts so runtime Zod validation no longer fails for persisted "simple"
values.
Comment on lines
+79
to
+101
| export const getNodeSource = ( | ||
| python_module?: string, | ||
| essentials_category?: string, | ||
| name?: string | ||
| ): NodeSource => { | ||
| const resolvedEssentialsCategory = getEssentialsCategory( | ||
| name, | ||
| essentials_category | ||
| ) | ||
| if (!python_module) { | ||
| return UNKNOWN_NODE_SOURCE | ||
| } | ||
| const modules = python_module.split('.') | ||
| if (['nodes', 'comfy_extras', 'comfy_api_nodes'].includes(modules[0])) { | ||
| if (resolvedEssentialsCategory) { | ||
| const moduleName = modules[1] ?? modules[0] ?? 'essentials' | ||
| const displayName = shortenNodeName(moduleName.split('@')[0]) | ||
| return { | ||
| type: NodeSourceType.Essentials, | ||
| className: 'comfy-essentials', | ||
| displayText: displayName, | ||
| badgeText: displayName | ||
| } | ||
| } else if ( |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all calls to getNodeSource to understand the impact
rg -n --type=ts -C3 'getNodeSource\(' -g '!*.test.*' | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1432
🏁 Script executed:
#!/bin/bash
# Check the getEssentialsCategory function implementation
sed -n '1,78p' src/types/nodeSource.ts | tail -30Repository: Comfy-Org/ComfyUI_frontend
Length of output: 869
🏁 Script executed:
#!/bin/bash
# Check the full getNodeSource function to understand the classification logic
sed -n '79,150p' src/types/nodeSource.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1626
🏁 Script executed:
#!/bin/bash
# Verify the exact signatures of the existing callers
echo "=== nodeHelpUtil.ts call ==="
sed -n '14,20p' src/workbench/utils/nodeHelpUtil.ts
echo -e "\n=== nodeHelpService.ts call ==="
sed -n '5,12p' src/services/nodeHelpService.ts
echo -e "\n=== nodeDefStore.ts call ==="
sed -n '173,180p' src/stores/nodeDefStore.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 800
🏁 Script executed:
#!/bin/bash
# Check if there are any other callers or tests that verify the classification behavior
echo "=== All references to getNodeSource (including imports) ==="
rg -n 'getNodeSource' src --type=ts -g '!*.test.*'
echo -e "\n=== Check if any code relies on classification type checks ==="
rg -n 'NodeSourceType\.Core|NodeSourceType\.Essentials' src --type=ts -g '!*.test.*' | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1083
🏁 Script executed:
#!/bin/bash
# Check what the Core type checks do
echo "=== nodeDefStore.ts Core check ==="
sed -n '195,205p' src/stores/nodeDefStore.ts
echo -e "\n=== nodeOrganizationService.ts Core check ==="
sed -n '70,80p' src/services/nodeOrganizationService.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 934
🏁 Script executed:
#!/bin/bash
# Check the broader context: are there any tests or documentation about the classification priority?
echo "=== Check for tests related to getNodeSource ==="
find src -name "*.test.*" -o -name "*.spec.*" | xargs rg -l "getNodeSource" 2>/dev/null || echo "No tests found"
echo -e "\n=== Check nodeOrganizationService to see full classification handling ==="
sed -n '60,90p' src/services/nodeOrganizationService.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1300
🏁 Script executed:
#!/bin/bash
# Check the test file to understand expected behavior
cat src/types/nodeSource.test.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 3851
🏁 Script executed:
#!/bin/bash
# Check if nodeOrganizationService should handle Essentials type
echo "=== Check all NodeSourceType values ==="
sed -n '1,20p' src/types/nodeSource.ts
echo -e "\n=== Check if there's any mention of Essentials in organization ==="
rg -n 'Essentials' src/services/nodeOrganizationService.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 598
🏁 Script executed:
#!/bin/bash
# Check if nodeOrganizationService has been updated elsewhere or if there's a missing case
echo "=== Full node organization service classification ==="
sed -n '65,95p' src/services/nodeOrganizationService.ts
echo -e "\n=== Check if there are any uncommitted changes or other references ==="
rg -n 'api_node\|isCoreNode' src --type=ts | head -15Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1223
🏁 Script executed:
#!/bin/bash
# Check if Blueprint is also missing from the organization service handling
echo "=== Check if Blueprint nodes are tested ==="
rg -n 'Blueprint' src/types/nodeSource.test.ts
echo -e "\n=== Summary: Check all places where NodeSourceType is matched ==="
rg -n 'nodeSource\.type ===' src --type=ts -A1Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1923
🏁 Script executed:
#!/bin/bash
# Verify if this is part of the PR or if it's pre-existing
echo "=== Check nodeOrganizationService more carefully for any Essentials handling ==="
rg -n 'Essentials|essentials' src/services/nodeOrganizationService.ts
echo -e "\n=== Check the exact code path in getNodePath ==="
sed -n '70,85p' src/services/nodeOrganizationService.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1083
🏁 Script executed:
#!/bin/bash
# Check what happens with Blueprint nodes in the source grouping strategy
echo "=== Full context of source grouping in nodeOrganizationService ==="
sed -n '69,80p' src/services/nodeOrganizationService.ts
echo -e "\n=== Check if Blueprint is handled elsewhere in the grouping strategies ==="
rg -n 'Blueprint' src/services/nodeOrganizationService.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 750
🏁 Script executed:
#!/bin/bash
# Check if Essentials nodes will properly group in the source strategy
echo "=== Verify the classification fallthrough issue ==="
sed -n '69,85p' src/services/nodeOrganizationService.ts
echo -e "\n=== Check what types are in NodeSourceType ==="
sed -n '1,10p' src/types/nodeSource.ts
echo -e "\n=== Double-check callers of getNodeSource - do they pass all params? ==="
rg -B2 -A2 'getNodeSource\(' src --type=ts -g '!*.test.*' | grep -A2 'getNodeSource'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1692
🏁 Script executed:
#!/bin/bash
# Check what happens to Essentials and Blueprint nodes in nodeOrganizationService
echo "=== Is there a Bug Alert needed for nodeOrganizationService? ==="
echo "Essentials and Blueprint nodes will fall through to 'Unknown' in the source grouping"
echo ""
echo "Current nodeSourceType values: Core, CustomNodes, Blueprint, Essentials, Unknown"
echo "But nodeOrganizationService only handles: Core, CustomNodes, and everything else -> Unknown"
echo ""
echo "=== Check if this is the file that needs fixing ==="
sed -n '69,82p' src/services/nodeOrganizationService.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 998
🏁 Script executed:
#!/bin/bash
# Final verification: check if this nodeOrganizationService issue is in the PR scope
echo "=== Check git status or recent changes ==="
git log --oneline -n 5 src/services/nodeOrganizationService.ts 2>/dev/null || echo "Git history not available"
echo -e "\n=== Confirm the exact issue ==="
echo "NodeSourceType enum has: Core, CustomNodes, Blueprint, Essentials, Unknown"
echo ""
echo "nodeOrganizationService.getNodePath only explicitly handles:"
rg -n 'NodeSourceType\.(Core|CustomNodes)' src/services/nodeOrganizationService.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 597
Essentials classification priority is intentional, but nodeOrganizationService needs updating to display them correctly.
The essentials priority over core/custom is confirmed by tests and intentional for v2 UI. The three callers of getNodeSource are unaffected: nodeHelpUtil.ts and nodeHelpService.ts only pass python_module (not essentials info), while nodeDefStore.ts properly passes all three parameters to capture essentials classification.
However, there's a critical gap: nodeOrganizationService.ts (lines 69–82) has a "source" grouping strategy that only explicitly handles NodeSourceType.Core and NodeSourceType.CustomNodes. Essentials nodes will fall through to "Unknown" in the node library UI, along with Blueprint nodes. The getNodePath function must be updated to handle both NodeSourceType.Essentials and NodeSourceType.Blueprint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types/nodeSource.ts` around lines 79 - 101, Update the grouping logic in
nodeOrganizationService.ts's getNodePath to explicitly handle
NodeSourceType.Essentials and NodeSourceType.Blueprint (in addition to the
existing NodeSourceType.Core and NodeSourceType.CustomNodes branches) so
essentials and blueprint nodes do not fall through to "Unknown"; add case
handlers (or if-branches) that return the correct library path/label for
NodeSourceType.Essentials and NodeSourceType.Blueprint based on source.type
(mirroring how Core and CustomNodes are handled) so getNodeSource results are
displayed properly in the node library UI.
| @@ -1,14 +1,20 @@ | |||
| import type { ComfyNodeDefImpl } from '@/stores/nodeDefStore' | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if nodeDefStore or its imports reference treeExplorerTypes
echo "=== Checking nodeDefStore.ts for treeExplorerTypes imports ==="
rg -n "treeExplorerTypes" --type=ts src/stores/nodeDefStore.ts
echo ""
echo "=== Checking all files in src/stores/ for treeExplorerTypes imports ==="
rg -rn "treeExplorerTypes" --type=ts src/stores/
echo ""
echo "=== Checking what's imported in nodeDefStore.ts ==="
head -30 src/stores/nodeDefStore.ts
echo ""
echo "=== Reading treeExplorerTypes.ts to see the problematic lines ==="
cat -n src/types/treeExplorerTypes.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 5556
🏁 Script executed:
#!/bin/bash
# Find where ComfyNodeDefImpl is defined
echo "=== Where ComfyNodeDefImpl is defined ==="
rg -n "class ComfyNodeDefImpl|type ComfyNodeDefImpl|interface ComfyNodeDefImpl" --type=ts src/
echo ""
echo "=== All usages of ComfyNodeDefImpl ==="
rg -rn "ComfyNodeDefImpl" --type=ts src/ | head -20
echo ""
echo "=== Check nodeDefStore exports ==="
grep -n "export" src/stores/nodeDefStore.ts | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2612
🏁 Script executed:
#!/bin/bash
# Check the ComfyNodeDefImpl class definition and its size
echo "=== ComfyNodeDefImpl class definition ==="
sed -n '35,100p' src/stores/nodeDefStore.ts
echo ""
echo "=== Check if ComfyNodeDefImpl is instantiated in tests or only as a type ==="
rg -n "new ComfyNodeDefImpl|new n\(" --type=ts src/ | head -10Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3051
🏁 Script executed:
#!/bin/bash
# Check if the actual circular dependency causes issues at runtime/build
echo "=== Check the imports more carefully ==="
echo "treeExplorerTypes imports:"
sed -n '1,5p' src/types/treeExplorerTypes.ts
echo ""
echo "nodeDefStore imports around line 31:"
sed -n '25,35p' src/stores/nodeDefStore.ts
echo ""
echo "=== Verify: treeExplorerTypes only imports from nodeDefStore, not re-exports it ==="
grep -n "export.*nodeDefStore\|export.*ComfyNodeDefImpl" src/types/treeExplorerTypes.ts
echo ""
echo "=== Check if ComfyNodeDefImpl could be moved to a types file ==="
grep -A 5 "implements ComfyNodeDefV1, ComfyNodeDefV2" src/stores/nodeDefStore.ts | head -10Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1072
🏁 Script executed:
#!/bin/bash
# Final verification: Check if there are any other imports that could break the circular dependency
echo "=== All imports in treeExplorerTypes.ts ==="
head -5 src/types/treeExplorerTypes.ts
echo ""
echo "=== Check if ComfyNodeDefImpl is instantiated anywhere else outside nodeDefStore ==="
rg -rn "new ComfyNodeDefImpl|instanceof ComfyNodeDefImpl" --type=ts src/ | grep -v nodeDefStore.ts | grep -v "\.test\.ts"
echo ""
echo "=== Verify treeExplorerTypes does not re-export or depend on nodeDefStore beyond type import ==="
grep -n "nodeDefStore\|ComfyNodeDefImpl" src/types/treeExplorerTypes.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 970
Move domain-specific types to src/types/nodeLibraryTypes.ts to resolve circular dependency and domain coupling.
treeExplorerTypes.ts is a generic module, but NodeLibrarySection and InjectKeyContextMenuNode couple it to the node library domain via ComfyNodeDefImpl. More critically, this creates a circular dependency: treeExplorerTypes → nodeDefStore → treeExplorerTypes. While both imports use import type (erased at compile time), this circular module dependency causes issues with module resolution and tooling.
Additionally, exporting ComfyNodeDefImpl from a store violates the principle of keeping internal store implementations private. These types should be moved to a dedicated nodeLibraryTypes.ts module to decouple the generic tree explorer from the node library domain.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types/treeExplorerTypes.ts` at line 1, treeExplorerTypes.ts currently
imports the domain-specific ComfyNodeDefImpl which couples the generic tree
explorer to the node library and causes a circular dependency; move the domain
types into a new src/types/nodeLibraryTypes.ts and update references: extract
the definitions/types used by NodeLibrarySection and InjectKeyContextMenuNode
from treeExplorerTypes.ts into nodeLibraryTypes.ts, replace the import of
ComfyNodeDefImpl in treeExplorerTypes.ts with imports from
src/types/nodeLibraryTypes.ts, and update any exports so stores (like
nodeDefStore) no longer need to expose ComfyNodeDefImpl; ensure all files
referencing NodeLibrarySection or InjectKeyContextMenuNode import the domain
types from nodeLibraryTypes.ts instead of nodeDefStore to break the circular
dependency.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/settings/constants/coreSettings.ts (1)
33-40:⚠️ Potential issue | 🟠 MajorAdd
migrateDeprecatedValueto handle persisted'simple'valuesThe
'simple'option is removed without a migration guard. Users who had'simple'stored will either fail Zod validation or silently fall back to the default. The same migration pattern is used forComfy.UseNewMenu('Floating'/'Bottom'→'Top') andComfy.WorkflowTabsPosition.Based on the PR context,
'simple'maps to the renamed'v1 (legacy)'.🔧 Proposed fix
{ id: 'Comfy.NodeSearchBoxImpl', category: ['Comfy', 'Node Search Box', 'Implementation'], experimental: true, name: 'Node search box implementation', type: 'combo', options: ['default', 'v1 (legacy)', 'litegraph (legacy)'], - defaultValue: 'default' + defaultValue: 'default', + migrateDeprecatedValue: (val: unknown) => { + if (val === 'simple') return 'v1 (legacy)' + return val as string + } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/settings/constants/coreSettings.ts` around lines 33 - 40, Add a migrateDeprecatedValue handler to the setting with id 'Comfy.NodeSearchBoxImpl' so persisted values of 'simple' are translated to 'v1 (legacy)'; implement migrateDeprecatedValue to detect the deprecated 'simple' string and return 'v1 (legacy)' (otherwise return the original value) to avoid Zod validation failures or silent falls back to default.
🧹 Nitpick comments (2)
src/components/searchbox/NodeSearchBoxPopover.vue (2)
94-99: Extract magic breakpoint1320to a named constant
1320is not a standard Tailwind breakpoint (xl= 1280px,2xl= 1536px) and its intent (minimum width at which the preview card can fit beside the dialog without clipping) is non-obvious at the call site.♻️ Suggested refactor
+// Minimum viewport width at which the node preview panel can sit +// beside the dialog (dialog max-width 56rem ≈ 896px + preview card margin) +const MIN_WIDTH_FOR_PREVIEW = 1320 const enableNodePreview = computed( () => useSearchBoxV2.value && settingStore.get('Comfy.NodeSearchBoxImpl.NodePreview') && - windowWidth.value >= 1320 + windowWidth.value >= MIN_WIDTH_FOR_PREVIEW )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/NodeSearchBoxPopover.vue` around lines 94 - 99, Extract the magic number 1320 into a named constant (e.g., NODE_PREVIEW_BREAKPOINT) and replace the literal in the computed `enableNodePreview` expression so the condition uses `windowWidth.value >= NODE_PREVIEW_BREAKPOINT`; define the constant near the top of this component (or import from a shared breakpoints/constants module if available) and add a short comment explaining that it represents the minimum width required to render the preview card beside the dialog.
9-15: Prefer Tailwindoverflow-visibleclass over the separatestylepropertyThe
style: 'overflow: visible'entry can be folded into theclassstring, keeping styling fully in Tailwind and eliminating the mixedclass/styleduality in the PT object.♻️ Suggested refactor
root: { - class: useSearchBoxV2 + class: cn(useSearchBoxV2 ? 'w-4/5 min-w-[32rem] max-w-[56rem] border-0 bg-transparent mt-[10vh] max-md:w-[95%] max-md:min-w-0' - : 'invisible-dialog-root', - style: useSearchBoxV2 ? 'overflow: visible' : undefined + : 'invisible-dialog-root', + useSearchBoxV2 && 'overflow-visible')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/NodeSearchBoxPopover.vue` around lines 9 - 15, The PT object mixes a style property with Tailwind classes; replace the inline style by adding the Tailwind overflow-visible class to the useSearchBoxV2 branch of the class string and remove the style property entirely. Specifically, update the object where useSearchBoxV2 is checked (the entry currently containing style: useSearchBoxV2 ? 'overflow: visible' : undefined) to include 'overflow-visible' in the true branch of the class value (the same string built when useSearchBoxV2 is true) and delete the style key so all styling for the popover is kept in classes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/platform/settings/constants/coreSettings.ts`:
- Around line 33-40: Add a migrateDeprecatedValue handler to the setting with id
'Comfy.NodeSearchBoxImpl' so persisted values of 'simple' are translated to 'v1
(legacy)'; implement migrateDeprecatedValue to detect the deprecated 'simple'
string and return 'v1 (legacy)' (otherwise return the original value) to avoid
Zod validation failures or silent falls back to default.
---
Duplicate comments:
In `@src/schemas/apiSchema.ts`:
- Around line 330-334: The enum Comfy.NodeSearchBoxImpl removed the 'simple'
value but there is no migration for persisted settings; add a
migrateDeprecatedValue call in coreSettings.ts for the Comfy.NodeSearchBoxImpl
setting that maps the legacy 'simple' value to the current equivalent (e.g.,
'default'), using the same migrateDeprecatedValue helper used for other keys
(see how Comfy.UseNewMenu maps 'Floating' → 'Top') so that existing saved
settings with 'simple' are converted during load.
---
Nitpick comments:
In `@src/components/searchbox/NodeSearchBoxPopover.vue`:
- Around line 94-99: Extract the magic number 1320 into a named constant (e.g.,
NODE_PREVIEW_BREAKPOINT) and replace the literal in the computed
`enableNodePreview` expression so the condition uses `windowWidth.value >=
NODE_PREVIEW_BREAKPOINT`; define the constant near the top of this component (or
import from a shared breakpoints/constants module if available) and add a short
comment explaining that it represents the minimum width required to render the
preview card beside the dialog.
- Around line 9-15: The PT object mixes a style property with Tailwind classes;
replace the inline style by adding the Tailwind overflow-visible class to the
useSearchBoxV2 branch of the class string and remove the style property
entirely. Specifically, update the object where useSearchBoxV2 is checked (the
entry currently containing style: useSearchBoxV2 ? 'overflow: visible' :
undefined) to include 'overflow-visible' in the true branch of the class value
(the same string built when useSearchBoxV2 is true) and delete the style key so
all styling for the popover is kept in classes.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
src/components/searchbox/v2/NodeSearchCategorySidebar.test.ts (2)
20-20: Use the component import as thedescribetitle, not a string literal.The project's vitest rule (
vitest/prefer-describe-function-title) requires passing the component (or function) reference as the first argument to the top-leveldescribeinstead of a string.♻️ Proposed fix
-describe('NodeSearchCategorySidebar', () => { +describe(NodeSearchCategorySidebar, () => {Based on learnings: "follow the vitest/prefer-describe-function-title rule by using describe(ComponentOrFunction, ...) instead of a string literal description when naming test suites."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchCategorySidebar.test.ts` at line 20, Replace the top-level describe string with the actual component reference: change describe('NodeSearchCategorySidebar', ...) to describe(NodeSearchCategorySidebar, ...) so the test uses the imported NodeSearchCategorySidebar symbol (ensure the component is imported at top of the file and used as the first argument to describe to satisfy vitest/prefer-describe-function-title).
26-33: Type thepropsparameter increateWrapper.The untyped
props = {}accepts any shape. UsePartial<ComponentProps<typeof NodeSearchCategorySidebar>>so the compiler catches invalid prop names/types at authoring time.♻️ Proposed fix
+import type { ComponentProps } from 'vue-component-type-helpers' - async function createWrapper(props = {}) { + async function createWrapper( + props: Partial<ComponentProps<typeof NodeSearchCategorySidebar>> = {} + ) { const wrapper = mount(NodeSearchCategorySidebar, { - props: { selectedCategory: 'most-relevant', ...props }, + props: { + selectedCategory: 'most-relevant', + ...props + } as ComponentProps<typeof NodeSearchCategorySidebar>, global: { plugins: [testI18n] } }) await nextTick() return wrapper }Based on learnings: "for test helpers like mountComponent, type the props parameter as
Partial<ComponentProps<typeof Component>>. When passing to mount, cast asComponentProps<typeof Component>."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchCategorySidebar.test.ts` around lines 26 - 33, The createWrapper helper's props parameter is untyped; change its signature to accept props: Partial<ComponentProps<typeof NodeSearchCategorySidebar>> and when calling mount(NodeSearchCategorySidebar, { props: ... }) cast the merged props to ComponentProps<typeof NodeSearchCategorySidebar> so TypeScript validates prop names/types at authoring time; update references in the createWrapper function accordingly.src/components/common/TextTicker.test.ts (2)
27-30: Addwrapper.unmount()cleanup to prevent Vue instance / event-listener leaks.No test unmounts the wrapper, which leaves dangling Vue instances and attached DOM event listeners in happy-dom between tests.
♻️ Suggested afterEach extension
+ let wrapper: ReturnType<typeof mount> + afterEach(() => { + wrapper?.unmount() vi.useRealTimers() vi.restoreAllMocks() })And assign
wrapperinstead of a localconstin eachitblock, e.g.:- const wrapper = mount(TextTicker, { ... }) + wrapper = mount(TextTicker, { ... })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/TextTicker.test.ts` around lines 27 - 30, Tests in TextTicker.test.ts are leaving Vue instances and event listeners mounted; declare a top-level wrapper variable (e.g., let wrapper) instead of using local consts in each it block, assign the result of mount()/shallowMount() to that wrapper inside tests, and extend the existing afterEach block to call wrapper?.unmount() before vi.useRealTimers() and vi.restoreAllMocks() so each test cleans up the Vue instance and DOM listeners.
107-119: Explicitly mock dimensions in the "content fits" test to avoid implicit happy-dom defaults.The test relies on
scrollWidthdefaulting to0in happy-dom (equal toclientWidth/offsetWidth), which meansscrollWidth > containerWidthevaluates to0 > 0 = false. This works today but isn't self-documenting and would silently break if the test environment's layout defaults change. Explicitly set up the "fits" scenario:✨ Suggested explicit setup
it('does not scroll when content fits', async () => { const wrapper = mount(TextTicker, { slots: { default: 'Short' } }) + const el = wrapper.element as HTMLElement + // scrollWidth <= offsetWidth → no overflow → should not scroll + mockScrollWidth(el, 50) + Object.defineProperty(el, 'offsetWidth', { value: 100, configurable: true }) await nextTick() await wrapper.trigger('mouseenter')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/TextTicker.test.ts` around lines 107 - 119, The test for TextTicker ("does not scroll when content fits") relies on implicit happy-dom defaults for element dimensions; explicitly mock the container and content sizes so scrollWidth <= containerWidth to show the "fits" case. In the test set the wrapper's mounted elements' clientWidth/offsetWidth and scrollWidth (or mock Element.prototype properties) so the content element's scrollWidth is equal to or less than the container's clientWidth (e.g., set content scrollWidth to 50 and container clientWidth/offsetWidth to 50) before triggering mouseenter and advancing timers; update references in the test to the mounted wrapper/slots and ensure rafCallbacks remains empty.src/components/searchbox/v2/NodeSearchFilterBar.test.ts (2)
33-40: Type thepropsparameter for compile-time safety.The untyped
props = {}allows passing arbitrary objects without TypeScript validation. Per project learnings, test mount helpers should usePartial<ComponentProps<typeof Component>>.♻️ Proposed fix
+import type { ComponentProps } from 'vue-component-type-helpers' ... - async function createWrapper(props = {}) { + async function createWrapper(props: Partial<ComponentProps<typeof NodeSearchFilterBar>> = {}) { const wrapper = mount(NodeSearchFilterBar, { - props, + props: props as ComponentProps<typeof NodeSearchFilterBar>, global: { plugins: [testI18n] } })Based on learnings: "In test files (e.g., any file ending with .test.ts or .test.tsx), for test helpers like mountComponent, type the props parameter as
Partial<ComponentProps<typeof Component>>. When passing to mount, cast asComponentProps<typeof Component>instead of usingRecord<string, unknown>."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchFilterBar.test.ts` around lines 33 - 40, The createWrapper test helper currently accepts an untyped props parameter; change its signature to props: Partial<ComponentProps<typeof NodeSearchFilterBar>> to enable compile-time prop validation, and when calling mount(NodeSearchFilterBar, { props, ... }) cast the props argument to ComponentProps<typeof NodeSearchFilterBar> so mount receives the correct type; update the import/use of ComponentProps if needed to satisfy TypeScript.
21-31:vi.restoreAllMocks()should precede test data setup.Calling
vi.restoreAllMocks()aftersetupTestPinia()/updateNodeDefs()is unconventional — ifsetupTestPiniainternally installs anyvi.spyOnwrappers, they would be immediately restored before any test runs, potentially breaking the setup. The conventional placement is either at the top ofbeforeEach(before setup) or inafterEach.♻️ Proposed fix
beforeEach(() => { + vi.restoreAllMocks() setupTestPinia() useNodeDefStore().updateNodeDefs([ createMockNodeDef({ name: 'ImageNode', input: { required: { image: ['IMAGE', {}] } }, output: ['IMAGE'] }) ]) - vi.restoreAllMocks() })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchFilterBar.test.ts` around lines 21 - 31, Move the vi.restoreAllMocks() call to run before any test setup (i.e., at the top of the beforeEach) so mocks are cleared prior to calling setupTestPinia() and useNodeDefStore().updateNodeDefs(); specifically, place vi.restoreAllMocks() before setupTestPinia() in the beforeEach (or alternatively move it into afterEach) to ensure any spies installed by setupTestPinia or node-def setup aren't immediately restored.src/components/searchbox/v2/NodeSearchContent.vue (1)
222-227: PrefereffectiveCategory.valueoverselectedCategory.valuein thedefaultcase for consistency.The
switchdiscriminates oneffectiveCategory.value, but thedefaultbranch referencesselectedCategory.value. While currently equivalent (thedefaultbranch is only reachable wheneffectiveCategory.value === selectedCategory.value), mixing the two in the sameswitchis a readability hazard — a future change toeffectiveCategory's derivation could silently diverge.♻️ Proposed fix
default: results = allNodes.filter( (n) => - n.category === selectedCategory.value || - n.category.startsWith(selectedCategory.value + '/') + n.category === effectiveCategory.value || + n.category.startsWith(effectiveCategory.value + '/') ) break🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchContent.vue` around lines 222 - 227, In the default case where results is computed from allNodes.filter, replace references to selectedCategory.value with effectiveCategory.value so the switch consistently uses effectiveCategory as the discriminant; update the filter predicate (the call site where n.category === selectedCategory.value || n.category.startsWith(selectedCategory.value + '/')) to use effectiveCategory.value instead, ensuring results, allNodes.filter, effectiveCategory and selectedCategory names are used to locate and change the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/shared-frontend-utils/package.json`:
- Around line 16-17: Update the dependency declaration for dompurify: add an
entry for dompurify with version ^3.3.1 to the pnpm workspace catalog
(pnpm-workspace.yaml) and change the dependency in
packages/shared-frontend-utils/package.json from "dompurify": "^3.2.5" to use
the catalog reference ("dompurify": "catalog:"); ensure the catalog key matches
other entries like "axios" so the package resolves via the workspace catalog.
In `@src/components/searchbox/v2/NodeSearchCategorySidebar.test.ts`:
- Around line 197-212: The test "should highlight selected subcategory when
expanded" currently only asserts the emitted 'update:selectedCategory' event;
update it to also assert the DOM highlight by selecting the subcategory element
after calling clickCategory (use the same wrapper and clickCategory helper) and
verifying its aria-current (or equivalent class) is set (e.g.,
expect(subcategoryElement.getAttribute('aria-current')).toBe('true')), or if you
prefer to only test emission, rename the test to reflect that it verifies event
emission (e.g., "should emit selected subcategory when expanded"). Ensure you
reference the existing helpers/wrapper (createWrapper, clickCategory) and the
emitted variable when making the change.
---
Duplicate comments:
In `@src/components/searchbox/v2/NodeSearchContent.vue`:
- Around line 77-79: No code changes required: the badge and bookmark visibility
props are already correctly derived from effectiveCategory (see the bindings
:show-source-badge and :hide-bookmark-icon on the component) so leave the logic
as-is; confirm that effectiveCategory is the computed property used elsewhere
(not selectedCategory) and ensure no other places still reference
selectedCategory for badge/icon visibility.
---
Nitpick comments:
In `@src/components/common/TextTicker.test.ts`:
- Around line 27-30: Tests in TextTicker.test.ts are leaving Vue instances and
event listeners mounted; declare a top-level wrapper variable (e.g., let
wrapper) instead of using local consts in each it block, assign the result of
mount()/shallowMount() to that wrapper inside tests, and extend the existing
afterEach block to call wrapper?.unmount() before vi.useRealTimers() and
vi.restoreAllMocks() so each test cleans up the Vue instance and DOM listeners.
- Around line 107-119: The test for TextTicker ("does not scroll when content
fits") relies on implicit happy-dom defaults for element dimensions; explicitly
mock the container and content sizes so scrollWidth <= containerWidth to show
the "fits" case. In the test set the wrapper's mounted elements'
clientWidth/offsetWidth and scrollWidth (or mock Element.prototype properties)
so the content element's scrollWidth is equal to or less than the container's
clientWidth (e.g., set content scrollWidth to 50 and container
clientWidth/offsetWidth to 50) before triggering mouseenter and advancing
timers; update references in the test to the mounted wrapper/slots and ensure
rafCallbacks remains empty.
In `@src/components/searchbox/v2/NodeSearchCategorySidebar.test.ts`:
- Line 20: Replace the top-level describe string with the actual component
reference: change describe('NodeSearchCategorySidebar', ...) to
describe(NodeSearchCategorySidebar, ...) so the test uses the imported
NodeSearchCategorySidebar symbol (ensure the component is imported at top of the
file and used as the first argument to describe to satisfy
vitest/prefer-describe-function-title).
- Around line 26-33: The createWrapper helper's props parameter is untyped;
change its signature to accept props: Partial<ComponentProps<typeof
NodeSearchCategorySidebar>> and when calling mount(NodeSearchCategorySidebar, {
props: ... }) cast the merged props to ComponentProps<typeof
NodeSearchCategorySidebar> so TypeScript validates prop names/types at authoring
time; update references in the createWrapper function accordingly.
In `@src/components/searchbox/v2/NodeSearchContent.vue`:
- Around line 222-227: In the default case where results is computed from
allNodes.filter, replace references to selectedCategory.value with
effectiveCategory.value so the switch consistently uses effectiveCategory as the
discriminant; update the filter predicate (the call site where n.category ===
selectedCategory.value || n.category.startsWith(selectedCategory.value + '/'))
to use effectiveCategory.value instead, ensuring results, allNodes.filter,
effectiveCategory and selectedCategory names are used to locate and change the
code.
In `@src/components/searchbox/v2/NodeSearchFilterBar.test.ts`:
- Around line 33-40: The createWrapper test helper currently accepts an untyped
props parameter; change its signature to props: Partial<ComponentProps<typeof
NodeSearchFilterBar>> to enable compile-time prop validation, and when calling
mount(NodeSearchFilterBar, { props, ... }) cast the props argument to
ComponentProps<typeof NodeSearchFilterBar> so mount receives the correct type;
update the import/use of ComponentProps if needed to satisfy TypeScript.
- Around line 21-31: Move the vi.restoreAllMocks() call to run before any test
setup (i.e., at the top of the beforeEach) so mocks are cleared prior to calling
setupTestPinia() and useNodeDefStore().updateNodeDefs(); specifically, place
vi.restoreAllMocks() before setupTestPinia() in the beforeEach (or alternatively
move it into afterEach) to ensure any spies installed by setupTestPinia or
node-def setup aren't immediately restored.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts (1)
931-934: Extract the duplicatesetSettingcall to a nestedbeforeEachin theRelease actionsblock.Both tests inside
test.describe('Release actions (Shift-drop)')independently setComfy.NodeSearchBoxImplto'v1 (legacy)'. Any new test added to this block would silently run under the v2 default unless the author remembers to add the setting manually.♻️ Proposed refactor: hoist to a nested `beforeEach`
test.describe('Release actions (Shift-drop)', () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.settings.setSetting('Comfy.NodeSearchBoxImpl', 'v1 (legacy)') + }) + test('Context menu opens and endpoint is pinned on Shift-drop', async ({ comfyPage, comfyMouse }) => { await comfyPage.settings.setSetting( 'Comfy.LinkRelease.ActionShift', 'context menu' ) // no setSetting for NodeSearchBoxImpl needed here ... }) test('Context menu -> Search pre-filters by link type and connects after selection', async ({ comfyPage, comfyMouse }) => { await comfyPage.settings.setSetting( 'Comfy.LinkRelease.ActionShift', 'context menu' ) - await comfyPage.settings.setSetting( - 'Comfy.NodeSearchBoxImpl', - 'v1 (legacy)' - ) ... }) test('Search box opens on Shift-drop and connects after selection', async ({ comfyPage, comfyMouse }) => { await comfyPage.settings.setSetting( 'Comfy.LinkRelease.ActionShift', 'search box' ) - await comfyPage.settings.setSetting( - 'Comfy.NodeSearchBoxImpl', - 'v1 (legacy)' - ) ... }) })Also applies to: 1000-1003
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts` around lines 931 - 934, The two tests inside the test.describe('Release actions (Shift-drop)') block both call comfyPage.settings.setSetting('Comfy.NodeSearchBoxImpl','v1 (legacy)'); extract that duplicate call into a nested beforeEach inside the same describe so the setting is applied for every test in that block; remove the duplicate setSetting calls from the individual tests and ensure the nested beforeEach calls comfyPage.settings.setSetting with the same key and value before each test runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts`:
- Around line 1058-1062: The test mutates app state by calling
comfyPage.settings.setSetting('Comfy.NodeSearchBoxImpl', 'v1 (legacy)') and
never resets it, risking test-order dependency; update the outer beforeEach to
explicitly reset this setting back to the default (e.g., call
comfyPage.settings.setSetting('Comfy.NodeSearchBoxImpl', 'v2') or the app's
default) so that loadWorkflow() and subsequent tests see the expected setting,
ensuring the reset runs before each test that uses loadWorkflow() or comfyPage.
---
Nitpick comments:
In `@browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts`:
- Around line 931-934: The two tests inside the test.describe('Release actions
(Shift-drop)') block both call
comfyPage.settings.setSetting('Comfy.NodeSearchBoxImpl','v1 (legacy)'); extract
that duplicate call into a nested beforeEach inside the same describe so the
setting is applied for every test in that block; remove the duplicate setSetting
calls from the individual tests and ensure the nested beforeEach calls
comfyPage.settings.setSetting with the same key and value before each test runs.
Comment on lines
+1058
to
+1062
| await comfyPage.settings.setSetting( | ||
| 'Comfy.NodeSearchBoxImpl', | ||
| 'v1 (legacy)' | ||
| ) | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Check loadWorkflow implementation
echo "=== loadWorkflow implementation ==="
rg -n "loadWorkflow" browser_tests/fixtures/ComfyPage.ts -A 20
# 2. Check the test file structure around lines 102-109 and 1058-1062
echo -e "\n=== Test file structure (beforeEach and relevant test) ==="
sed -n '100,115p' browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
echo -e "\n=== Settings setter call at line 1058 ==="
sed -n '1055,1065p' browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
# 3. Search for other setSetting calls in the test file to understand patterns
echo -e "\n=== All setSetting calls in test file ==="
rg -n "setSetting" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
# 4. Check ComfyPage setup method to understand what it does
echo -e "\n=== ComfyPage setup method ==="
rg -n "setup()" browser_tests/fixtures/ComfyPage.ts -A 15 | head -40Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2835
🏁 Script executed:
#!/bin/bash
# The first search didn't find loadWorkflow with -A 20. Let me search more broadly
echo "=== Searching for loadWorkflow definition ==="
rg -n "loadWorkflow\(" browser_tests/fixtures/ -A 20 | head -60
echo -e "\n=== Check if loadWorkflow is in WorkflowFixture ==="
fd "Workflow" browser_tests/fixtures/ --type f
echo -e "\n=== Check what workflow object contains ==="
rg -n "workflow\." browser_tests/fixtures/ComfyPage.ts | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2099
🏁 Script executed:
#!/bin/bash
# Now let me check what happens during workflow upload and if settings persist
echo "=== Check if workflowUploadInput triggers page reload ==="
rg -n "workflowUploadInput" browser_tests/fixtures/ComfyPage.ts -A 5 -B 2
echo -e "\n=== Check the setup method implementation (continued) ==="
rg -n "async setup" browser_tests/fixtures/ComfyPage.ts -A 50 | head -80
echo -e "\n=== Check if clearStorage clears settings ==="
rg -n "clearStorage" browser_tests/fixtures/ComfyPage.ts -B 2 -A 10
echo -e "\n=== Check localStorage/settings handling ==="
rg -n "localStorage" browser_tests/fixtures/ -lRepository: Comfy-Org/ComfyUI_frontend
Length of output: 4247
🏁 Script executed:
#!/bin/bash
# Check the structure of tests after line 1058 to see if any rely on v2 default
echo "=== Tests after the 'Dragging from subgraph input' test (line 1058) ==="
rg -n "test\(" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts | tail -20
echo -e "\n=== Check the full test name and location ==="
sed -n '1050,1080p' browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
echo -e "\n=== Check if any tests after line 1058 are still in the same describe block ==="
tail -50 browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 4504
🏁 Script executed:
#!/bin/bash
# Check what's after the closing braces at the end of the file
echo "=== Check end of file structure ==="
wc -l browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
echo -e "\n=== Last 20 lines of file ==="
tail -20 browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
echo -e "\n=== Check if there are more test() calls after line 1054 ==="
rg -n "^ test\(" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts | tail -5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1268
🏁 Script executed:
#!/bin/bash
# Check if there are other describe blocks after this one
echo "=== Check for other describe blocks ==="
rg -n "test\.describe\|describe\(" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
echo -e "\n=== Verify the closing structure around line 1108-1109 ==="
sed -n '1100,1109p' browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
echo -e "\n=== Check if NewBlankWorkflow command resets settings/page state ==="
rg -n "NewBlankWorkflow" browser_tests/ -l
echo -e "\n=== Check if page reload happens in other tests ==="
rg -n "goto\(\)" browser_tests/fixtures/ComfyPage.ts | head -10Repository: Comfy-Org/ComfyUI_frontend
Length of output: 964
🏁 Script executed:
#!/bin/bash
# Check if tests share the same page context across describe blocks
echo "=== Check test file structure and page fixture scope ==="
head -20 browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
echo -e "\n=== Check how comfyPage fixture is created ==="
rg -n "comfyPage" browser_tests/fixtures/index.ts -A 5 | head -30
echo -e "\n=== Confirm: Is this the last test in the describe block? ==="
sed -n '1054,1109p' browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts | grep -n "^})"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1083
🏁 Script executed:
#!/bin/bash
# Check fixture setup
echo "=== Check ComfyPage fixture setup ==="
ls -la browser_tests/fixtures/*.ts | head -10
echo -e "\n=== Check how the page is managed in tests ==="
rg -n "comfyPageFixture\|test\(" browser_tests/fixtures/ComfyPage.ts | head -20
echo -e "\n=== Key question: Does NewBlankWorkflow clear settings or localStorage? ==="
rg -n "NewBlankWorkflow\|Comfy\.NewBlankWorkflow" browser_tests -l
echo -e "\n=== Check if each test gets a fresh page or shares state ==="
rg -n "use\(" browser_tests/fixtures/ComfyPage.ts | head -10Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1008
🏁 Script executed:
#!/bin/bash
# Verify this is truly the last test in the file and the last in the describe block
echo "=== Verify test count and structure ==="
rg -c "^ test\(" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
echo -e "\n=== Show all top-level test() calls with line numbers ==="
rg -n "^ test\(" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
echo -e "\n=== Confirm structure: last test ends before final closing braces ==="
sed -n '1054,1109p' browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts | tail -10
echo -e "\n=== Verify the describe block boundaries ==="
rg -n "test\.describe\|^}\)" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts | tail -3Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1852
🏁 Script executed:
#!/bin/bash
# Final verification: confirm that settings use localStorage and persist across navigations
echo "=== Check how Comfy.NodeSearchBoxImpl setting is stored/retrieved ==="
rg -n "Comfy\.NodeSearchBoxImpl" browser_tests -l
echo -e "\n=== Verify setSetting method stores in localStorage ==="
rg -n "setSetting" browser_tests/fixtures/ -l
echo -e "\n=== Quick check: Does any test file have cleanup/teardown after tests? ==="
rg -n "afterEach\|teardown" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 795
🏁 Script executed:
#!/bin/bash
# Check how setSetting is implemented - does it use localStorage?
echo "=== SettingsHelper implementation ==="
cat browser_tests/fixtures/helpers/SettingsHelper.ts
echo -e "\n=== Check what the default value of NodeSearchBoxImpl should be ==="
rg -n "NodeSearchBoxImpl" browser_tests/tests/nodeSearchBox.spec.ts | head -5
echo -e "\n=== Check if there are nested describe blocks within the main one ==="
sed -n '100,120p' browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1815
Settings leak: test sets NodeSearchBoxImpl to 'v1 (legacy)' without resetting in beforeEach.
This test is currently the last one in the describe block, so there's no immediate impact. However, since loadWorkflow() doesn't reload the page (it only uploads JSON and calls nextFrame()), the setting persists in app state. If tests are added after this one, they will silently inherit 'v1 (legacy)' instead of the default 'v2'.
Add an explicit reset to the outer beforeEach to prevent accidental test-order dependency:
Proposed safeguard
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top')
await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', true)
+ await comfyPage.settings.setSetting('Comfy.NodeSearchBoxImpl', 'v2')
await comfyPage.workflow.loadWorkflow('vueNodes/simple-triple')
await comfyPage.vueNodes.waitForNodes()
await fitToViewInstant(comfyPage)
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts`
around lines 1058 - 1062, The test mutates app state by calling
comfyPage.settings.setSetting('Comfy.NodeSearchBoxImpl', 'v1 (legacy)') and
never resets it, risking test-order dependency; update the outer beforeEach to
explicitly reset this setting back to the default (e.g., call
comfyPage.settings.setSetting('Comfy.NodeSearchBoxImpl', 'v2') or the app's
default) so that loadWorkflow() and subsequent tests see the expected setting,
ensuring the reset runs before each test that uses loadWorkflow() or comfyPage.
- Add three-tab structure (Essential, All, Custom) using Reka UI Tabs - Implement TreeExplorerV2 with virtualized tree using TreeRoot/TreeVirtualizer - Add node hover preview with teleport to show NodePreview component - Implement context menu for toggling favorites on nodes - Add search functionality that auto-expands matching folders - Create panel components: EssentialNodesPanel, AllNodesPanel, CustomNodesPanel - Add 'Open Manager' button in CustomNodesPanel - Use custom icons: comfy--node for nodes, ph--folder-fill for folders Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019c1ee5-bb3c-70fb-8c24-33966d8dbef8
- Add main_category field to node definitions in api.ts - Add main_category to ComfyNodeDefImpl class - Group nodes by main_category in 'all' and 'custom' tabs - Format category names from snake_case to Title Case Amp-Thread-ID: https://ampcode.com/threads/T-019c1ee5-bb3c-70fb-8c24-33966d8dbef8 Co-authored-by: Amp <amp@ampcode.com>
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
christian-byrne
approved these changes
Feb 20, 2026
huntcsg
pushed a commit
that referenced
this pull request
Feb 21, 2026
## Summary Redesigned node search with categories ## Changes - **What**: Adds a v2 search component, leaving the existing implementation untouched - It also brings onboard the incomplete node library & preview changes, disabled and behind a hidden setting - **Breaking**: Changes the 'default' value of the node search setting to v2, adding v1 (legacy) as an option ## Screenshots (if applicable) https://github.com/user-attachments/assets/2ab797df-58f0-48e8-8b20-2a1809e3735f ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8987-V2-Node-Search-hidden-Node-Library-changes-30c6d73d36508160902bcb92553f147c) by [Unito](https://www.unito.io) --------- Co-authored-by: Yourz <crazilou@vip.qq.com> Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Christian Byrne <cbyrne@comfy.org>
christian-byrne
added a commit
that referenced
this pull request
Feb 22, 2026
- Category sidebar: nested expanded subfolders wrap in bg-secondary-background container - Chevrons appear on sidebar hover for categories with children - Filter chips: three visual states (active/applied/default) with applied count - Preview card pricing badge truncation for overflow - Pass appliedFilters prop to NodeSearchFilterBar - Fix paddingLeft to preserve depth hierarchy when chevrons are visible - Deduplicate button template in NodeSearchCategoryTreeNode - Restore draftTypes.ts removed by merge conflict with #8993/#8519 Amp-Thread-ID: https://ampcode.com/threads/T-019c8410-57a4-7432-85f0-44ffb23788e2
christian-byrne
added a commit
that referenced
this pull request
Feb 22, 2026
…n improvements - Reorder tabs to All → Essentials → Blueprints, rename Custom to Blueprints - Add uppercase section headers in All and Blueprints tabs - Move chevron to left of folder icon in TreeExplorerV2Node - Add bookmark button on hover for node rows - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Update sort labels to Categorized/A-Z with label-left/check-right layout - Hide section headers in A-Z sort mode - Expand search dialog filter chips from 3 to 6 - Add Recents and source categories to search sidebar - Remove Filter by label from search dialog - Pull foundation V2 components from merged PR #8548 Depends on: #8987 (V2 Node Search), #8548 (NodeLibrarySidebarTabV2) Amp-Thread-ID: https://ampcode.com/threads/T-019c8410-45ce-7777-8d4d-1d947c742b61
christian-byrne
added a commit
that referenced
this pull request
Feb 22, 2026
- Category sidebar: nested expanded subfolders wrap in bg-secondary-background container - Chevrons appear on sidebar hover for categories with children - Filter chips: three visual states (active/applied/default) with applied count - Preview card pricing badge truncation for overflow - Pass appliedFilters prop to NodeSearchFilterBar - Fix paddingLeft to preserve depth hierarchy when chevrons are visible - Deduplicate button template in NodeSearchCategoryTreeNode - Restore draftTypes.ts removed by merge conflict with #8993/#8519 Amp-Thread-ID: https://ampcode.com/threads/T-019c8410-57a4-7432-85f0-44ffb23788e2
christian-byrne
added a commit
that referenced
this pull request
Feb 22, 2026
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c8410-45ce-7777-8d4d-1d947c742b61
christian-byrne
added a commit
that referenced
this pull request
Feb 23, 2026
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-7093-7315-a498-a195d3e67fdf
christian-byrne
added a commit
that referenced
this pull request
Feb 23, 2026
- Category sidebar: nested expanded subfolders wrap in bg-secondary-background container - Chevrons appear on sidebar hover for categories with children - Filter chips: three visual states (active/applied/default) with applied count - Preview card pricing badge truncation for overflow - Pass appliedFilters prop to NodeSearchFilterBar - Fix paddingLeft to preserve depth hierarchy when chevrons are visible - Deduplicate button template in NodeSearchCategoryTreeNode - Restore draftTypes.ts removed by merge conflict with #8993/#8519 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-aa28-7290-bdf0-ea5f86aacde3
christian-byrne
added a commit
that referenced
this pull request
Feb 23, 2026
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-7093-7315-a498-a195d3e67fdf
Yourz
pushed a commit
that referenced
this pull request
Feb 25, 2026
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-7093-7315-a498-a195d3e67fdf
Yourz
pushed a commit
that referenced
this pull request
Feb 26, 2026
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-7093-7315-a498-a195d3e67fdf
Yourz
pushed a commit
that referenced
this pull request
Feb 26, 2026
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-7093-7315-a498-a195d3e67fdf
Yourz
pushed a commit
that referenced
this pull request
Feb 27, 2026
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-7093-7315-a498-a195d3e67fdf
Yourz
pushed a commit
that referenced
this pull request
Feb 27, 2026
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-7093-7315-a498-a195d3e67fdf
AustinMroz
added a commit
that referenced
this pull request
Feb 28, 2026
In adding an essentials cateogory for nodes, #8987 introduced a regression where core nodes which are also essential are marked as being from a `nodes` custom node instead of being marked core. Since the essentials designation should pre-empt core and custom nodes can choose to mark themself as essential, the getter for `isCoreNode` is updated to instead repeat the existing check for if a node is core. | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/f1b8bf80-d072-409a-a0f9-4837e1d11767" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/14ff525b-9833-4e73-888f-791aff6cf531"/>| ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9287-Fix-essentials-nodes-not-being-marked-core-3146d73d365081fca2a0f8bdc2baf01a) by [Unito](https://www.unito.io)
Yourz
pushed a commit
that referenced
this pull request
Feb 28, 2026
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-7093-7315-a498-a195d3e67fdf
Yourz
pushed a commit
that referenced
this pull request
Feb 28, 2026
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-7093-7315-a498-a195d3e67fdf
Yourz
pushed a commit
that referenced
this pull request
Feb 28, 2026
…ents Sidebar: - Reorder tabs to All/Essentials/Blueprints, rename Custom to Blueprints - Add uppercase section headers, hide in A-Z sort mode - Move chevron to left of folder icon, add bookmark button on hover - Add filter dropdown (Blueprints, Partner Nodes, Comfy Nodes, Extensions) - Sort labels: Categorized/A-Z with label-left/check-right layout Search dialog: - Expand filter chips from 3 to 6 - Add Recents and source categories to sidebar - Remove Filter by label, add category filtering Depends on: #8987, #8548 Amp-Thread-ID: https://ampcode.com/threads/T-019c87f4-7093-7315-a498-a195d3e67fdf
Yourz
added a commit
that referenced
this pull request
Feb 28, 2026
## Summary Implement 11 Figma design discrepancies for the Node Library sidebar and V2 Node Search dialog, aligning the UI with the [Toolbox Figma design](https://www.figma.com/design/xMFxCziXJe6Denz4dpDGTq/Toolbox?node-id=2074-21394&m=dev). ## Changes - **What**: Sidebar: reorder tabs (All/Essentials/Blueprints), rename Custom→Blueprints, uppercase section headers, chevron-left of folder icon, bookmark-on-hover for node rows, filter dropdown with checkbox items, sort labels (Categorized/A-Z) with label-left/check-right layout, hide section headers in A-Z mode. Search dialog: expand filter chips from 3→6, add Recents and source categories to sidebar, remove "Filter by" label. Pull foundation V2 components from merged PR #8548. - **Dependencies**: Depends on #8987 (V2 Node Search) and #8548 (NodeLibrarySidebarTabV2) ## Review Focus - Filter dropdown (`filterOptions`) is UI-scaffolded but not yet wired to filtering logic (pending V2 integration) - "Recents" category currently returns frequency-based results as placeholder until a usage-tracking store is implemented - Pre-existing type errors from V2 PR dependencies not in the base commit (SearchBoxV2, usePerTabState, TextTicker, getProviderIcon, getLinkTypeColor, SidebarContainerKey) are expected and will resolve when rebased onto main after parent PRs land ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9085-feat-Node-Library-sidebar-and-V2-Search-dialog-Figma-design-improvements-30f6d73d36508175bf72d716f5904476) by [Unito](https://www.unito.io) --------- Co-authored-by: Yourz <crazilou@vip.qq.com> Co-authored-by: github-actions <github-actions@github.com>
DrJKL
pushed a commit
that referenced
this pull request
Feb 28, 2026
In adding an essentials cateogory for nodes, #8987 introduced a regression where core nodes which are also essential are marked as being from a `nodes` custom node instead of being marked core. Since the essentials designation should pre-empt core and custom nodes can choose to mark themself as essential, the getter for `isCoreNode` is updated to instead repeat the existing check for if a node is core. | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/f1b8bf80-d072-409a-a0f9-4837e1d11767" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/14ff525b-9833-4e73-888f-791aff6cf531"/>| ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9287-Fix-essentials-nodes-not-being-marked-core-3146d73d365081fca2a0f8bdc2baf01a) by [Unito](https://www.unito.io)
DrJKL
pushed a commit
that referenced
this pull request
Feb 28, 2026
## Summary Implement 11 Figma design discrepancies for the Node Library sidebar and V2 Node Search dialog, aligning the UI with the [Toolbox Figma design](https://www.figma.com/design/xMFxCziXJe6Denz4dpDGTq/Toolbox?node-id=2074-21394&m=dev). ## Changes - **What**: Sidebar: reorder tabs (All/Essentials/Blueprints), rename Custom→Blueprints, uppercase section headers, chevron-left of folder icon, bookmark-on-hover for node rows, filter dropdown with checkbox items, sort labels (Categorized/A-Z) with label-left/check-right layout, hide section headers in A-Z mode. Search dialog: expand filter chips from 3→6, add Recents and source categories to sidebar, remove "Filter by" label. Pull foundation V2 components from merged PR #8548. - **Dependencies**: Depends on #8987 (V2 Node Search) and #8548 (NodeLibrarySidebarTabV2) ## Review Focus - Filter dropdown (`filterOptions`) is UI-scaffolded but not yet wired to filtering logic (pending V2 integration) - "Recents" category currently returns frequency-based results as placeholder until a usage-tracking store is implemented - Pre-existing type errors from V2 PR dependencies not in the base commit (SearchBoxV2, usePerTabState, TextTicker, getProviderIcon, getLinkTypeColor, SidebarContainerKey) are expected and will resolve when rebased onto main after parent PRs land ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9085-feat-Node-Library-sidebar-and-V2-Search-dialog-Figma-design-improvements-30f6d73d36508175bf72d716f5904476) by [Unito](https://www.unito.io) --------- Co-authored-by: Yourz <crazilou@vip.qq.com> Co-authored-by: github-actions <github-actions@github.com>
christian-byrne
pushed a commit
that referenced
this pull request
Mar 8, 2026
In adding an essentials cateogory for nodes, #8987 introduced a regression where core nodes which are also essential are marked as being from a `nodes` custom node instead of being marked core. Since the essentials designation should pre-empt core and custom nodes can choose to mark themself as essential, the getter for `isCoreNode` is updated to instead repeat the existing check for if a node is core. | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/f1b8bf80-d072-409a-a0f9-4837e1d11767" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/14ff525b-9833-4e73-888f-791aff6cf531"/>| ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9287-Fix-essentials-nodes-not-being-marked-core-3146d73d365081fca2a0f8bdc2baf01a) by [Unito](https://www.unito.io)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Redesigned node search with categories
Changes
Screenshots (if applicable)
node.search.mp4
┆Issue is synchronized with this Notion page by Unito