mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-05 13:41:59 +00:00
1be1ac71f4e75568e59a831a4e28c04c248bd60c
6 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
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> |
||
|
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> |
||
|
jaeone94
|
a321d66583 |
refactor: remove legacy missing nodes dialog (#10102)
## Summary - Remove the legacy missing nodes modal dialog and migrate all functionality to the existing Error Overlay / TabErrors system - Migrate core node version warning from `MissingCoreNodesMessage.vue` to `MissingNodeCard` in the errors tab - Remove `Comfy.Workflow.ShowMissingNodesWarning` setting (errors tab always surfaces missing nodes) - Delete 6 legacy files: `useMissingNodesDialog.ts`, `MissingNodesContent.vue`, `MissingNodesFooter.vue`, `MissingNodesHeader.vue`, `MissingCoreNodesMessage.vue` and its test - Rename `showMissingNodesDialog`/`showMissingModelsDialog` params to `showMissingNodes`/`showMissingModels` - Add `errorOverlay` and `missingNodeCard` to centralized `TestIds` - Migrate all E2E tests from legacy dialog selectors to error overlay testIds - Add new E2E test: MissingNodeCard visible via "See Errors" button flow - Add new E2E test: subgraph missing node type verified by expanding pack row - Add `surfaceMissingNodes` unit tests to `executionErrorStore` - Guard `semver.compare` against invalid version strings - Add `role="alert"`, `aria-hidden` for accessibility - Use reactive props destructuring in `MissingNodeCard` and `MissingPackGroupRow` **Net change: -669 lines** (19 files, +323 / -992) <img width="733" height="579" alt="image" src="https://github.com/user-attachments/assets/c497809d-b176-43bf-9872-34bd74c6ea0d" /> ## Test plan - [x] Unit tests: MissingNodeCard core node warning (7 tests) - [x] Unit tests: surfaceMissingNodes (4 tests) - [x] Unit tests: workflowService showPendingWarnings (updated) - [x] E2E: Error overlay visible on missing nodes workflow - [x] E2E: Error overlay visible on subgraph missing nodes - [x] E2E: MissingNodeCard visible via See Errors button - [x] E2E: Subgraph node type visible after expanding pack row - [x] E2E: Error overlay does not resurface on undo/redo - [x] E2E: Error overlay does not reappear on workflow tab switch - [x] Typecheck, lint, knip all passing ## Related issues - Closes #9923 (partially — `errorOverlay` and `missingNodeCard` added to TestIds) - References #10027 (mock hoisting inconsistency) - References #10033 (i18n-based test selectors) - References #10085 (DDD layer violation + focusedErrorNodeId) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10102-refactor-remove-legacy-missing-nodes-dialog-3256d73d365081c194d2e90bc6401846) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
31a33a0ba2 |
feat: auto-resolve simple validation errors on widget change and slot connection (#9464)
## Summary Automatically clears transient validation errors (`value_bigger_than_max`, `value_smaller_than_min`, `value_not_in_list`, `required_input_missing`) when the user modifies a widget value or connects an input slot, so resolved errors don't linger in the error panel. Also clears missing model state when the user changes a combo widget value. ## Changes - **`useNodeErrorAutoResolve` composable**: watches widget changes and slot connections, clears matching errors via `executionErrorStore` - **`executionErrorStore`**: adds `clearSimpleNodeErrors` and `clearSimpleWidgetErrorIfValid` with granular per-slot error removal - **`executionErrorUtil`**: adds `isValueStillOutOfRange` to prevent premature clearing when a new value still violates the constraint - **`graphTraversalUtil`**: adds `getExecutionIdFromNodeData` for subgraph-aware execution ID resolution - **`GraphCanvas.vue`**: fixes subgraph error key lookup by using `getExecutionIdByNode` instead of raw `node.id` - **`NodeWidgets.vue`**: wires up the new composable to the widget layer - **`missingModelStore`**: adds `removeMissingModelByWidget` to clear missing model state on widget value change - **`useGraphNodeManager`**: registers composable per node - **Tests**: 126 new unit tests covering error clearing, range validation, and graph traversal edge cases ## Screenshots https://github.com/user-attachments/assets/515ea811-ff84-482a-a866-a17e5c779c39 https://github.com/user-attachments/assets/a2b30f02-4929-4537-952c-a0febe20f02e ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9464-feat-auto-resolve-simple-validation-errors-on-widget-change-and-slot-connection-31b6d73d3650816b8afdc34f4b40295a) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
jaeone94
|
2f7f3c4e56 |
[feat] Surface missing models in Errors tab (Cloud) (#9743)
## Summary When a workflow is loaded with missing models, users currently have no way to identify or resolve them from within the UI. This PR adds a full missing-model detection and resolution pipeline that surfaces missing models in the Errors tab, allowing users to install or import them without leaving the editor. ## Changes ### Missing Model Detection - Scan all COMBO widgets across root graph and subgraphs for model-like filenames during workflow load - Enrich candidates with embedded workflow metadata (url, hash, directory) when available - Verify asset-supported candidates against the asset store asynchronously to confirm installation status - Propagate missing model state to `executionErrorStore` alongside existing node/prompt errors ### Errors Tab UI — Model Resolution - Group missing models by directory (e.g. `checkpoints`, `loras`, `vae`) with collapsible category cards - Each model row displays: - Model name with copy-to-clipboard button - Expandable list of referencing nodes with locate-on-canvas button - **Library selector**: Pick an alternative from the user's existing models to substitute the missing model with one click - **URL import**: Paste a Civitai or HuggingFace URL to import a model directly; debounced metadata fetch shows filename and file size before confirming; type-mismatch warnings (e.g. importing a LoRA into checkpoints directory) are surfaced with an "Import Anyway" option - **Upgrade prompt**: In cloud environment, free-tier subscribers are shown an upgrade modal when attempting URL import - Separate "Import Not Supported" section for custom-node models that cannot be auto-resolved - Status card with live download progress, completion, failure, and category-mismatch states ### Canvas Integration - Highlight nodes and widgets that reference missing models with error indicators - Propagate missing-model badges through subgraph containers so issues are visible at every graph level ### Code Cleanup - Simplify `surfacePendingWarnings` in workflowService, remove stale widget-detected model merging logic - Add `flattenWorkflowNodes` utility to workflowSchema for traversing nested subgraph structures - Extract `MissingModelUrlInput`, `MissingModelLibrarySelect`, `MissingModelStatusCard` as focused single-responsibility components ## Testing - Unit tests for scan pipeline (`missingModelScan.test.ts`): enrichment, skip-installed, subgraph flattening - Unit tests for store (`missingModelStore.test.ts`): state management, removal helpers - Unit tests for interactions (`useMissingModelInteractions.test.ts`): combo select, URL input, import flow, library confirm - Component tests for `MissingModelCard` and error grouping (`useErrorGroups.test.ts`) - Updated `workflowService.test.ts` and `workflowSchema.test.ts` for new logic ## Review Focus - Missing model scan + enrichment pipeline in `missingModelScan.ts` - Interaction composable `useMissingModelInteractions.ts` — URL metadata fetch, library install, upload fallback - Store integration and canvas-level error propagation ## Screenshots https://github.com/user-attachments/assets/339a6d5b-93a3-43cd-98dd-0fb00681b66f ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9743-feat-Surface-missing-models-in-Errors-tab-Cloud-3206d73d365081678326d3a16c2165d8) by [Unito](https://www.unito.io) |
||
|
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 |