Comfy-Org/ComfyUI_frontend
1
0
Fork 0
mirror of https://github.com/Comfy-Org/ComfyUI_frontend.git synced 2026-05-25 23:25:02 +00:00
Code Issues Packages Projects Releases Wiki Activity
8,096 Commits 1,371 Branches 660 Tags
fd67f26a9143a47cd2e94b2044ce62971ad733bd
Commit Graph

10 Commits

Author SHA1 Message Date
jaeone94
fd67f26a91 Clarify validation catalog rule overrides 2026-05-25 01:13:56 +09:00
jaeone94
3bc0b154b2 Refine validation error catalog messages 2026-05-24 23:31:06 +09:00
jaeone94
654a7d6904 Add error catalog display resolver (#12402) 
## Summary

This PR introduces the frontend error catalog display resolver as the
foundation for the DES-220 / FE-816 error messaging work.

The main goal is to create a single FE boundary where raw Core/Cloud
error payloads can be converted into human-friendly display fields,
while preserving the original API contract fields (`message` and
`details`) unchanged. UI components can now prefer resolved display copy
when it exists and fall back to the raw API copy otherwise.

As a small concrete sample, this PR implements the first cataloged
validation error:

- `required_input_missing` is resolved as the `missing_connection`
catalog item.
- Panel title: `Missing connection`
- Panel message: `Required input slots have no connection feeding them.`
- Detail/item copy can include the node and input name, e.g. `KSampler
is missing a required input: model` and `KSampler - model`.
- Single-error toast/overlay-oriented fields are added to the data model
for follow-up UI work, but this PR does not redesign the overlay.

## What This PR Targets

This PR is intentionally scoped as the skeleton PR for the error catalog
UX system.

It adds:

- A new resolver module under `src/platform/errorCatalog`.
- Shared resolved display fields:
  - `catalogId`
  - `displayTitle`
  - `displayMessage`
  - `displayDetails`
  - `displayItemLabel`
  - `toastTitle`
  - `toastMessage`
- A resolver entry point for run-time workflow errors:
  - node validation errors
  - execution/runtime errors
  - prompt errors
- A resolver entry point for pre-run missing-resource groups:
  - missing node packs
  - swap nodes
  - missing models
  - missing media
- Error group wiring so `useErrorGroups` resolves display copy in one
place instead of making UI components own message decisions.
- The first real validation rule for `required_input_missing` / Missing
connection.
- The existing prompt error copy moved into the
`errorCatalog.promptErrors` namespace in `src/locales/en/main.json`.
- Tests covering the resolver, grouping behavior, panel rendering,
prompt error copy, missing group copy, and fallback behavior.

## What This PR Deliberately Does Not Target

This PR avoids the larger UX and product behavior changes so the
foundation can land separately.

It does not:

- Redesign the error overlay.
- Redesign the right-side error panel.
- Change the shape of Core/Cloud API error payloads.
- Replace raw `message` / `details`; those remain intact for
API-contract alignment and technical debugging.
- Re-group execution errors by final message type yet.
- Add special runtime error messaging for credits, timeouts, content
policy, OOM, or rate limits.
- Render the new `displayItemLabel` everywhere it will eventually be
useful.

## User-Facing Behavior

Most behavior is preserved.

The main visible change is for missing required input validation errors.
Those now display as Missing connection copy instead of exposing the raw
validation message directly.

Prompt errors should keep the same user-facing wording as before, but
the source of that wording now lives under the error catalog namespace.

Missing node/model/media/swap-node groups still preserve the existing
titles, counts, and friendly messages, but their display copy now flows
through the same resolver boundary.

Execution/runtime errors receive catalog fields for future toast/overlay
usage, but the current runtime overlay path intentionally keeps the raw
technical error copy until the overlay redesign PR decides how to
consume the new fields.

## Screenshots 
Before
<img width="505" height="266" alt="스크린샷 2026-05-22 오후 2 15 27"
src="https://github.com/user-attachments/assets/09e8eb31-dca4-42d8-8237-9474cb71a14c"
/>
<img width="463" height="317" alt="스크린샷 2026-05-22 오후 2 16 09"
src="https://github.com/user-attachments/assets/c0a0159e-5bd9-4b3f-9c21-c0040373fbca"
/>

After
<img width="482" height="297" alt="스크린샷 2026-05-22 오후 2 14 25"
src="https://github.com/user-attachments/assets/4ca10bf0-29d2-4b65-940e-0d78db3fd278"
/>
<img width="460" height="194" alt="스크린샷 2026-05-22 오후 2 16 55"
src="https://github.com/user-attachments/assets/20848054-5012-4dd3-b903-ef8c920f70c8"
/>


## Follow-Up PR Plan

This PR is the first stacked PR in the error catalog work. Follow-up PRs
are expected to build on this foundation in roughly this order:

1. Expand general execution error messaging.
- Add broader validation error handling beyond `required_input_missing`,
including list/range/value validation cases.
   - Add general runtime execution messaging.
- Continue migrating prompt error display decisions into the catalog
resolver.

2. Add special runtime error messaging.
   - Credits / insufficient credits.
   - Timeout.
   - Content not allowed / blocked content.
   - Server crash.
   - Out of memory.
   - Rate limiting.
   - Other high-volume Cloud-only runtime failures from DES-220.

3. Re-group execution errors by message/catalog type.
- Move away from grouping primarily by node class when the cataloged
error type is the more useful user-facing grouping key.
   - Keep raw technical details available inside cards/logs.

4. Update the error overlay behavior.
   - Use `toastTitle` and `toastMessage` for single-error cases.
   - Use aggregate copy such as "N errors found" for multi-error cases.
   - Add node navigation affordances where appropriate.

5. Update the right-side error panel design.
   - Render resolved item labels such as `Node name - Input name`.
   - Align expanded card details and logs with the new design.
   - Preserve copy/debug affordances for technical details.

6. Fold in related missing media/model/node messaging improvements.
- FE-583 should become a child/follow-up issue in this stack for
improving missing image/media messaging.

## Validation

- `pnpm format`
- `pnpm lint`
- `pnpm typecheck`
- `pnpm test:unit`
- Targeted resolver/grouping tests during review iterations
- `pnpm knip`

`pnpm knip` passes with only the pre-existing tag hint:

`Unused tag in src/scripts/metadata/flac.ts: getFromFlacBuffer →
@knipIgnoreUnusedButUsedByCustomNodes`
 2026-05-23 01:31:29 +00:00
jaeone94
693b8383d6 fix: missing-asset correctness follow-ups from #10856 (#11233) 
Follow-up to #10856. Four correctness issues and their regression tests.

## Bugs fixed

### 1. ErrorOverlay model count reflected node selection

`useErrorGroups` exposed `filteredMissingModelGroups` under the public
name `missingModelGroups`. `ErrorOverlay.vue` read that alias to compute
its model count label, so selecting a node shrank the overlay total. The
overlay must always show the whole workflow's errors.

Exposed both shapes explicitly: `missingModelGroups` /
`missingMediaGroups` (unfiltered totals) and
`filteredMissingModelGroups` / `filteredMissingMediaGroups`
(selection-scoped). `TabErrors.vue` destructures the filtered variant
with an alias.


Before 


https://github.com/user-attachments/assets/eb848c5f-d092-4a4f-b86f-d22bb4408003

After 


https://github.com/user-attachments/assets/75e67819-c9f2-45ec-9241-74023eca6120



### 2. Bypass → un-bypass dropped url/hash metadata

Realtime `scanNodeModelCandidates` only reads widget values, so
un-bypass produced a fresh candidate without the url that
`enrichWithEmbeddedMetadata` had previously attached from
`graphData.models`. `MissingModelRow`'s download/copy-url buttons
disappeared after a bypass/un-bypass cycle.

Added `enrichCandidateFromNodeProperties` that copies
`url`/`hash`/`directory` from the node's own `properties.models` — which
persists across mode toggles — into each scanned candidate. Applied to
every call site of the per-node scan. A later fix in the same branch
also enforces directory agreement to prevent a same-name /
different-directory collision from stamping the wrong metadata.

Before 


https://github.com/user-attachments/assets/39039d83-4d55-41a9-9d01-dec40843741b

After 


https://github.com/user-attachments/assets/047a603b-fb52-4320-886d-dfeed457d833



### 3. Initial full scan surfaced interior errors of a muted/bypassed
subgraph container

`scanAllModelCandidates`, `scanAllMediaCandidates`, and the JSON-based
missing-node scan only check each node's own mode. Interior nodes whose
parent container was bypassed passed the filter.

Added `isAncestorPathActive(rootGraph, executionId)` to
`graphTraversalUtil` and post-filter the three pipelines in `app.ts`
after the live rootGraph is configured. The filter uses the execution-ID
path (`"65:63"` → check node 65's mode) so it handles both
live-scan-produced and JSON-enrichment-produced candidates.

Before


https://github.com/user-attachments/assets/3032d46b-81cd-420e-ab8e-f58392267602

After 


https://github.com/user-attachments/assets/02a01931-951d-4a48-986c-06424044fbf8




### 4. Bypassed subgraph entry re-surfaced interior errors

`useGraphNodeManager` replays `graph.onNodeAdded` for each existing
interior node when the Vue node manager initializes on subgraph entry.
That chain reached `scanSingleNodeErrors` via
`installErrorClearingHooks`' `onNodeAdded` override. Each interior
node's own mode was active, so the caller guards passed and the scan
re-introduced the error that the initial pipeline had correctly
suppressed.

Added an ancestor-activity gate at the top of `scanSingleNodeErrors`,
the single entry point shared by paste, un-bypass, subgraph entry, and
subgraph container activation. A later commit also hardens this guard
against detached nodes (null execution ID → skip) and applies the same
ancestor check to `isCandidateStillActive` in the realtime verification
callback.

Before


https://github.com/user-attachments/assets/fe44862d-f1d6-41ed-982d-614a7e83d441

After


https://github.com/user-attachments/assets/497a76ce-3caa-479f-9024-4cd0f7bd20a4



## Tests

- 6 unit tests for `isAncestorPathActive` (root, active,
immediate-bypass, deep-nested mute, unresolvable ancestor, null
rootGraph)
- 4 unit tests for `enrichCandidateFromNodeProperties` (enrichment,
no-overwrite, name mismatch, directory mismatch)
- 1 unit test for `scanSingleNodeErrors` ancestor guard (subgraph entry
replaying onNodeAdded)
- 2 unit tests for `useErrorGroups` dual export + ErrorOverlay contract
- 4 E2E tests:
- ErrorOverlay model count stays constant when a node is selected (new
fixture `missing_models_distinct.json`)
- Bypass/un-bypass cycle preserves Copy URL button (uses
`missing_models_from_node_properties`)
- Loading a workflow with bypassed subgraph suppresses interior missing
model error (new fixture `missing_models_in_bypassed_subgraph.json`)
- Entering a bypassed subgraph does not resurface interior missing model
error (shares the above fixture)

`pnpm typecheck`, `pnpm lint`, 206 related unit tests passing.

## Follow-up

Several items raised by code review are deferred as pre-existing tech
debt or scope-avoided refactors. Tracked via comments on #11215 and
#11216.

---
Follows up on #10856.
 2026-04-15 10:58:24 +00: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
jaeone94
e2ef041170 feat: surface missing models in Error Tab for OSS and remove legacy dialog (#9921) 
## Summary
- Surface missing models in the Error Tab for OSS environments,
replacing the legacy modal dialog
- Add Download button per model and Download All button in group header
with file size display
- Move download business logic from `components/dialog/content` to
`platform/missingModel`
- Remove legacy missing models dialog components and composable

## Changes
- **Pipeline**: Remove `isCloud` guard from `scanAllModelCandidates` and
`surfaceMissingModels` so OSS detects missing models
- **Grouping**: Group non-asset-supported models by directory in OSS
instead of lumping into UNSUPPORTED
- **UI**: Add Download button (matching Install Node Pack design) and
Download All header button
- **Store**: Add `folderPaths`/`fileSizes` state with setter methods,
race condition guard
- **Cleanup**: Delete `MissingModelsContent`, `MissingModelsHeader`,
`MissingModelsFooter`, `useMissingModelsDialog`, `missingModelsUtils`
- **Tests**: Add OSS/Cloud grouping tests, migrate Playwright E2E to
Error Tab, improve test isolation
- **Snapshots**: Reset Playwright screenshot expectations since OSS
missing model error detection now causes red highlights on affected
nodes
- **Accessibility**: Add `aria-label` with model name, `aria-expanded`
on toggle, warning icon for unknown category

## Test plan
- [x] Unit tests pass (86 tests)
- [x] TypeScript typecheck passes
- [x] knip passes
- [x] Load workflow with missing models in OSS → Error Tab shows missing
models grouped by directory
- [x] Download button triggers browser download with correct URL
- [x] Download All button downloads all downloadable models
- [x] Cloud environment behavior unchanged
- [x] Playwright E2E: `pnpm test:browser:local -- --grep "Missing models
in Error Tab"`

## Screenshots


https://github.com/user-attachments/assets/12f15e09-215a-4c58-87ed-39bbffd1359c

 


┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9921-feat-surface-missing-models-in-Error-Tab-for-OSS-and-remove-legacy-dialog-3236d73d365081f0a9dfc291978f5ecf)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
 2026-03-15 22:46:47 +09:00
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)
 2026-03-12 16:21:54 +09:00
jaeone94
9e2299ca65 feat(error-groups): sort execution error cards by node execution ID (#9334) 
## Summary

Sort execution error cards within each error group by their node
execution ID in ascending numeric order, ensuring consistent and
predictable display order.

## Changes

- **What**: Added `compareExecutionId` utility to
`src/types/nodeIdentification.ts` that splits node IDs on `:` and
compares segments numerically left-to-right; applied it as a sort
comparator when building `ErrorGroup.cards` in `useErrorGroups.ts`

## Review Focus

- The comparison treats missing segments as `0`, so `"1"` sorts before
`"1:20"` (subgraph nodes follow their parent); confirm this ordering
matches user expectations
- All comparisons are purely numeric — non-numeric segment values would
sort as `NaN` (treated as `0`)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9334-feat-error-groups-sort-execution-error-cards-by-node-execution-ID-3176d73d365081e1b3e4e4fa8831fe16)
by [Unito](https://www.unito.io)
 2026-03-04 13:59:58 -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