Commit Graph

20 Commits

Author SHA1 Message Date
Dante
f1ea3b02a6 test: add unit tests for useNodeReplacement transfer edge cases (#11677)
## Summary

Extends `useNodeReplacement.test.ts` with five connection-transfer and
graph-mutation edge cases that the existing 23-case suite did not cover:
missing-old-input-slot skip, missing-new-output-index resilience,
set_value on a non-existent widget, set_value with dot-notation new_id,
and the Vue-node refresh path via `nodeGraph.onNodeAdded`.

## Changes

- **What**: Adds 5 Vitest cases in a new `transfer edge cases` describe
block. Reuses the existing `createPlaceholderNode`, `createNewNode`,
`createMockGraph`, `createMockLink`, and `makeMissingNodeType` helpers —
no new test infrastructure introduced.

## Review Focus

- The "missing new output index" test verifies that `replaceWithMapping`
does not throw when `newNode.outputs[newOutputIdx]` is absent, and
asserts the original link's `origin_id` is unchanged so the silent-skip
behavior is pinned (not a swallowed exception).
- The dot-notation `set_value` test pins that the existing dot-notation
guard at `useNodeReplacement.ts:203` covers the `set_value` branch (not
just the `old_id` connection branch already covered at line 187).
- The `onNodeAdded` test asserts the Vue-node sync path that runs after
`replaceWithMapping` bypasses `graph.add()` — a future refactor that
drops the explicit call would silently break the Vue node renderer
otherwise.

## Testing

\`\`\`bash
pnpm exec vitest run
src/platform/nodeReplacement/useNodeReplacement.test.ts
pnpm format -- src/platform/nodeReplacement/useNodeReplacement.test.ts
pnpm lint
pnpm typecheck
pnpm knip
\`\`\`

28 tests pass (23 prior + 5 new).

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11677-test-add-unit-tests-for-useNodeReplacement-transfer-edge-cases-34f6d73d3650817aa2ffccdb9fb4a947)
by [Unito](https://www.unito.io)
2026-04-27 10:25:26 -04:00
Christian Byrne
ef59f46495 refactor: migrate cn imports from @/utils/tailwindUtil shim to @comfyorg/tailwind-utils directly (#11453)
*PR Created by the Glary-Bot Agent*

---

## Summary

- Replace all `cn` / `ClassValue` imports from the
`@/utils/tailwindUtil` re-export shim with direct imports from
`@comfyorg/tailwind-utils` across 198 source files in `src/` and 3 in
`apps/desktop-ui/`
- Delete both shim files (`src/utils/tailwindUtil.ts` and
`apps/desktop-ui/src/utils/tailwindUtil.ts`)
- Add explicit `@comfyorg/tailwind-utils` dependency to
`apps/desktop-ui/package.json`
- Update documentation references in `AGENTS.md`,
`docs/guidance/design-standards.md`, and
`docs/guidance/vue-components.md`

Fixes #11288

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11453-refactor-migrate-cn-imports-from-utils-tailwindUtil-shim-to-comfyorg-tailwind-utils--3486d73d365081ec92cce91fbf88e6e4)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
2026-04-22 18:39:57 -07:00
AustinMroz
91ed6a37e2 Fix nodeReplacement not triggering onRemoved (#11509)
Node Replacement failed to call onRemoved on the old node. This would
cause domWidgets to persist after a node is replaced.

<img width="474" height="257" alt="image"
src="https://github.com/user-attachments/assets/51641de7-81e9-4355-88d9-d1605f397076"
/>

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11509-Fix-nodeReplacement-not-triggering-onRemoved-3496d73d365081e19a4ae252aa87172d)
by [Unito](https://www.unito.io)
2026-04-21 08:14:51 +00:00
jaeone94
521019d173 fix: exclude muted/bypassed nodes from missing asset detection (#10856)
## Summary

Muted and bypassed nodes are excluded from execution but were still
triggering missing model/media/node warnings. This PR makes the error
system mode-aware: muted/bypassed nodes no longer produce missing asset
errors, and all error lifecycle events (mode toggle, deletion, paste,
undo, tab switch) are handled consistently.

- Fixes Comfy-Org/ComfyUI#13256

## Behavioral notes

- **Tab switch overlay suppression (intentional)**: Switching back to a
workflow with missing assets no longer re-shows the error overlay. This
reverses the behavior introduced in #10190. The error state is still
restored silently in the errors tab — users can access it via the
properties panel without being interrupted by the overlay on every tab
switch.

## Changes

### 1. Scan filtering

- `scanAllModelCandidates`, `scanAllMediaCandidates`,
`scanMissingNodes`: skip nodes with `mode === NEVER || BYPASS`
- `collectMissingNodes` (serialized data): skip error reporting for
muted/bypassed nodes while still calling `sanitizeNodeName` for safe
`configure()`
- `collectEmbeddedModelsWithSource`: skip muted/bypassed nodes;
workflow-level `graphData.models` only create candidates when active
nodes exist
- `enrichWithEmbeddedMetadata`: filter unmatched workflow-level models
when all referencing nodes are inactive

### 2. Realtime mode change handling

- `useErrorClearingHooks.ts` chains `graph.onTrigger` to detect
`node:property:changed` (mode)
- Deactivation (active → muted/bypassed): remove missing
model/media/node errors for the node
- Activation (muted/bypassed → active): scan the node and add confirmed
errors, show overlay
- Subgraph container deactivation: remove all interior node errors
(execution ID prefix match)
- Subgraph container activation: scan all active interior nodes
recursively
- Subgraph interior mode change: resolve node via
`localGraph.getNodeById()` then compute execution ID from root graph

### 3. Node deletion

- `graph.onNodeRemoved`: remove missing model/media/node errors for the
deleted node
- Handle `node.graph === null` at callback time by using
`String(node.id)` for root-level nodes

### 4. Node paste/duplicate

- `graph.onNodeAdded`: scan via `queueMicrotask` (deferred until after
`node.configure()` restores widget values)
- Guard: skip during `ChangeTracker.isLoadingGraph` (undo/redo/tab
switch handled by pipeline)
- Guard: skip muted/bypassed nodes

### 5. Workflow tab switch optimization

- `skipAssetScans` option in `loadGraphData`: skip full pipeline on tab
switch
- Cache missing model/media/node state per workflow via
`PendingWarnings`
- `beforeLoadNewGraph`: save current store state to outgoing workflow's
`pendingWarnings`
- `showPendingWarnings`: restore cached errors silently (no overlay),
always sync missing nodes store (even when null)
- Preserve UI state (`fileSizes`, `urlInputs`) on tab switch by using
`setMissingModels([])` instead of `clearMissingModels()`
- `MissingModelRow.vue`: fetch file size on mount via
`fetchModelMetadata` memory cache

### 6. Undo/redo overlay suppression

- `silentAssetErrors` option propagated through pipeline →
`surfaceMissingModels`/`surfaceMissingMedia` `{ silent }` option
- `showPendingWarnings` `{ silent }` option for missing nodes overlay
- `changeTracker.ts`: pass `silentAssetErrors: true` on undo/redo

### 7. Error tab node filtering

- Selected node filters missing model/media card contents (not just
group visibility)
- `isAssetErrorInSelection`: resolve execution ID → graph node for
selection matching
- Missing nodes intentionally unfiltered (pack-level scope)
- `hasMissingMediaSelected` added to `RightSidePanel.vue` error tab
visibility
- Download All button: show only when 2+ downloadable models exist

### 8. New store functions

- `missingModelStore`: `addMissingModels`, `removeMissingModelsByNodeId`
- `missingMediaStore`: `addMissingMedia`, `removeMissingMediaByNodeId`
- `missingNodesErrorStore`: `removeMissingNodesByNodeId`
- `missingModelScan`: `scanNodeModelCandidates` (extracted single-node
scan)
- `missingMediaScan`: `scanNodeMediaCandidates` (extracted single-node
scan)

### 9. Test infrastructure improvements

- `data-testid` on `RightSidePanel.vue` tabs (`panel-tab-{value}`)
- Error-related TestIds moved from `dialogs` to `errorsTab` namespace in
`selectors.ts`
- Removed unused `TestIdValue` type
- Extracted `cleanupFakeModel` to shared `ErrorsTabHelper.ts`
- Renamed `openErrorsTabViaSeeErrors` → `loadWorkflowAndOpenErrorsTab`
- Added `aria-label` to pencil edit button and subgraph toggle button

## Test plan

### Unit tests (41 new)

- Store functions: `addMissing*`, `removeMissing*ByNodeId`
- `executionErrorStore`: `surfaceMissing*` silent option
- Scan functions: muted/bypassed filtering, `scanNodeModelCandidates`,
`scanNodeMediaCandidates`
- `workflowService`: `showPendingWarnings` silent, `beforeLoadNewGraph`
caching

### E2E tests (17 new in `errorsTabModeAware.spec.ts`)

**Missing nodes**
- [x] Deleting a missing node removes its error from the errors tab
- [x] Undo after bypass restores error without showing overlay

**Missing models**
- [x] Loading a workflow with all nodes bypassed shows no errors
- [x] Bypassing a node hides its error, un-bypassing restores it
- [x] Deleting a node with missing model removes its error
- [x] Undo after bypass restores error without showing overlay
- [x] Pasting a node with missing model increases referencing node count
- [x] Pasting a bypassed node does not add a new error
- [x] Selecting a node filters errors tab to only that node

**Missing media**
- [x] Loading a workflow with all nodes bypassed shows no errors
- [x] Bypassing a node hides its error, un-bypassing restores it
- [x] Pasting a bypassed node does not add a new error
- [x] Selecting a node filters errors tab to only that node

**Subgraph**
- [x] Bypassing a subgraph hides interior errors, un-bypassing restores
them
- [x] Bypassing a node inside a subgraph hides its error, un-bypassing
restores it

**Workflow switching**
- [x] Does not resurface error overlay when switching back to workflow
with missing nodes
- [x] Restores missing nodes in errors tab when switching back to
workflow

# Screenshots


https://github.com/user-attachments/assets/e0a5bcb8-69ba-4120-ab7f-5c83e4cfc3c5



## Follow-up work

- Extract error-detection computed properties from `RightSidePanel.vue`
into a composable (e.g. `useErrorsTabVisibility`)

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: GitHub Action <action@github.com>
2026-04-13 12:51:19 +00:00
Alexander Brown
f90d6cf607 test: migrate 132 test files from @vue/test-utils to @testing-library/vue (#10965)
## Summary

Migrate 132 test files from `@vue/test-utils` (VTU) to
`@testing-library/vue` (VTL) with `@testing-library/user-event`,
adopting user-centric behavioral testing patterns across the codebase.

## Changes

- **What**: Systematic migration of component/unit tests from VTU's
`mount`/`wrapper` API to VTL's `render`/`screen`/`userEvent` API across
132 files in `src/`
- **Breaking**: None — test-only changes, no production code affected

### Migration breakdown

| Batch | Files | Description |
|-------|-------|-------------|
| 1 | 19 | Simple render/assert tests |
| 2A | 16 | Interactive tests with user events |
| 2B-1 | 14 | Interactive tests (continued) |
| 2B-2 | 32 | Interactive tests (continued) |
| 3A–3E | 51 | Complex tests (stores, composables, heavy mocking) |
| Lint fix | 7 | `await` on `fireEvent` calls for `no-floating-promises`
|
| Review fixes | 15 | Address CodeRabbit feedback (3 rounds) |

### Review feedback addressed

- Removed class-based assertions (`text-ellipsis`, `pr-3`, `.pi-save`,
`.skeleton`, `.bg-black\/15`, Tailwind utilities) in favor of
behavioral/accessible queries
- Added null guards before `querySelector` casts
- Added `expect(roots).toHaveLength(N)` guards before indexed NodeList
access
- Wrapped fake timer tests in `try/finally` for guaranteed cleanup
- Split double-render tests into focused single-render tests
- Replaced CSS class selectors with
`screen.getByText`/`screen.getByRole` queries
- Updated stubs to use semantic `role`/`aria-label` instead of CSS
classes
- Consolidated redundant edge-case tests
- Removed manual `document.body.appendChild` in favor of VTL container
management
- Used distinct mock return values to verify command wiring

### VTU holdouts (2 files)

These files intentionally retain `@vue/test-utils` because their
components use `<script setup>` without `defineExpose`, making internal
computed properties and methods inaccessible via VTL:

1. **`NodeWidgets.test.ts`** — partial VTU for `vm.processedWidgets`
2. **`WidgetSelectDropdown.test.ts`** — full VTU for heavy
`wrapper.vm.*` access

## Follow-up

Deferred items (`ComponentProps` typing, camelCase listener props)
tracked in #10966.

## Review Focus

- Test correctness: all migrated tests preserve original behavioral
coverage
- VTL idioms: proper use of `screen` queries, `userEvent`, and
accessibility-based selectors
- The 2 VTU holdout files are intentional, not oversights

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10965-test-migrate-132-test-files-from-vue-test-utils-to-testing-library-vue-33c6d73d36508199a6a7e513cf5d8296)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: Christian Byrne <cbyrne@comfy.org>
2026-04-08 19:21:42 -07:00
Alexander Brown
661e3d7949 test: migrate as unknown as to @total-typescript/shoehorn (#10761)
*PR Created by the Glary-Bot Agent*

---

## Summary

- Replace all `as unknown as Type` assertions in 59 unit test files with
type-safe `@total-typescript/shoehorn` functions
- Use `fromPartial<Type>()` for partial mock objects where deep-partial
type-checks (21 files)
- Use `fromAny<Type>()` for fundamentally incompatible types: null,
undefined, primitives, variables, class expressions, and mocks with
test-specific extra properties that `PartialDeepObject` rejects
(remaining files)
- All explicit type parameters preserved so TypeScript return types are
correct
- Browser test `.spec.ts` files excluded (shoehorn unavailable in
`page.evaluate` browser context)

## Verification

- `pnpm typecheck` 
- `pnpm lint` 
- `pnpm format` 
- Pre-commit hooks passed (format + oxlint + eslint + typecheck)
- Migrated test files verified passing (ran representative subset)
- No test behavior changes — only type assertion syntax changed
- No UI changes — screenshots not applicable

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10761-test-migrate-as-unknown-as-to-total-typescript-shoehorn-3336d73d365081f6b8adc44db5dcc380)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
Co-authored-by: Amp <amp@ampcode.com>
2026-03-30 19:20:18 -07:00
jaeone94
66daa6d645 refactor: error system cleanup — store separation, DDD fix, test improvements (#10302)
## Summary

Refactors the error system to improve separation of concerns, fix DDD
layer violations, and address code quality issues.

- Extract `missingNodesErrorStore` from `executionErrorStore`, removing
the delegation pattern that coupled missing-node logic into the
execution error store
- Extract `useNodeErrorFlagSync` composable for node error flag
reconciliation (previously inlined)
- Extract `useErrorClearingHooks` composable with explicit callback
cleanup on node removal
- Extract `useErrorActions` composable to deduplicate telemetry+command
patterns across error card components
- Move `getCnrIdFromNode`/`getCnrIdFromProperties` to
`platform/nodeReplacement` layer (DDD fix)
- Move `missingNodesErrorStore` to `platform/nodeReplacement` (DDD
alignment)
- Add unmount cancellation guard to `useErrorReport` async `onMounted`
- Return watch stop handle from `useNodeErrorFlagSync`
- Add `asyncResolvedIds` eviction on `missingNodesError` reset
- Add `console.warn` to silent catch blocks and empty array guard
- Hoist `useCommandStore` to setup scope, fix floating promises
- Add `data-testid` to error groups, image/video error spans, copy
button
- Update E2E tests to use scoped locators and testids
- Add unit tests for `onNodeRemoved` restoration and double-install
guard

Fixes #9875, Fixes #10027, Fixes #10033, Fixes #10085

## Test plan

- [x] Existing unit tests pass with updated imports and mocks
- [x] New unit tests for `useErrorClearingHooks` (callback restoration,
double-install guard)
- [x] E2E tests updated to use scoped locators and `data-testid`
- [ ] Manual: verify error tab shows runtime errors and missing nodes
correctly
- [ ] Manual: verify "Find on GitHub", "Copy", and "Get Help" buttons
work in error cards

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10302-refactor-error-system-cleanup-store-separation-DDD-fix-test-improvements-3286d73d365081838279d045b8dd957a)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-03-24 16:43:22 +09:00
Dante
cdf74c36f7 tool: add layer architecture boundary lint rule (#10109)
## Summary
- Add `import-x/no-restricted-paths` ESLint rule enforcing the `base →
platform → workbench → renderer` layer hierarchy
- Set to `error` with eslint-disable comments on existing violations
(~11 suppressions)
- Consolidate zone definitions using array syntax for `from`/`target`
- Add `layer-audit` Claude skill for auditing violations
- Fix knip false positive for `zod` dependency in ingest-types

## Context
Ref:
https://github.com/Comfy-Org/ComfyUI_frontend/pull/10021#discussion_r2939392141

The codebase is migrating toward a layered architecture. This adds
static enforcement so new violations are caught in PR CI.

### Layer rules
| Layer | Can import from |
|---|---|
| `base/` | nothing |
| `platform/` | `base/` |
| `workbench/` | `platform/`, `base/` |
| `renderer/` | `workbench/`, `platform/`, `base/` |

### Current violations (pre-existing, suppressed with eslint-disable)
| Direction | Count |
|---|---|
| base → platform | 2 |
| platform → workbench | 3 |
| platform → renderer | 5 |
| workbench → renderer | 1 |

## Test plan
- [x] `pnpm lint` passes (0 errors, 0 warnings)
- [x] `pnpm typecheck` passes
- [x] `pnpm knip` passes
- [ ] CI green

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-17 02:08:25 -07:00
Christian Byrne
725a0a2b89 fix: remove timeouts from error toasts so they persist until dismissed (#9543)
## Summary

Remove `life` (timeout) property from all error-severity toast calls so
they persist until manually dismissed, preventing users from missing
important error messages.

## Changes

- **What**: Removed `life` property from 86 error toast calls across 46
files. Error toasts now use PrimeVue's default behavior (no
auto-dismiss). Non-error toasts (success, warn, info) are unchanged.
- Also fixed a pre-existing lint issue in `TaskListPanel.vue` (`import {
t } from '@/i18n'` → `useI18n()`)

## Review Focus

- One conditional toast in `useMediaAssetActions.ts` intentionally keeps
`life` because its severity alternates between `warn` and `error`

Fixes
https://www.notion.so/comfy-org/Implement-Remove-timeouts-for-all-error-toasts-31b6d73d365081cead54fddc77ae7c3d

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9543-fix-remove-timeouts-from-error-toasts-so-they-persist-until-dismissed-31c6d73d365081fa8d30f6366e9bfe38)
by [Unito](https://www.unito.io)
2026-03-07 15:08:13 -08:00
Christian Byrne
ef4e4a69d5 fix: enable enforce-consistent-class-order tailwind lint rule (#9428)
## Summary

Enable `better-tailwindcss/enforce-consistent-class-order` lint rule and
auto-fix all 1027 violations across 263 files. Stacked on #9427.

## Changes

- **What**: Sort Tailwind classes into consistent order via `eslint
--fix`
- Enable `enforce-consistent-class-order` as `'error'` in eslint config
- Purely cosmetic reordering — no behavioral or visual changes

## Review Focus

Mechanical auto-fix PR — all changes are class reordering only. This is
the largest diff but lowest risk since it changes no class names, only
their order.

**Stack:** #9417#9427 → **this PR**

Fixes #9300 (partial — 3 of 3 rules)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9428-fix-enable-enforce-consistent-class-order-tailwind-lint-rule-31a6d73d3650811c9065f5178ba3e724)
by [Unito](https://www.unito.io)
2026-03-05 17:24:34 -08:00
Christian Byrne
1221756e05 fix: enable enforce-canonical-classes tailwind lint rule (#9427)
## Summary

Enable `better-tailwindcss/enforce-canonical-classes` lint rule and
auto-fix all 611 violations across 173 files. Stacked on #9417.

## Changes

- **What**: Simplify Tailwind classes to canonical forms via `eslint
--fix`:
  - `h-X w-X` → `size-X`
  - `overflow-x-hidden overflow-y-hidden` → `overflow-hidden`
  - and other canonical simplifications
- Enable `enforce-canonical-classes` as `'error'` in eslint config

## Review Focus

Mechanical auto-fix PR — all changes produced by `eslint --fix`. No
visual or behavioral changes; canonical forms are functionally
identical.

**Stack:** #9417 → **this PR** → PR 3 (class order)

Fixes #9300 (partial — 2 of 3 rules)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9427-fix-enable-enforce-canonical-classes-tailwind-lint-rule-31a6d73d365081a49340d7d4640ede45)
by [Unito](https://www.unito.io)
2026-03-05 17:07:46 -08:00
jaeone94
a0e518aa98 refactor(node-replacement): reorganize domain components and expand comprehensive test suite (#9301)
## Summary

Resolves six open issues by reorganizing node replacement components
into a domain-driven folder structure, refactoring event handling to
follow the emit pattern, and adding comprehensive test coverage across
all affected modules.

## Changes

- **What**:
- Moved `SwapNodeGroupRow.vue` and `SwapNodesCard.vue` from
`src/components/rightSidePanel/errors/` to
`src/platform/nodeReplacement/components/` (Issues #9255)
- Moved `useMissingNodeScan.ts` from `src/composables/` to
`src/platform/nodeReplacement/missingNodeScan.ts`, renamed to reflect it
is a plain function not a Vue composable (Issues #9254)
- Refactored `SwapNodeGroupRow.vue` to emit a `'replace'` event instead
of calling `useNodeReplacement()` and `useExecutionErrorStore()`
directly; replacement logic now handled in `TabErrors.vue` (Issue #9267)
- Added unit tests for `removeMissingNodesByType`
(`executionErrorStore.test.ts`), `scanMissingNodes`
(`missingNodeScan.test.ts`), and `swapNodeGroups` computed
(`swapNodeGroups.test.ts`, `useErrorGroups.test.ts`) (Issue #9270)
- Added placeholder detection tests covering unregistered-type detection
when `has_errors` is false, and exclusion of registered types
(`useNodeReplacement.test.ts`) (Issue #9271)
- Added component tests for `MissingNodeCard` and `MissingPackGroupRow`
covering rendering, expand/collapse, events, install states, and edge
cases (Issue #9231)
- Added component tests for `SwapNodeGroupRow` and `SwapNodesCard`
(Issues #9255, #9267)

## Additional Changes (Post-Review)

- **Edge case guard in placeholder detection**
(`useNodeReplacement.ts`): When `last_serialization.type` is absent (old
serialization format), the predicate falls back to `n.type`, which the
app may have already run through `sanitizeNodeName` — stripping HTML
special characters (`& < > " ' \` =`). In that case, a `Set.has()`
lookup against the original unsanitized type name would silently miss,
causing replacement to be skipped.

Fixed by including sanitized variants of each target type in the
`targetTypes` Set at construction time. For the overwhelmingly common
case (no special characters in type names), the Set deduplicates the
entries and runtime behavior is identical to before.

A regression test was added to cover the specific scenario:
`last_serialization.type` absent + live `n.type` already sanitized.

## Review Focus

- `TabErrors.vue`: confirm the new `@replace` event handler correctly
replaces nodes and removes them from missing nodes list (mirrors the old
inline logic in `SwapNodeGroupRow`)
- `missingNodeScan.ts`: filename/export name change from
`useMissingNodeScan` — verify all call sites updated via `app.ts`
- Test mocking strategy: module-level `vi.mock()` factories use closures
over `ref`/plain objects to allow per-test overrides without global
mutable state

- Fixes #9231
- Fixes #9254
- Fixes #9255
- Fixes #9267
- Fixes #9270
- Fixes #9271
2026-02-28 06:17:30 -08:00
jaeone94
45f112e226 fix: node replacement fails after execution and modal sync (#9269)
## Summary

Fixes two bugs in the node replacement flow: placeholder detection
failing after workflow execution or pack reinstallation, and missing UI
sync in the Errors Tab when replacements are applied from the modal
dialog.

## Changes

- **Placeholder detection**: Node placeholder detection now matches
against `targetTypes` (derived from the replaceable node list built at
workflow load time) instead of relying on `has_errors` flag or
`registered_node_types` lookup. This ensures replacement works reliably
after execution (where `has_errors` gets cleared) and after pack
reinstallation (where the type becomes registered).
- **Modal → Errors Tab sync**: Added
`executionErrorStore.removeMissingNodesByType()` call in
`MissingNodesContent.vue` after replacement, so the Errors Tab reflects
changes immediately without requiring a page reload.

## Review Focus

- `collectAllNodes` predicate change in `useNodeReplacement.ts`: now
uses `targetTypes.has(originalType)` to find nodes by their original
serialized type. This is independent of runtime state like `has_errors`
or `registered_node_types`.
- `executionErrorStore.removeMissingNodesByType` call timing in
`MissingNodesContent.vue` — runs synchronously after
`replaceNodesInPlace` resolves, before auto-close logic.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9269-fix-node-replacement-fails-after-execution-and-modal-sync-3146d73d365081218398c961639b450f)
by [Unito](https://www.unito.io)
2026-02-28 04:05:58 -08:00
jaeone94
1c3984a178 feat: add node replacement UI to Errors Tab (#9253)
## 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)
2026-02-26 17:37:48 -08:00
Jin Yi
2b69d7b49c [bugfix] Fix node replacements not loading due to feature flag timing (#9037)
## Summary
- Node replacements were never loaded because
`useNodeReplacementStore().load()` was called before `api.init()`,
meaning `serverFeatureFlags` was always empty at that point
- Dispatch `feature_flags` as a custom event from `api.ts` and trigger
`load()` in response within `addApiUpdateHandlers()`

## Changes
- **`api.ts`**: Dispatch `feature_flags` custom event after storing
server feature flags (already typed in `BackendApiCalls`)
- **`app.ts`**: Replace eager `load()` call with `feature_flags` event
listener inside `addApiUpdateHandlers()`, consistent with other API
event handlers
- **`nodeReplacementStore.ts`**: Use `api.getServerFeature()` directly
instead of `useFeatureFlags` composable; remove side effects from store
setup
- **`nodeReplacementStore.test.ts`**: Update mocks to match new
`api.getServerFeature` usage

## Review Focus
- Initialization ordering: listener registered before `api.init()`
ensures no missed events

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9037-bugfix-Fix-node-replacements-not-loading-due-to-feature-flag-timing-30e6d73d36508107ae2cd72e83c01e1a)
by [Unito](https://www.unito.io)
2026-02-20 19:16:01 -08:00
Christian Byrne
e83e396c09 feat: gate node replacement loading on server feature flag (#8750)
## Summary

Gates the node replacement store's `load()` call behind the
`node_replacements` server feature flag, so the frontend only calls
`/api/node_replacements` when the backend advertises support.

## Changes

- Added `NODE_REPLACEMENTS = 'node_replacements'` to `ServerFeatureFlag`
enum
- Added `nodeReplacementsEnabled` getter to `useFeatureFlags()`
- Added `api.serverSupportsFeature('node_replacements')` guard in
`useNodeReplacementStore.load()`

## Context

Without this guard, the frontend would attempt to fetch node
replacements from backends that don't support the endpoint, causing 404
errors.

Companion backend PR: https://github.com/Comfy-Org/ComfyUI/pull/12362

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8750-feat-gate-node-replacement-loading-on-server-feature-flag-3026d73d365081ec9246d77ad88f5bdc)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Jin Yi <jin12cc@gmail.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: Claude <noreply@anthropic.com>
2026-02-17 11:39:07 -08:00
Jin Yi
efe78b799f [feat] Node replacement UI (#8604)
## Summary
Add node replacement UI to the missing nodes dialog. Users can select
and replace deprecated/missing nodes with compatible alternatives
directly from the dialog.

## Changes
- Classify missing nodes into **Replaceable** (quick fix) and **Install
Required** sections
- Add select-all checkbox + per-node checkboxes for batch replacement
- `useNodeReplacement` composable handles in-place node replacement on
the graph:
  - Simple replacement (configure+copy) for nodes without mapping
  - Input/output connection remapping for nodes with mapping
  - Widget value transfer via `old_widget_ids`
  - Dot-notation input handling for Autogrow/DynamicCombo
  - Undo/redo support via `changeTracker` (try/finally)
  - Title and properties preservation
- Footer UX: "Skip for Now" button when all nodes are replaceable (cloud
+ OSS)
- Auto-close dialog when all replaceable nodes are replaced and no
non-replaceable remain
- Settings navigation link from "Don't show again" checkbox
- 505-line unit test suite for `useNodeReplacement`

## Review Focus
- `useNodeReplacement.ts` — core graph manipulation logic
- `MissingNodesContent.vue` — checkbox selection state management
- `MissingNodesFooter.vue` — conditional button rendering (cloud vs OSS
vs all-replaceable)


[screen-capture.webm](https://github.com/user-attachments/assets/7dae891c-926c-4f26-987f-9637c4a2ca16)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8604-feat-Node-replacement-UI-2fd6d73d36508148a371dabb8f4115af)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
2026-02-16 23:33:41 -08:00
Jin Yi
96b9e886ea feat: classify missing nodes by replacement availability (#8483)
## Summary
- Extend `MissingNodeType` with `isReplaceable` and `replacement` fields
- Classify missing nodes by checking
`nodeReplacementStore.getReplacementFor()` during graph load
- Wrap hardcoded node patches (T2IAdapterLoader, ConditioningAverage,
etc.) in `if (!isEnabled)` guard so they only run when node replacement
setting is disabled
- Change `useNodeReplacementStore().load()` from fire-and-forget
(`void`) to `await` so replacement data is available before missing node
detection
- Fix guard condition order in `nodeReplacementStore.load()`: check
`isEnabled` before `isLoaded`
- Align `InputMap` types with actual API response (flat
`old_id`/`set_value` fields instead of nested `assign` wrapper)

## Test plan
- [x] Load workflow with deprecated nodes (T2IAdapterLoader,
Load3DAnimation, SDV_img2vid_Conditioning)
- [x] Verify missing nodes are classified with `isReplaceable: true` and
`replacement` object
- [x] Verify hardcoded patches only run when node replacement setting is
OFF
- [x] Verify `nodeReplacementStore.load()` is not called when setting is
disabled
- [x] Unit tests pass (`nodeReplacementStore.test.ts` - 16 tests)
- [x] Typecheck passes

🤖 Generated with [Claude Code](https://claude.ai/code)
2026-02-15 11:26:46 +09:00
Jin Yi
1ca6e57ac4 fix: skip node replacement API call when feature is disabled (#8618) 2026-02-05 16:37:18 +09:00
Jin Yi
5847071bc1 [feat] Add node replacement store and types (#8364)
## Summary
Add infrastructure for automatic node replacement feature that allows
missing/deprecated nodes to be replaced with their newer equivalents.

## Changes
- **Types**: `NodeReplacement`, `NodeReplacementResponse` types matching
backend API spec (PR #12014)
- **Service**: `fetchNodeReplacements()` API wrapper
- **Store**: `useNodeReplacementStore` with `getReplacementFor()`,
`hasReplacement()`, `isEnabled()`
- **Setting**: `Comfy.NodeReplacement.Enabled` toggle (experimental)
- **Tests**: 11 unit tests covering store functionality

## Related
- Backend PR: Comfy-Org/ComfyUI#12014

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8364-feat-Add-node-replacement-store-and-types-2f66d73d3650816bb771c9cc6a8e1774)
by [Unito](https://www.unito.io)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Automatic node replacement is now available, allowing missing nodes to
be replaced with their newer equivalents when replacement mappings
exist.
* Added "Enable automatic node replacement" experimental setting in
Workflow preferences (enabled by default).
  * Replacement data is loaded during app initialization.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-02-03 14:15:05 +09:00