mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-27 16:34:40 +00:00
b4d209b5f649f8fe2d883ddc4eee264b5efcaf13
8 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
b4d209b5f6 |
feat: refresh missing models through pipeline (#11661)
## Summary Follow-up to the closed earlier attempt in #11646. This PR keeps the same user-facing goal, but changes the implementation to reuse the existing missing model pipeline for refresh instead of maintaining a separate candidate-only recheck path. Adds a missing model refresh action in the Errors tab by reusing the existing missing model pipeline, so users can re-check models after downloading or manually placing files without reloading the workflow. ## Changes - **What**: - Adds `app.refreshMissingModels()` as a reusable refresh entry point for the current root graph. - Splits node definition reloading into `app.reloadNodeDefs()` so missing-model refresh can pull fresh `object_info` without showing the generic combo refresh success flow. - Reuses the existing missing model pipeline instead of adding a separate candidate-only checker. The refresh path serializes the current graph, reuses active workflow model metadata when available, falls back to current missing-model metadata, and then reruns the same candidate discovery/enrichment/surfacing flow used during workflow load. - Adds missing model refresh state and error handling to `missingModelStore`. - Adds a Refresh button next to Download all in the missing model card action bar. - Moves Download all from the Errors tab header into the missing model card, so the Download all and Refresh actions render or hide together. - Changes Download all visibility from “more than one downloadable model” to “at least one downloadable model.” - Keeps the action bar hidden when there are no downloadable missing models; Cloud still does not render this action area. - Normalizes active workflow `pendingWarnings` updates so resolved missing model warnings do not get revived by stale empty warning objects. - Adds test IDs and coverage for the new action bar, refresh state, refresh delegation, pending warning sync, and E2E refresh behavior. - **Breaking**: None. - **Dependencies**: None. ## Review Focus The main design choice is intentionally reusing the missing model pipeline for refresh instead of implementing a smaller candidate-only recheck. The earlier candidate-only approach was cheaper, but it created a separate source of truth for missing-model resolution and made edge cases harder to reason about. In particular, it could diverge from the behavior used when a workflow is loaded, and it did not naturally handle the case where a model becomes missing after the workflow is already open. This version pays the cost of refreshing node definitions and rerunning the missing-model scan for the current graph, but keeps the refresh behavior aligned with workflow load semantics. Expected behavior by environment: - OSS browser: - The action bar appears when at least one missing model has a downloadable URL and directory. - Download all uses the existing browser download path. - Refresh reloads `object_info`, refreshes node definitions/combo values, reruns missing-model detection for the current graph, and clears the error if the selected model is now available. - OSS desktop: - The same action bar appears under the same downloadable-model condition. - Download all uses the existing Electron DownloadManager path. - Refresh uses the same missing-model pipeline as browser, so manually placed files or desktop-downloaded files can be rechecked without reloading the workflow. - Cloud: - The action bar remains hidden because model download/import is not supported in this section for Cloud. A few boundaries are intentional: - This PR does not add automatic filesystem watching. Browser OSS cannot reliably observe local model folder changes, so the user-triggered Refresh button remains the cross-environment mechanism. - This PR does not redesign the public `refreshComboInNodes` API beyond extracting `reloadNodeDefs()` for reuse. Further cleanup of toast behavior or a more explicit object-info reload API can be follow-up work. - This PR keeps refresh scoped to missing-model validation; missing media and missing nodes continue to use their existing flows. Linear: FE-417 ## Screenshots (if applicable) https://github.com/user-attachments/assets/2e02799f-1374-4377-b7b3-172241517772 ## Validation - `pnpm format` - `pnpm lint` (passes; existing unrelated warning remains in `src/platform/workspace/composables/useWorkspaceBilling.test.ts`) - `pnpm typecheck` - `pnpm test:unit` - `pnpm test:browser:local -- --project=chromium browser_tests/tests/propertiesPanel/errorsTabMissingModels.spec.ts` - `pnpm build` - `NX_SKIP_NX_CACHE=true DISTRIBUTION=desktop USE_PROD_CONFIG=true NODE_OPTIONS='--max-old-space-size=8192' pnpm exec nx build` - Manual desktop verification through `~/Projects/desktop` after copying the desktop build into `assets/ComfyUI/web_custom_versions/desktop_app`: - confirmed the FE bundle is built with `DISTRIBUTION = "desktop"` - confirmed missing model Download uses the desktop download path instead of browser download - confirmed Refresh can clear the missing model error after the model is available - Push hook: `pnpm knip --cache` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11661-feat-refresh-missing-models-through-pipeline-34f6d73d3650811488defee54a7a6667) by [Unito](https://www.unito.io) |
||
|
|
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> |
||
|
|
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> |
||
|
|
0106372201 | feat: enhance error tab with full error report and GitHub search (#10135) | ||
|
|
80fe51bb8c |
feat: show missing node packs in Errors Tab with install support (#9213)
## Summary Surfaces missing node pack information in the Errors Tab, grouped by registry pack, with one-click install support via ComfyUI Manager. ## Changes - **What**: Errors Tab now groups missing nodes by their registry pack and shows a `MissingPackGroupRow` with pack name, node/pack counts, and an Install button that triggers Manager installation. A `MissingNodeCard` shows individual unresolvable nodes that have no associated pack. `useErrorGroups` was extended to resolve missing node types to their registry packs using the `/api/workflow/missing_nodes` endpoint. `executionErrorStore` was refactored to track missing node types separately from execution errors and expose them reactively. - **Breaking**: None ## Review Focus - `useErrorGroups.ts` — the new `resolveMissingNodePacks` logic fetches pack metadata and maps node types to pack IDs; edge cases around partial resolution (some nodes have a pack, some don't) produce both `MissingPackGroupRow` and `MissingNodeCard` entries - `executionErrorStore.ts` — the store now separates `missingNodeTypes` state from `errors`; the deferred-warnings path in `app.ts` now calls `setMissingNodeTypes` so the Errors Tab is populated even when a workflow loads without executing ## Screenshots (if applicable) https://github.com/user-attachments/assets/97f8d009-0cac-4739-8740-fd3333b5a85b ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9213-feat-show-missing-node-packs-in-Errors-Tab-with-install-support-3126d73d36508197bc4bf8ebfd2125c8) by [Unito](https://www.unito.io) |
||
|
|
8aa4e36fd5 |
[refactor] Extract executionErrorStore from executionStore (#9060)
## Summary Extracts error-related state and logic from `executionStore` into a dedicated `executionErrorStore` for better separation of concerns. ## Changes - **New store**: `executionErrorStore` with all error state (`lastNodeErrors`, `lastExecutionError`, `lastPromptError`), computed properties (`hasAnyError`, `totalErrorCount`, `activeGraphErrorNodeIds`), and UI state (`isErrorOverlayOpen`, `showErrorOverlay`, `dismissErrorOverlay`) - **Moved util**: `executionIdToNodeLocatorId` extracted to `graphTraversalUtil`, reusing `traverseSubgraphPath` and accepting `rootGraph` as parameter - **Updated consumers**: 12 files updated to import from `executionErrorStore` - **Backward compat**: Deprecated getters retained in `ComfyApp` for extension compatibility ## Review Focus - Deprecated getters in `app.ts` — can be removed in a future breaking-change PR once extension authors migrate ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9060-refactor-Extract-executionErrorStore-from-executionStore-30e6d73d36508101973de835ab6b199f) by [Unito](https://www.unito.io) |
||
|
|
46c40c755e |
feat: node-specific error tab with selection-aware grouping and error overlay (#8956)
## Summary Enhances the error panel with node-specific views: single-node selection shows errors grouped by message in compact mode, container nodes (subgraph/group) expose child errors via a badge and "See Error" button, and a floating ErrorOverlay appears after execution failure with a deduplicated summary and quick navigation to the errors tab. ## Changes - **Consolidate error tab**: Remove `TabError.vue`; merge all error display into `TabErrors.vue` and drop the separate `error` tab type from `rightSidePanelStore` - **Selection-aware grouping**: Single-node selection regroups errors by message (not `class_type`) and renders `ErrorNodeCard` in compact mode - **Container node support**: Detect child-node errors in subgraph/group nodes via execution ID prefix matching; show error badge and "See Error" button in `SectionWidgets` - **ErrorOverlay**: New floating card shown after execution failure with deduplicated error messages, "Dismiss" and "See Errors" actions; `isErrorOverlayOpen` / `showErrorOverlay` / `dismissErrorOverlay` added to `executionStore` - **Refactor**: Centralize error ID collection in `executionStore` (`allErrorExecutionIds`, `hasInternalErrorForNode`); split `errorGroups` into `allErrorGroups` (unfiltered) and `tabErrorGroups` (selection-filtered); move `ErrorOverlay` business logic into `useErrorGroups` ## Review Focus - `useErrorGroups.ts`: split into `allErrorGroups` / `tabErrorGroups` and the new `filterBySelection` parameter flow - `executionStore.ts`: `hasInternalErrorForNode` helper and `allErrorExecutionIds` computed - `ErrorOverlay.vue`: integration with `executionStore` overlay state and `useErrorGroups` ## Screenshots <img width="853" height="461" alt="image" src="https://github.com/user-attachments/assets/a49ab620-4209-4ae7-b547-fba13da0c633" /> <img width="854" height="203" alt="image" src="https://github.com/user-attachments/assets/c119da54-cd78-4e7a-8b7a-456cfd348f1d" /> <img width="497" height="361" alt="image" src="https://github.com/user-attachments/assets/74b16161-cf45-454b-ae60-24922fe36931" /> --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: github-actions <github-actions@github.com> |
||
|
|
1349fffbce |
Feat/errors tab panel (#8807)
## Summary Add a dedicated **Errors tab** to the Right Side Panel that displays prompt-level, node validation, and runtime execution errors in a unified, searchable, grouped view — replacing the need to rely solely on modal dialogs for error inspection. ## Changes - **What**: - **New components** (`errors/` directory): - `TabErrors.vue` — Main error tab with search, grouping by class type, and canvas navigation (locate node / enter subgraph). - `ErrorNodeCard.vue` — Renders a single error card with node ID badge, title, action buttons, and error details. - `types.ts` — Shared type definitions (`ErrorItem`, `ErrorCardData`, `ErrorGroup`). - **`executionStore.ts`** — Added `PromptError` interface, `lastPromptError` ref and `hasAnyError` computed getter. Clears `lastPromptError` alongside existing error state on execution start and graph clear. - **`rightSidePanelStore.ts`** — Registered `'errors'` as a valid tab value. - **`app.ts`** — On prompt submission failure (`PromptExecutionError`), stores prompt-level errors (when no node errors exist) into `lastPromptError`. On both runtime execution error and prompt error, deselects all nodes and opens the errors tab automatically. - **`RightSidePanel.vue`** — Shows the `'errors'` tab (with ⚠ icon) when errors exist and no node is selected. Routes to `TabErrors` component. - **`TopMenuSection.vue`** — Highlights the action bar with a red border when any error exists, using `hasAnyError`. - **`SectionWidgets.vue`** — Detects per-node errors by matching execution IDs to graph node IDs. Shows an error icon (⚠) and "See Error" button that navigates to the errors tab. - **`en/main.json`** — Added i18n keys: `errors`, `noErrors`, `enterSubgraph`, `seeError`, `promptErrors.*`, and `errorHelp*`. - **Testing**: 6 unit tests (`TabErrors.test.ts`) covering prompt/node/runtime errors, search filtering, and clipboard copy. - **Storybook**: 7 stories (`ErrorNodeCard.stories.ts`) for badge visibility, subgraph buttons, multiple errors, runtime tracebacks, and prompt-only errors. - **Breaking**: None - **Dependencies**: None — uses only existing project dependencies (`vue-i18n`, `pinia`, `primevue`) ## Related Work > **Note**: Upstream PR #8603 (`New bottom button and badges`) introduced a separate `TabError.vue` (singular) that shows per-node errors when a specific node is selected. Our `TabErrors.vue` (plural) provides the **global error overview** — a different scope. The two tabs coexist: > - `'error'` (singular) → appears when a node with errors is selected → shows only that node's errors > - `'errors'` (plural) → appears when no node is selected and errors exist → shows all errors grouped by class type > > A future consolidation of these two tabs may be desirable after design review. ## Architecture ``` executionStore ├── lastPromptError: PromptError | null ← NEW (prompt-level errors without node IDs) ├── lastNodeErrors: Record<string, NodeError> (existing) ├── lastExecutionError: ExecutionError (existing) └── hasAnyError: ComputedRef<boolean> ← NEW (centralized error detection) TabErrors.vue (errors tab - global view) ├── errorGroups: ComputedRef<ErrorGroup[]> ← normalizes all 3 error sources ├── filteredGroups ← search-filtered view ├── locateNode() ← pan canvas to node ├── enterSubgraph() ← navigate into subgraph └── ErrorNodeCard.vue ← per-node card with copy/locate actions types.ts ├── ErrorItem { message, details?, isRuntimeError? } ├── ErrorCardData { id, title, nodeId?, errors[] } └── ErrorGroup { title, cards[], priority } ``` ## Review Focus 1. **Error normalization logic** (`TabErrors.vue` L75–150): Three different error sources (prompt, node validation, runtime) are normalized into a common `ErrorGroup → ErrorCardData → ErrorItem` hierarchy. Edge cases to verify: - Prompt errors with known vs unknown types (known types use localized descriptions) - Multiple errors on the same node (grouped into one card) - Runtime errors with long tracebacks (capped height with scroll) 2. **Canvas navigation** (`TabErrors.vue` L210–250): The `locateNode` and `enterSubgraph` functions navigate to potentially nested subgraphs. The double `requestAnimationFrame` is required due to LiteGraph's asynchronous subgraph switching — worth verifying this timing is sufficient. 3. **Store getter consolidation**: `hasAnyError` replaces duplicated logic in `TopMenuSection` and `RightSidePanel`. Confirm that the reactive dependency chain works correctly (it depends on 3 separate refs). 4. **Coexistence with upstream `TabError.vue`**: The singular `'error'` tab (upstream, PR #8603) and our plural `'errors'` tab serve different purposes but share similar naming. Consider whether a unified approach is preferred. ## Test Results ``` ✓ renders "no errors" state when store is empty ✓ renders prompt-level errors (Group title = error message) ✓ renders node validation errors grouped by class_type ✓ renders runtime execution errors from WebSocket ✓ filters errors based on search query ✓ calls copyToClipboard when copy button is clicked Test Files 1 passed (1) Tests 6 passed (6) ``` ## Screenshots (if applicable) <img width="1238" height="1914" alt="image" src="https://github.com/user-attachments/assets/ec39b872-cca1-4076-8795-8bc7c05dc665" /> <img width="669" height="1028" alt="image" src="https://github.com/user-attachments/assets/bdcaa82a-34b0-46a5-a08f-14950c5a479b" /> <img width="644" height="1005" alt="image" src="https://github.com/user-attachments/assets/ffef38c6-8f42-4c01-a0de-11709d54b638" /> <img width="672" height="505" alt="image" src="https://github.com/user-attachments/assets/5cff7f57-8d79-4808-a71e-9ad05bab6e17" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8807-Feat-errors-tab-panel-3046d73d36508127981ac670a70da467) by [Unito](https://www.unito.io) |