feat: add node replacement UI to Errors Tab#9253
Merged
christian-byrne merged 7 commits intomainfrom Feb 27, 2026
Merged
Conversation
Integrate the existing node replacement functionality into the Errors Tab, allowing users to replace missing nodes directly from the side panel without opening the modal dialog. New components: - SwapNodesCard: container with guidance label and grouped rows - SwapNodeGroupRow: per-type replacement row with expand/collapse, node instance list, locate button, and replace action Bug fixes discovered during implementation: - Fix stale canvas rendering after replacement by calling onNodeAdded to refresh VueNodeData (bypassed by replaceWithMapping) - Guard initializeVueNodeLayout against duplicate layout creation - Fix missing node list being overwritten by incomplete server 400 response — replaced with full graph rescan via useMissingNodeScan - Add removeMissingNodesByType to prune replaced types from error list Cleanup: - Remove dead code: buildMissingNodeHint, createMissingNodeTypeFromError
- Add error toast in replaceNodesInPlace for user-visible failure feedback, returning empty array on error instead of throwing - Guard removeMissingNodesByType behind replacement success check (replaced.length > 0) to prevent stale error list updates - Sort buildMissingNodeGroups by priority for deterministic UI order (Swap Nodes 0 → Missing Node Packs 1 → Execution Errors) - Add aux_id fallback and cnr_id precedence tests for getCnrIdFromNode - Split replaceAllWarning from replaceWarning to fix i18n key mismatch between TabErrors tooltip and MissingNodesContent dialog
jaeone94
requested review from
a team,
christian-byrne,
ltdrdata and
viva-jinyi
as code owners
dosubot
bot
added
the
size:XL
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 547 passed, 0 failed · 5 flaky📊 Browser Reports
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds client-side missing-node detection and a new "swap_nodes" replacement UI: new SwapNodesCard/SwapNodeGroupRow components, grouping logic for swap_nodes, full-graph rescan to surface missing types, node-replacement flow updates, and a store action to remove replaced missing-node entries. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant TabErrors as TabErrors.vue
participant Scanner as useMissingNodeScan
participant Graph as LiteGraph/Graph
participant Store as executionErrorStore
participant Replacement as useNodeReplacement
participant UI as SwapNodesCard/SwapNodeGroupRow
User->>TabErrors: open errors tab
TabErrors->>Scanner: rescanAndSurfaceMissingNodes(rootGraph)
Scanner->>Graph: iterate nodes, check registered types
Scanner->>Store: surfaceMissingNodes(missingTypes)
Store->>TabErrors: expose swapNodeGroups
TabErrors->>UI: render SwapNodesCard with groups
User->>UI: click "Replace Node" or "Replace All"
UI->>TabErrors: emit locate-node or call handleReplaceAll
TabErrors->>Replacement: replaceNodesInPlace(nodeTypes)
Replacement->>Graph: perform replacements, call onNodeAdded
TabErrors->>Store: removeMissingNodesByType(replaced types)
Store-->>TabErrors: updated missingNodes state
TabErrors-->>User: UI updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
📦 Bundle: 4.44 MB gzip 🔴 +1.16 kBDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.01 MB (baseline 1 MB) • 🔴 +8.9 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 435 kB (baseline 435 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: 5 added / 5 removed Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47.1 kB (baseline 47.1 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.55 MB (baseline 2.55 MB) • 🔴 +507 BStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 12 added / 12 removed Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.77 MB (baseline 7.77 MB) • 🔴 +355 BBundles that do not match a named category
Status: 56 added / 56 removed |
⚡ Performance Report
Raw data{
"timestamp": "2026-02-26T15:18:43.382Z",
"gitSha": "a93213a3c8c443ea7d98177663ce6c5333c1f071",
"branch": "feat/replace-missing-node-from-error-tab",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2030.4490000000044,
"styleRecalcs": 122,
"styleRecalcDurationMs": 27.434,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 501.04400000000004,
"heapDeltaBytes": -3788508
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2012.3250000000041,
"styleRecalcs": 182,
"styleRecalcDurationMs": 62.727000000000004,
"layouts": 12,
"layoutDurationMs": 3.666,
"taskDurationMs": 990.298,
"heapDeltaBytes": -2805248
},
{
"name": "dom-widget-clipping",
"durationMs": 647.0950000000073,
"styleRecalcs": 47,
"styleRecalcDurationMs": 16.111,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 412.24100000000004,
"heapDeltaBytes": 6424456
}
]
} |
![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: 3
🧹 Nitpick comments (3)
src/workbench/extensions/manager/utils/missingNodeErrorUtil.test.ts (2)
39-41: Add fallback coverage whencnr_idis invalid butaux_idis valid.This edge case protects the intended fallback order from regressions.
➕ Suggested test
it('returns undefined when cnr_id is not a string', () => { expect(getCnrIdFromProperties({ cnr_id: 123 })).toBeUndefined() }) + + it('falls back to aux_id when cnr_id is not a string', () => { + expect( + getCnrIdFromProperties({ cnr_id: 123, aux_id: 'my-aux-pack' }) + ).toBe('my-aux-pack') + })As per coding guidelines: "aim for behavioral coverage of critical and new features."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workbench/extensions/manager/utils/missingNodeErrorUtil.test.ts` around lines 39 - 41, Add a test covering the fallback behavior of getCnrIdFromProperties when cnr_id is invalid but aux_id is valid: create a case where cnr_id is a non-string (e.g., number or null) and aux_id is a valid string and assert the function returns the aux_id string; update the existing test suite (missingNodeErrorUtil.test.ts) to include this scenario so the fallback order (cnr_id first, aux_id second) is guarded against regressions.
46-48: Avoid repeatedas unknown as LGraphNodein test fixtures.Prefer a small shape-checked helper to reduce unsafe assertions and keep fixtures easier to maintain.
♻️ Suggested refactor
+function createNodeWithProperties( + properties: Record<string, unknown> +): LGraphNode { + const node = { properties } satisfies Pick<LGraphNode, 'properties'> + return node as LGraphNode +} + describe('getCnrIdFromNode', () => { it('returns cnr_id from node properties', () => { - const node = { - properties: { cnr_id: 'node-pack' } - } as unknown as LGraphNode + const node = createNodeWithProperties({ cnr_id: 'node-pack' }) expect(getCnrIdFromNode(node)).toBe('node-pack') }) it('returns aux_id when cnr_id is absent', () => { - const node = { - properties: { aux_id: 'node-aux-pack' } - } as unknown as LGraphNode + const node = createNodeWithProperties({ aux_id: 'node-aux-pack' }) expect(getCnrIdFromNode(node)).toBe('node-aux-pack') }) it('prefers cnr_id over aux_id in node properties', () => { - const node = { - properties: { cnr_id: 'primary', aux_id: 'secondary' } - } as unknown as LGraphNode + const node = createNodeWithProperties({ + cnr_id: 'primary', + aux_id: 'secondary' + }) expect(getCnrIdFromNode(node)).toBe('primary') }) it('returns undefined when node has no cnr_id or aux_id', () => { - const node = { properties: {} } as unknown as LGraphNode + const node = createNodeWithProperties({}) expect(getCnrIdFromNode(node)).toBeUndefined() }) })Based on learnings: "In test files matching **/*.test.ts under src, when creating test helper functions that construct mock objects implementing an interface (e.g., AssetItem), prefer using satisfies InterfaceType for shape validation instead of type assertions like as Partial as InterfaceType or as any."
Also applies to: 53-55, 60-62, 67-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workbench/extensions/manager/utils/missingNodeErrorUtil.test.ts` around lines 46 - 48, Replace the repeated unsafe casts for LGraphNode test fixtures by adding a small factory helper (e.g., makeNode or createTestNode) in the test file that returns a properly shaped object using TypeScript's "satisfies LGraphNode" to get compile-time shape checking; update the existing fixtures (the const node and other occurrences at the noted locations) to call this helper with the desired properties instead of using "as unknown as LGraphNode", ensuring the helper includes any minimal required fields of LGraphNode so the tests remain valid and type-checked.src/components/rightSidePanel/errors/SwapNodesCard.vue (1)
30-33: Prefer Vue 3.5 reactive props destructuring here.Use destructured
definePropsto align with the project's standard for new Vue SFCs.♻️ Proposed refactor
-const props = defineProps<{ +const { swapNodeGroups, showNodeIdBadge } = defineProps<{ swapNodeGroups: SwapNodeGroup[] showNodeIdBadge: boolean }>()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/rightSidePanel/errors/SwapNodesCard.vue` around lines 30 - 33, Replace the generic props object with Vue 3.5-style destructured reactive props: remove the const props = defineProps<...>() block and instead destructure the props directly from defineProps so you get swapNodeGroups and showNodeIdBadge as standalone reactive bindings; update any references to props.swapNodeGroups and props.showNodeIdBadge to use the destructured names (SwapNodesCard.vue: replace usages referencing props, and adjust defineProps call around swapNodeGroups and showNodeIdBadge).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/rightSidePanel/errors/SwapNodeGroupRow.vue`:
- Around line 144-149: handleReplaceNode currently calls
replaceNodesInPlace(props.group.nodeTypes) and only clears executionErrorStore
when replacements occur; add user feedback for the case where
replaceNodesInPlace returns an empty array by showing a toast informing the user
that no replaceable nodes of the selected type were found. Modify
handleReplaceNode to detect when replaced.length === 0 and invoke the same toast
mechanism used in useNodeReplacement.ts (the success/error toasts around
replaceNodesInPlace) so the message style is consistent; keep the existing
executionErrorStore.removeMissingNodesByType([props.group.type]) behavior when
replacements happen.
In `@src/components/rightSidePanel/errors/useErrorGroups.ts`:
- Around line 536-542: Replace the hardcoded English title in the useErrorGroups
function where groups.push({...}) is adding the 'swap_nodes' group: use the
provided t translation function (e.g. title: t('swapNodesTitle')) instead of the
literal 'Swap Nodes', and add the corresponding key "swapNodesTitle": "Swap
Nodes" to src/locales/en/main.json so the UI text uses vue-i18n consistently.
In `@src/platform/nodeReplacement/useNodeReplacement.ts`:
- Around line 286-295: The catch block in useNodeReplacement currently always
returns [] which discards any partial replacements; declare the array that
collects successful replacements (e.g., replacedNodes or replacements) outside
the try in useNodeReplacement, push into that array as each node is replaced
inside the try, and in the catch return that array (not an empty array) so
callers see partial results; keep the error toast/logging behavior but change
the return in the catch to return the external replacements variable.
---
Nitpick comments:
In `@src/components/rightSidePanel/errors/SwapNodesCard.vue`:
- Around line 30-33: Replace the generic props object with Vue 3.5-style
destructured reactive props: remove the const props = defineProps<...>() block
and instead destructure the props directly from defineProps so you get
swapNodeGroups and showNodeIdBadge as standalone reactive bindings; update any
references to props.swapNodeGroups and props.showNodeIdBadge to use the
destructured names (SwapNodesCard.vue: replace usages referencing props, and
adjust defineProps call around swapNodeGroups and showNodeIdBadge).
In `@src/workbench/extensions/manager/utils/missingNodeErrorUtil.test.ts`:
- Around line 39-41: Add a test covering the fallback behavior of
getCnrIdFromProperties when cnr_id is invalid but aux_id is valid: create a case
where cnr_id is a non-string (e.g., number or null) and aux_id is a valid string
and assert the function returns the aux_id string; update the existing test
suite (missingNodeErrorUtil.test.ts) to include this scenario so the fallback
order (cnr_id first, aux_id second) is guarded against regressions.
- Around line 46-48: Replace the repeated unsafe casts for LGraphNode test
fixtures by adding a small factory helper (e.g., makeNode or createTestNode) in
the test file that returns a properly shaped object using TypeScript's
"satisfies LGraphNode" to get compile-time shape checking; update the existing
fixtures (the const node and other occurrences at the noted locations) to call
this helper with the desired properties instead of using "as unknown as
LGraphNode", ensuring the helper includes any minimal required fields of
LGraphNode so the tests remain valid and type-checked.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/components/rightSidePanel/errors/SwapNodeGroupRow.vuesrc/components/rightSidePanel/errors/SwapNodesCard.vuesrc/components/rightSidePanel/errors/TabErrors.vuesrc/components/rightSidePanel/errors/types.tssrc/components/rightSidePanel/errors/useErrorGroups.tssrc/composables/graph/useGraphNodeManager.tssrc/composables/useMissingNodeScan.tssrc/locales/en/main.jsonsrc/platform/nodeReplacement/useNodeReplacement.tssrc/scripts/app.tssrc/stores/executionErrorStore.tssrc/workbench/extensions/manager/types/missingNodeErrorTypes.tssrc/workbench/extensions/manager/utils/missingNodeErrorUtil.test.tssrc/workbench/extensions/manager/utils/missingNodeErrorUtil.ts
💤 Files with no reviewable changes (2)
- src/workbench/extensions/manager/types/missingNodeErrorTypes.ts
- src/workbench/extensions/manager/utils/missingNodeErrorUtil.ts
- Use i18n key for 'Swap Nodes' group title - Preserve partial replacement results on error instead of returning empty array
This was referenced Feb 26, 2026
- Remove redundant parameter that shadowed composable ref - Only remove actually replaced types from error list on partial success
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/rightSidePanel/errors/TabErrors.vue (1)
78-92: Consider adding loading state and moving i18n strings to translation file.Two observations:
Loading state: Unlike the "Install All" button which has
isInstallingAllfor feedback and preventing double-clicks, this button has no loading/disabled state during replacement. IfreplaceNodesInPlaceis slow or if users double-click, it could cause unexpected behavior.i18n: The inline fallback strings (
'Replaces all available nodes in this group.','Replace All') should be added tosrc/locales/en/main.jsonrather than relying on inline defaults. As per coding guidelines: "place new translation entries in src/locales/en/main.json".🔧 Suggested improvement for loading state
const { isInstalling: isInstallingAll, installAllPacks: installAll } = usePackInstall(() => missingNodePacks.value) const { replaceNodesInPlace } = useNodeReplacement() const executionErrorStore = useExecutionErrorStore() +const isReplacingAll = ref(false) // ... -function handleReplaceAll() { +async function handleReplaceAll() { + if (isReplacingAll.value) return + isReplacingAll.value = true const allNodeTypes = swapNodeGroups.value.flatMap((g) => g.nodeTypes) const replaced = replaceNodesInPlace(allNodeTypes) if (replaced.length > 0) { executionErrorStore.removeMissingNodesByType(replaced) } + isReplacingAll.value = false }And in the template:
<Button v-else-if="group.type === 'swap_nodes'" v-tooltip.top="t('nodeReplacement.replaceAllWarning')" variant="secondary" size="sm" class="shrink-0 mr-2 h-8 rounded-lg text-sm" + :disabled="isReplacingAll" `@click.stop`="handleReplaceAll()" > + <DotSpinner v-if="isReplacingAll" duration="1s" :size="12" /> {{ t('nodeReplacement.replaceAll') }} </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/rightSidePanel/errors/TabErrors.vue` around lines 78 - 92, Add a loading/disabled state and move inline i18n strings to the locale file: create a reactive flag (e.g., isReplacingAll) in the TabErrors component, update the template Button (v-else-if block) to bind loading/disabled/aria-busy to isReplacingAll and remove inline fallback strings so the label and tooltip use t('nodeReplacement.replaceAll') and t('nodeReplacement.replaceAllWarning') keys; add those keys to src/locales/en/main.json; update the handleReplaceAll method to set isReplacingAll = true before calling replaceNodesInPlace(...) and set it back to false in finally (and handle errors) to prevent double clicks while the operation is in progress.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/rightSidePanel/errors/TabErrors.vue`:
- Around line 78-92: Add a loading/disabled state and move inline i18n strings
to the locale file: create a reactive flag (e.g., isReplacingAll) in the
TabErrors component, update the template Button (v-else-if block) to bind
loading/disabled/aria-busy to isReplacingAll and remove inline fallback strings
so the label and tooltip use t('nodeReplacement.replaceAll') and
t('nodeReplacement.replaceAllWarning') keys; add those keys to
src/locales/en/main.json; update the handleReplaceAll method to set
isReplacingAll = true before calling replaceNodesInPlace(...) and set it back to
false in finally (and handle errors) to prevent double clicks while the
operation is in progress.
Await nodeReplacementStore.load() before collectMissingNodesAndModels to prevent race condition where replacement mappings are not yet available when determining isReplaceable flag.
christian-byrne
approved these changes
Feb 27, 2026
christian-byrne
approved these changes
Feb 27, 2026
| import type { MissingNodeType } from '@/types/comfy' | ||
| import type { SwapNodeGroup } from './useErrorGroups' | ||
| import { useNodeReplacement } from '@/platform/nodeReplacement/useNodeReplacement' | ||
| import { useExecutionErrorStore } from '@/stores/executionErrorStore' |
Contributor
There was a problem hiding this comment.
suggestion (non-blocking): this component directly imports and calls useNodeReplacement() and useExecutionErrorStore() to perform replacements. The sibling MissingPackGroupRow.vue follows the emit pattern instead - it emits locateNode/openManagerInfo and lets the parent handle store interactions. Consider emitting a 'replace' event and handling the replace+remove logic in TabErrors.vue (which already has handleReplaceAll with the same pattern). This keeps replacement logic centralized and consistent with the component guideline to prefer emit for state changes.
@coderabbitai Make an Issue to do this in a followup PR on a future date
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create GitHub issue for refactoring SwapNodeGroupRow to use emit pattern
gh issue create \
--title "Refactor SwapNodeGroupRow to emit 'replace' event instead of calling stores directly" \
--body "## Context
In PR `#9253`, a new component \`SwapNodeGroupRow.vue\` was introduced that directly imports and calls \`useNodeReplacement()\` and \`useExecutionErrorStore()\` to perform node replacements.
## Current Implementation
\`SwapNodeGroupRow.vue\` currently:
- Directly imports \`useNodeReplacement()\` and \`useExecutionErrorStore()\`
- Handles replacement logic internally within the component
- Calls stores directly to perform state changes
## Proposed Refactoring
The sibling component \`MissingPackGroupRow.vue\` follows the emit pattern, emitting \`locateNode\` and \`openManagerInfo\` events and letting the parent (\`TabErrors.vue\`) handle store interactions.
**Suggested approach:**
1. Emit a \`'replace'\` event from \`SwapNodeGroupRow.vue\` instead of calling stores directly
2. Move the replacement and cleanup logic to \`TabErrors.vue\`, which already has \`handleReplaceAll\` following this pattern
3. Keep the component focused on presentation and user interaction
## Benefits
- **Centralized logic**: All replacement operations handled in one place (\`TabErrors.vue\`)
- **Consistent pattern**: Aligns with existing \`MissingPackGroupRow.vue\` implementation
- **Component guidelines**: Follows the codebase preference for \`emit/event-name\` for state changes (see learning aa08db24)
- **Better separation of concerns**: Component focuses on UI, parent handles business logic
## References
- Original PR: `#9253`
- Discussion: https://github.com/Comfy-Org/ComfyUI_frontend/pull/9253#discussion_r2862082614
- Suggested by: `@christian-byrne`
- Related file: \`src/components/rightSidePanel/errors/SwapNodeGroupRow.vue\`
## File Location
\`src/components/rightSidePanel/errors/SwapNodeGroupRow.vue\`" \
--assignee christian-byrne \
--label enhancement
Length of output: 57
@christian-byrne Issue created! 🎯
I've opened #9267 to track the refactoring of SwapNodeGroupRow.vue to use the emit pattern instead of direct store calls. The issue documents the current implementation, proposed changes, benefits, and includes references to this PR and discussion thread.
The issue has been assigned to you and labeled as an enhancement.
🧠 Learnings used
Learnt from: LittleSound
Repo: Comfy-Org/ComfyUI_frontend PR: 7812
File: src/components/rightSidePanel/RightSidePanel.vue:100-132
Timestamp: 2026-01-09T07:29:32.501Z
Learning: The `findParentGroupInGraph` function in `src/components/rightSidePanel/RightSidePanel.vue` is a temporary workaround for a bug where group sub-items were not updating correctly after a page refresh. It can be removed once that underlying bug is fixed.
Learnt from: jaeone94
Repo: Comfy-Org/ComfyUI_frontend PR: 8875
File: src/components/rightSidePanel/errors/TabErrors.vue:244-280
Timestamp: 2026-02-15T11:04:33.183Z
Learning: In `src/components/rightSidePanel/errors/TabErrors.vue`, the `regroupByErrorMessage` function hardcodes `priority: 1` for all message groups because it operates only in single-node mode where: (1) `processPromptError` is excluded by the `!selectedGraphNodeIds.value` guard, so prompt errors never appear, and (2) sorting uses `title.localeCompare()` instead of priority. The field exists solely to satisfy the `ErrorGroup` interface.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 8548
File: src/components/common/TreeExplorerV2.vue:2-89
Timestamp: 2026-02-07T14:46:21.179Z
Learning: In Vue components (e.g., TreeExplorerV2.vue and similar Recka UI components) avoid wrapping ContextMenuRoot with a dynamic <component :is="..."> and rendering alternative branches. Use v-if / v-else to render separate ContextMenuRoot and non-ContextMenuRoot branches to ensure correct behavior with Reka UI, even if it duplicates some tree structure. This guideline applies to Vue components under src/components and can be checked by ensuring there is a dedicated render path for the ContextMenuRoot without dynamic component switching.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:50.043Z
Learning: Applies to src/components/**/*.vue : Prefer `emit/event-name` for state changes in Vue components
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.{vue,ts} : Implement proper error handling
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-27T20:51:47.936Z
Learning: Applies to src/**/*.{js,ts,vue} : Implement proper error propagation
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-27T20:51:47.936Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid `ts-expect-error` - fix the underlying type issue instead
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 8548
File: src/components/common/TreeExplorerV2.vue:3-5
Timestamp: 2026-02-08T16:04:58.463Z
Learning: In TreeExplorerV2.vue and similar Re- ka UI Tree components, avoid defensive copies in bindings like :expanded="[...expandedKeys]". Use v-model:expanded="expandedKeys" instead. TreeRoot follows Vue's controlled component pattern and emits update:expanded with the new array; it does not mutate the passed array. Defensive copies break reactivity and can desynchronize state. Ensure components rely on update:expanded and do not mutate input arrays.
Learnt from: Yourz
Repo: Comfy-Org/ComfyUI_frontend PR: 8548
File: src/components/common/TreeExplorerV2Node.vue:9-47
Timestamp: 2026-02-07T14:47:06.751Z
Learning: In all Vue files that render Reka UI context menu components (ContextMenuRoot, ContextMenuTrigger, etc.), avoid dynamic component wrapping with <component :is="...">. Use conditional rendering with v-if / v-else to render separate branches, even if it results in some template duplication. This improves readability and maintainability across files that use these components.
Learnt from: benceruleanlu
Repo: Comfy-Org/ComfyUI_frontend PR: 7297
File: src/components/actionbar/ComfyActionbar.vue:33-43
Timestamp: 2025-12-09T21:40:12.361Z
Learning: In Vue single-file components, allow inline Tailwind CSS class strings for static classes and avoid extracting them into computed properties solely for readability. Prefer keeping static class names inline for simplicity and performance. For dynamic or conditional classes, use Vue bindings (e.g., :class) to compose classes.
Applies to all Vue files in the repository (e.g., src/**/*.vue) where Tailwind utilities are used for static styling.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.vue:17-17
Timestamp: 2025-12-16T22:26:49.463Z
Learning: In Vue 3.5+ with <script setup>, when using defineProps<Props>() with partial destructuring (e.g., const { as = 'button', class: customClass = '' } = defineProps<Props>() ), props that are not destructured (e.g., variant, size) stay accessible by name in the template scope. This pattern is valid: you can destructure only a subset of props for convenience while referencing the remaining props directly in template expressions. Apply this guideline to Vue components across the codebase (all .vue files).
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7598
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:131-131
Timestamp: 2025-12-18T02:07:38.870Z
Learning: Tailwind CSS v4 safe utilities (e.g., items-center-safe, justify-*-safe, place-*-safe) are allowed in Vue components under src/ and in story files. Do not flag these specific safe variants as invalid when reviewing code in src/**/*.vue or related stories.
Learnt from: henrikvilhelmberglund
Repo: Comfy-Org/ComfyUI_frontend PR: 7617
File: src/components/actionbar/ComfyActionbar.vue:301-308
Timestamp: 2025-12-18T16:03:02.066Z
Learning: In the ComfyUI frontend queue system, useQueuePendingTaskCountStore().count indicates the number of tasks in the queue, where count = 1 means a single active/running task and count > 1 means there are pending tasks in addition to the active task. Therefore, in src/components/actionbar/ComfyActionbar.vue, enable the 'Clear Pending Tasks' button only when count > 1 to avoid clearing the currently running task. The active task should be canceled using the 'Cancel current run' button instead. This rule should be enforced via a conditional check on the queue count, with appropriate disabled/aria-disabled states for accessibility, and tests should verify behavior for count = 1 and count > 1.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7603
File: src/components/queue/QueueOverlayHeader.vue:49-59
Timestamp: 2025-12-18T21:15:46.862Z
Learning: In the ComfyUI_frontend repository, for Vue components, do not add aria-label to buttons that have visible text content (e.g., buttons containing <span> text). The visible text provides the accessible name. Use aria-label only for elements without visible labels (e.g., icon-only buttons). If a button has no visible label, provide a clear aria-label or associate with an aria-labelledby describing its action.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/components/graph/selectionToolbox/ColorPickerButton.vue:15-18
Timestamp: 2025-12-21T01:06:02.786Z
Learning: In Comfy-Org/ComfyUI_frontend, in Vue component files, when a filled icon is required (e.g., 'pi pi-circle-fill'), you may mix PrimeIcons with Lucide icons since Lucide lacks filled variants. This mixed usage is acceptable when one icon library does not provide an equivalent filled icon. Apply consistently across Vue components in the src directory where icons are used, and document the rationale when a mixed approach is chosen.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/platform/cloud/subscription/components/PricingTable.vue:185-201
Timestamp: 2025-12-22T21:36:08.369Z
Learning: In Vue components, avoid creating single-use variants for common UI components (e.g., Button and other shared components). Aim for reusable variants that cover multiple use cases. It’s acceptable to temporarily mix variant props with inline Tailwind classes when a styling need is unique to one place, but plan and consolidate into shared, reusable variants as patterns emerge across the codebase.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7893
File: src/components/button/IconGroup.vue:5-6
Timestamp: 2026-01-08T02:26:18.357Z
Learning: In components that use the cn utility from '@/utils/tailwindUtil' with tailwind-merge, rely on the behavior that conflicting Tailwind classes are resolved by keeping the last one. For example, cn('base-classes bg-default', propClass) will have any conflicting background class from propClass override bg-default. This additive pattern is intentional and aligns with the shadcn-ui convention; ensure you document or review expectations accordingly in Vue components.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7906
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:545-552
Timestamp: 2026-01-12T17:39:27.738Z
Learning: In Vue/TypeScript files (src/**/*.{ts,tsx,vue}), prefer if/else statements over ternary operators when performing side effects or actions (e.g., mutating state, calling methods with side effects). Ternaries should be reserved for computing and returning values.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8195
File: src/platform/assets/components/MediaAssetFilterBar.vue:16-16
Timestamp: 2026-01-21T01:28:27.626Z
Learning: In Vue templates (Vue 3.4+ with the build step), when binding to data or props that are camelCase (e.g., mediaTypeFilters), you can use kebab-case in the template bindings (e.g., :media-type-filters). This is acceptable and will resolve to the corresponding camelCase variable. Do not require CamelCase in template bindings.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8090
File: src/platform/assets/components/modelInfo/ModelInfoField.vue:8-11
Timestamp: 2026-01-22T02:28:58.105Z
Learning: In Vue 3 script setup, props defined with defineProps are automatically available by name in the template without destructuring. Destructuring the result of defineProps inside script can break reactivity; prefer accessing props by name in the template. If you need to use props in the script, reference them via the defined props object rather than destructuring, or use toRefs when you intend to destructure while preserving reactivity.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8497
File: src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue:223-236
Timestamp: 2026-02-01T21:10:29.567Z
Learning: In Vue single-file components, do not review or require edits to comments. Favor self-documenting code through clear naming, strong types, and explicit APIs. If a comment is misleading or outdated, consider removing it, but avoid suggesting adding or fixing comments. This guideline aligns with preferring code clarity over comment maintenance across all .vue files.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8592
File: src/components/topbar/WorkflowExecutionIndicator.vue:28-28
Timestamp: 2026-02-03T21:35:40.889Z
Learning: In Vue single-file components where the i18n t function is only used within the template, prefer using the built-in $t in the template instead of importing useI18n and destructuring t in the script. This avoids unnecessary imports when t is not used in the script. If you need i18n in the script (Composition API), only then use useI18n and access t from its returned object. Ensure this pattern applies to all Vue components with template-only i18n usage.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8753
File: src/renderer/extensions/vueNodes/widgets/components/WidgetDOM.vue:17-28
Timestamp: 2026-02-09T03:24:47.113Z
Learning: When destructuring reactive properties from Pinia stores in Vue components, use storeToRefs() to preserve reactivity. Example: const store = useCanvasStore(); const { canvas } = storeToRefs(store); access as canvas.value (e.g., canvas.value). Ensure you import storeToRefs from 'pinia' and use it wherever you destructure store properties in the setup function.
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 8761
File: src/components/dialog/content/MissingModelsWarning.vue:21-27
Timestamp: 2026-02-10T00:58:07.904Z
Learning: In the ComfyUI_frontend Vue codebase, replace raw <button> HTML elements with the shared Button component located at src/components/ui/button/Button.vue. Import and use it with appropriate variants (e.g., variant="link") to align with the design system. Apply this pattern across Vue components under src/components, ensuring consistent styling and behavior instead of ad-hoc button markup.
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI_frontend PR: 8775
File: src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue:62-68
Timestamp: 2026-02-10T17:59:25.893Z
Learning: In Vue 3 single-file components, code defined outside the script setup scope cannot access variables or helpers defined inside setup. If a default function (e.g., defineModel) is hoisted outside script setup, it will not be able to call setup helpers or reference setup-scoped variables. Place such logic inside setup or expose necessary values via returns/defineExpose to be accessible.
| return !removeSet.has(nodeType) | ||
| }) | ||
| setMissingNodeTypes(remaining) | ||
| } |
Contributor
There was a problem hiding this comment.
suggestion (non-blocking): the PR adds several new testable functions (removeMissingNodesByType, scanMissingNodes, the swapNodeGroups computed) without corresponding tests. At minimum, removeMissingNodesByType (set-based filtering) and the swapNodeGroups grouping logic in useErrorGroups.ts would benefit from unit tests to catch regressions if the filtering/grouping criteria change.
@coderabbitai Make an Issue to do this in a followup PR on a future date
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for adding unit tests to new functions from PR `#9253`
gh issue create \
--title "Add unit tests for new error handling functions from PR `#9253`" \
--body "## Description
This issue tracks the addition of unit tests for new testable functions introduced in PR `#9253` (node replacement UI in Errors Tab).
## Functions that need tests
1. **\`removeMissingNodesByType\`** in \`src/stores/executionErrorStore.ts\`
- Set-based filtering logic for removing specific node types from the missing nodes list
- Should test edge cases: empty array, non-existent types, string vs object node types
2. **\`scanMissingNodes\`** in \`src/composables/useMissingNodeScan.ts\`
- Graph scanning logic to detect unregistered node types
- Should test: registered vs unregistered nodes, CNR ID resolution, replacement detection
3. **\`swapNodeGroups\`** computed in \`src/components/rightSidePanel/errors/useErrorGroups.ts\`
- Grouping logic for replaceable missing nodes
- Should test: grouping by type, replacement info collection, filtering criteria
## Context
These tests would help catch regressions if the filtering/grouping criteria change in the future.
## References
- Original PR: `#9253`
- Comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/9253#discussion_r2862082616
- Requested by: `@christian-byrne`" \
--assignee christian-byrne \
--label "enhancement,testing"
Length of output: 75
@christian-byrne, I attempted to create the GitHub issue but the sandbox environment lacks GitHub authentication. Here's the issue content ready for you to create manually:
Title:
Add unit tests for new error handling functions from PR `#9253`
Body:
## Description
This issue tracks the addition of unit tests for new testable functions introduced in PR `#9253` (node replacement UI in Errors Tab).
## Functions that need tests
1. **`removeMissingNodesByType`** in `src/stores/executionErrorStore.ts`
- Set-based filtering logic for removing specific node types from the missing nodes list
- Should test edge cases: empty array, non-existent types, string vs object node types
2. **`scanMissingNodes`** in `src/composables/useMissingNodeScan.ts`
- Graph scanning logic to detect unregistered node types
- Should test: registered vs unregistered nodes, CNR ID resolution, replacement detection
3. **`swapNodeGroups`** computed in `src/components/rightSidePanel/errors/useErrorGroups.ts`
- Grouping logic for replaceable missing nodes
- Should test: grouping by type, replacement info collection, filtering criteria
## Context
These tests would help catch regressions if the filtering/grouping criteria change in the future.
## References
- Original PR: `#9253`
- Comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/9253#discussion_r2862082616
- Requested by: `@christian-byrne`Labels: enhancement, testing
Assignee: christian-byrne
You can create this issue directly on GitHub or run the following command locally if you have gh CLI configured:
gh issue create \
--repo Comfy-Org/ComfyUI_frontend \
--title "Add unit tests for new error handling functions from PR `#9253`" \
--body "..." \
--assignee christian-byrne \
--label "enhancement,testing"🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/AGENTS.md:0-0
Timestamp: 2026-02-23T00:41:09.436Z
Learning: Applies to src/lib/litegraph/**/*.test.{ts,tsx} : Use test helper functions `createTestSubgraph()` and `createTestSubgraphNode()` from `./__fixtures__/subgraphHelpers` when setting up subgraph tests
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-27T20:51:47.936Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid `ts-expect-error` - fix the underlying type issue instead
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8307
File: src/stores/bootstrapStore.ts:61-78
Timestamp: 2026-01-26T18:39:22.294Z
Learning: Do not expose internal state or error refs from stores unless there is demonstrated active usage elsewhere in the codebase. Keep internal state private by default; only expose what is actively consumed. Applies to all TypeScript files under src/stores (e.g., bootstrapStore.ts and siblings). Ensure exported API only includes properties/methods that are actually used by consumers; avoid leaking implementation details.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7906
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:545-552
Timestamp: 2026-01-12T17:39:27.738Z
Learning: In Vue/TypeScript files (src/**/*.{ts,tsx,vue}), prefer if/else statements over ternary operators when performing side effects or actions (e.g., mutating state, calling methods with side effects). Ternaries should be reserved for computing and returning values.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8248
File: src/platform/assets/schemas/assetSchema.ts:109-118
Timestamp: 2026-01-22T23:44:59.888Z
Learning: Do not export declarations (e.g., schemas, constants, types) unless they are actually used elsewhere in the codebase. Keep the public API surface minimal by exporting only items with concrete usage. This reduces coupling and keeps imports explicit. In src/platform/assets/schemas/assetSchema.ts, ensure only necessary exports are exposed; remove unused exports or switch to non-exported/internal where appropriate.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8722
File: src/composables/maskeditor/useBrushDrawing.ts:5-5
Timestamp: 2026-02-08T03:19:12.009Z
Learning: Treat unresolved import warnings caused by pnpm workspace catalog mappings (pnpm-workspace.yaml) and unplugin-typegpu in vite.config.mts as false positives. Do not flag these in PR reviews; they are expected in sandboxes without node_modules and are not indicative of actual missing dependencies in the TS codebase.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/load3d.ts:427-430
Timestamp: 2026-02-19T02:06:23.468Z
Learning: In TypeScript, you can use typeof SomeClass in type annotation positions (e.g., param: typeof LGraphNode) even when SomeClass is imported with import type. This is a type query that only exists at compile time and is erased in runtime, so it is safe to combine with type-only imports. Apply this pattern to TS files broadly when you need a type that references the shape of a class or constructor function without importing the value at runtime.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/uploadAudio.ts:91-94
Timestamp: 2026-02-19T02:06:38.395Z
Learning: In TypeScript files, you can use a type annotation like 'nodeType: typeof MyClass' even if MyClass is imported via 'import type'. Both the type-only import and 'typeof' operate at the type level and are erased at compile time. This pattern is commonly used for constructor types (e.g., 'nodeType: typeof LGraphNode'). Apply this pattern across TypeScript files in the repository (src/**/*.ts) as appropriate, ensuring the imported symbol is a type-only import when possible for clarity and to avoid runtime imports.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8992
File: src/lib/litegraph/src/widgets/GradientSliderWidget.ts:18-18
Timestamp: 2026-02-20T21:08:19.814Z
Learning: When drawing with CanvasRenderingContext2D in TypeScript/JavaScript, wrap the drawing logic with ctx.save() at the start and ctx.restore() at the end to preserve and restore the canvas state. Do not manually destructure and restore individual properties (e.g., fillStyle, strokeStyle); rely on save/restore to manage state changes in a scoped manner. This should be applied to all TS files that perform canvas drawing.
Learnt from: dante01yoon
Repo: Comfy-Org/ComfyUI_frontend PR: 9075
File: src/scripts/api.featureFlags.test.ts:237-268
Timestamp: 2026-02-22T04:27:33.379Z
Learning: In Vite/Vitest, import.meta.env.DEV is true for any mode that is not 'production' (i.e., DEV is the opposite of PROD, and can be true in 'test', 'development', etc.). Do not assume DEV implies only 'development' mode. When reviewing code and tests, treat DEV as a non-production flag and verify environment-specific logic accordingly. Reference: https://vite.dev/guide/env-and-mode#modes
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
@jaeone94, I'll retry creating the GitHub issue for the unit test follow-up.
🏁 Script executed:
#!/bin/bash
# Retry creating GitHub issue for adding unit tests to new functions from PR `#9253`
gh issue create \
--repo Comfy-Org/ComfyUI_frontend \
--title "Add unit tests for new error handling functions from PR `#9253`" \
--body "## Description
This issue tracks the addition of unit tests for new testable functions introduced in PR `#9253` (node replacement UI in Errors Tab).
## Functions that need tests
1. **\`removeMissingNodesByType\`** in \`src/stores/executionErrorStore.ts\`
- Set-based filtering logic for removing specific node types from the missing nodes list
- Should test edge cases: empty array, non-existent types, string vs object node types
2. **\`scanMissingNodes\`** in \`src/composables/useMissingNodeScan.ts\`
- Graph scanning logic to detect unregistered node types
- Should test: registered vs unregistered nodes, CNR ID resolution, replacement detection
3. **\`swapNodeGroups\`** computed in \`src/components/rightSidePanel/errors/useErrorGroups.ts\`
- Grouping logic for replaceable missing nodes
- Should test: grouping by type, replacement info collection, filtering criteria
## Context
These tests would help catch regressions if the filtering/grouping criteria change in the future.
## References
- Original PR: `#9253`
- Comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/9253#discussion_r2862082616
- Requested by: `@christian-byrne`" \
--assignee christian-byrne \
--label "enhancement,testing"
Length of output: 75
@jaeone94, unfortunately the sandbox environment doesn't have GitHub authentication configured, so I can't create the issue directly. However, here are two ways you can create it:
Option 1: Create manually on GitHub
Navigate to https://github.com/Comfy-Org/ComfyUI_frontend/issues/new and use this content:
Title:
Add unit tests for new error handling functions from PR `#9253`
Body:
## Description
This issue tracks the addition of unit tests for new testable functions introduced in PR `#9253` (node replacement UI in Errors Tab).
## Functions that need tests
1. **`removeMissingNodesByType`** in `src/stores/executionErrorStore.ts`
- Set-based filtering logic for removing specific node types from the missing nodes list
- Should test edge cases: empty array, non-existent types, string vs object node types
2. **`scanMissingNodes`** in `src/composables/useMissingNodeScan.ts`
- Graph scanning logic to detect unregistered node types
- Should test: registered vs unregistered nodes, CNR ID resolution, replacement detection
3. **`swapNodeGroups`** computed in `src/components/rightSidePanel/errors/useErrorGroups.ts`
- Grouping logic for replaceable missing nodes
- Should test: grouping by type, replacement info collection, filtering criteria
## Context
These tests would help catch regressions if the filtering/grouping criteria change in the future.
## References
- Original PR: `#9253`
- Comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/9253#discussion_r2862082616
- Requested by: `@christian-byrne`Labels: enhancement, testing
Assignee: christian-byrne
Option 2: Run locally with gh CLI
If you have gh CLI configured on your machine:
gh issue create \
--repo Comfy-Org/ComfyUI_frontend \
--title "Add unit tests for new error handling functions from PR `#9253`" \
--body "## Description
This issue tracks the addition of unit tests for new testable functions introduced in PR `#9253` (node replacement UI in Errors Tab).
## Functions that need tests
1. **\`removeMissingNodesByType\`** in \`src/stores/executionErrorStore.ts\`
- Set-based filtering logic for removing specific node types from the missing nodes list
- Should test edge cases: empty array, non-existent types, string vs object node types
2. **\`scanMissingNodes\`** in \`src/composables/useMissingNodeScan.ts\`
- Graph scanning logic to detect unregistered node types
- Should test: registered vs unregistered nodes, CNR ID resolution, replacement detection
3. **\`swapNodeGroups\`** computed in \`src/components/rightSidePanel/errors/useErrorGroups.ts\`
- Grouping logic for replaceable missing nodes
- Should test: grouping by type, replacement info collection, filtering criteria
## Context
These tests would help catch regressions if the filtering/grouping criteria change in the future.
## References
- Original PR: `#9253`
- Comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/9253#discussion_r2862082616
- Requested by: `@christian-byrne`" \
--assignee christian-byrne \
--label "enhancement,testing"🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-27T20:51:47.936Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid `ts-expect-error` - fix the underlying type issue instead
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-23T21:34:45.361Z
Learning: Applies to src/**/*.test.ts : For Vue component testing, use Vue Test Utils and follow advice about making components easy to test; aim for behavioral coverage of critical and new features
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7459
File: src/lib/litegraph/src/canvas/LinkConnector.ts:875-881
Timestamp: 2026-02-23T19:49:55.224Z
Learning: ComfyUI_frontend TypeScript review preference: Prefer user-defined type guards (e.g., function isMovingInputLink(x): x is MovingInputLink) over inline type assertions when narrowing union types like RenderLinkUnion in src/lib/litegraph/**/*.ts. Apply especially in LinkConnector and related canvas classes to avoid casts and improve reuse.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-27T20:51:47.936Z
Learning: Applies to src/**/*.{js,ts,vue} : Implement proper error propagation
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/AGENTS.md:0-0
Timestamp: 2026-02-23T00:41:09.436Z
Learning: Applies to src/lib/litegraph/**/*.test.{ts,tsx} : Use test helper functions `createTestSubgraph()` and `createTestSubgraphNode()` from `./__fixtures__/subgraphHelpers` when setting up subgraph tests
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:7-15
Timestamp: 2025-12-13T05:34:22.209Z
Learning: In Playwright tests in the Comfy-Org/ComfyUI_frontend repository, the `toPass()` assertion method implements incremental backoff during retries, so increasing the timeout value (e.g., to 5000ms) is sufficient for handling async operations without being overly aggressive.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8722
File: src/composables/maskeditor/useBrushDrawing.ts:5-5
Timestamp: 2026-02-08T03:19:12.009Z
Learning: Treat unresolved import warnings caused by pnpm workspace catalog mappings (pnpm-workspace.yaml) and unplugin-typegpu in vite.config.mts as false positives. Do not flag these in PR reviews; they are expected in sandboxes without node_modules and are not indicative of actual missing dependencies in the TS codebase.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8307
File: src/stores/bootstrapStore.ts:61-78
Timestamp: 2026-01-26T18:39:22.294Z
Learning: Do not expose internal state or error refs from stores unless there is demonstrated active usage elsewhere in the codebase. Keep internal state private by default; only expose what is actively consumed. Applies to all TypeScript files under src/stores (e.g., bootstrapStore.ts and siblings). Ensure exported API only includes properties/methods that are actually used by consumers; avoid leaking implementation details.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7906
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:545-552
Timestamp: 2026-01-12T17:39:27.738Z
Learning: In Vue/TypeScript files (src/**/*.{ts,tsx,vue}), prefer if/else statements over ternary operators when performing side effects or actions (e.g., mutating state, calling methods with side effects). Ternaries should be reserved for computing and returning values.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8248
File: src/platform/assets/schemas/assetSchema.ts:109-118
Timestamp: 2026-01-22T23:44:59.888Z
Learning: Do not export declarations (e.g., schemas, constants, types) unless they are actually used elsewhere in the codebase. Keep the public API surface minimal by exporting only items with concrete usage. This reduces coupling and keeps imports explicit. In src/platform/assets/schemas/assetSchema.ts, ensure only necessary exports are exposed; remove unused exports or switch to non-exported/internal where appropriate.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/load3d.ts:427-430
Timestamp: 2026-02-19T02:06:23.468Z
Learning: In TypeScript, you can use typeof SomeClass in type annotation positions (e.g., param: typeof LGraphNode) even when SomeClass is imported with import type. This is a type query that only exists at compile time and is erased in runtime, so it is safe to combine with type-only imports. Apply this pattern to TS files broadly when you need a type that references the shape of a class or constructor function without importing the value at runtime.
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 8966
File: src/extensions/core/uploadAudio.ts:91-94
Timestamp: 2026-02-19T02:06:38.395Z
Learning: In TypeScript files, you can use a type annotation like 'nodeType: typeof MyClass' even if MyClass is imported via 'import type'. Both the type-only import and 'typeof' operate at the type level and are erased at compile time. This pattern is commonly used for constructor types (e.g., 'nodeType: typeof LGraphNode'). Apply this pattern across TypeScript files in the repository (src/**/*.ts) as appropriate, ensuring the imported symbol is a type-only import when possible for clarity and to avoid runtime imports.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 8992
File: src/lib/litegraph/src/widgets/GradientSliderWidget.ts:18-18
Timestamp: 2026-02-20T21:08:19.814Z
Learning: When drawing with CanvasRenderingContext2D in TypeScript/JavaScript, wrap the drawing logic with ctx.save() at the start and ctx.restore() at the end to preserve and restore the canvas state. Do not manually destructure and restore individual properties (e.g., fillStyle, strokeStyle); rely on save/restore to manage state changes in a scoped manner. This should be applied to all TS files that perform canvas drawing.
Learnt from: dante01yoon
Repo: Comfy-Org/ComfyUI_frontend PR: 9075
File: src/scripts/api.featureFlags.test.ts:237-268
Timestamp: 2026-02-22T04:27:33.379Z
Learning: In Vite/Vitest, import.meta.env.DEV is true for any mode that is not 'production' (i.e., DEV is the opposite of PROD, and can be true in 'test', 'development', etc.). Do not assume DEV implies only 'development' mode. When reviewing code and tests, treat DEV as a non-production flag and verify environment-specific logic accordingly. Reference: https://vite.dev/guide/env-and-mode#modes
christian-byrne
pushed a commit
that referenced
this pull request
Feb 27, 2026
## Summary Adds a node replacement UI to the Errors Tab so users can swap missing nodes with compatible alternatives directly from the error panel, without opening a separate dialog. ## Changes - **What**: New `SwapNodesCard` and `SwapNodeGroupRow` components render swap groups in the Errors Tab; each group shows the missing node type, its instances (with locate buttons), and a Replace button. Added `useMissingNodeScan` composable to scan the graph for missing nodes and populate `executionErrorStore`. Added `removeMissingNodesByType()` to `executionErrorStore` so replaced nodes are pruned from the error list reactively. ## Bug Fixes Found During Implementation ### Bug 1: Replaced nodes render as empty shells until page refresh `replaceWithMapping()` directly mutates `_nodes[idx]`, bypassing the Vue rendering pipeline entirely. Because the replacement node reuses the same ID, `vueNodeData` retains the stale entry from the old placeholder (`hasErrors: true`, empty widgets/inputs). `graph.setDirtyCanvas()` only repaints the LiteGraph canvas and has no effect on Vue. **Fix**: After `replaceWithMapping()`, manually call `nodeGraph.onNodeAdded?.(newNode)` to trigger `handleNodeAdded` in `useGraphNodeManager`, which runs `extractVueNodeData(newNode)` and updates `vueNodeData` correctly. Also added a guard in `handleNodeAdded` to skip `layoutStore.createNode()` when a layout for the same ID already exists, preventing a duplicate `spatialIndex.insert()`. ### Bug 2: Missing node error list overwritten by incomplete server response Two compounding issues: (A) the server's `missing_node_type` error only reports the *first* missing node — the old handler parsed this and called `surfaceMissingNodes([singleNode])`, overwriting the full list collected at load time. (B) `queuePrompt()` calls `clearAllErrors()` before the API request; if the subsequent rescan used the stale `has_errors` flag and found nothing, the missing nodes were permanently lost. **Fix**: Created `useMissingNodeScan.ts` which scans `LiteGraph.registered_node_types` directly (not `has_errors`). The `missing_node_type` catch block in `app.ts` now calls `rescanAndSurfaceMissingNodes(this.rootGraph)` instead of parsing the server's partial response. ## Review Focus - `handleReplaceNode` removes the group from the store only when `replaceNodesInPlace` returns at least one replaced node — should we always clear, or only on full success? - `useMissingNodeScan` re-scans on every execution-error change; confirm no performance concerns for large graphs with many subgraphs. ## Screenshots https://github.com/user-attachments/assets/78310fc4-0424-4920-b369-cef60a123d50 https://github.com/user-attachments/assets/3d2fd5e1-5e85-4c20-86aa-8bf920e86987 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9253-feat-add-node-replacement-UI-to-Errors-Tab-3136d73d365081718d4ddfd628cb4449) by [Unito](https://www.unito.io)
This was referenced Feb 27, 2026
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
2 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
Adds a node replacement UI to the Errors Tab so users can swap missing nodes with compatible alternatives directly from the error panel, without opening a separate dialog.
Changes
SwapNodesCardandSwapNodeGroupRowcomponents render swap groups in the Errors Tab; each group shows the missing node type, its instances (with locate buttons), and a Replace button. AddeduseMissingNodeScancomposable to scan the graph for missing nodes and populateexecutionErrorStore. AddedremoveMissingNodesByType()toexecutionErrorStoreso replaced nodes are pruned from the error list reactively.Bug Fixes Found During Implementation
Bug 1: Replaced nodes render as empty shells until page refresh
replaceWithMapping()directly mutates_nodes[idx], bypassing the Vue rendering pipeline entirely. Because the replacement node reuses the same ID,vueNodeDataretains the stale entry from the old placeholder (hasErrors: true, empty widgets/inputs).graph.setDirtyCanvas()only repaints the LiteGraph canvas and has no effect on Vue.Fix: After
replaceWithMapping(), manually callnodeGraph.onNodeAdded?.(newNode)to triggerhandleNodeAddedinuseGraphNodeManager, which runsextractVueNodeData(newNode)and updatesvueNodeDatacorrectly. Also added a guard inhandleNodeAddedto skiplayoutStore.createNode()when a layout for the same ID already exists, preventing a duplicatespatialIndex.insert().Bug 2: Missing node error list overwritten by incomplete server response
Two compounding issues: (A) the server's
missing_node_typeerror only reports the first missing node — the old handler parsed this and calledsurfaceMissingNodes([singleNode]), overwriting the full list collected at load time. (B)queuePrompt()callsclearAllErrors()before the API request; if the subsequent rescan used the stalehas_errorsflag and found nothing, the missing nodes were permanently lost.Fix: Created
useMissingNodeScan.tswhich scansLiteGraph.registered_node_typesdirectly (nothas_errors). Themissing_node_typecatch block inapp.tsnow callsrescanAndSurfaceMissingNodes(this.rootGraph)instead of parsing the server's partial response.Review Focus
handleReplaceNoderemoves the group from the store only whenreplaceNodesInPlacereturns at least one replaced node — should we always clear, or only on full success?useMissingNodeScanre-scans on every execution-error change; confirm no performance concerns for large graphs with many subgraphs.Screenshots
2026-02-26.213523.mp4
2026-02-26.213618.mp4
┆Issue is synchronized with this Notion page by Unito