mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-13 17:26:22 +00:00
refactor: extract missing model refresh pipeline (#11751)
## Summary Extracts the missing-model pipeline orchestration out of `ComfyApp` and into an app-independent platform module, while tightening the workflow-flattening type boundary that refresh needs when rescanning the live LiteGraph graph. This PR is intentionally refactor-heavy. It is the follow-up to the earlier missing-model refresh work: instead of keeping refresh-specific candidate recheck logic beside the UI, this change makes the refresh path reuse the existing missing-model pipeline and removes the direct dependency on private `ComfyApp` pipeline methods. Linear: FE-499 Issues covered by this PR: - Fixes #11678 - Fixes #11680 - Partially addresses #11679 by removing the missing-model refresh path's unsafe `graph.serialize() as unknown as ComfyWorkflowJSON` cast and replacing it with the narrower flattenable workflow contract. Broader workflow serialization/type-boundary cleanup outside this missing-model refresh path remains deferred. ## Changes - **What**: - Added `src/platform/missingModel/missingModelPipeline.ts` as the orchestration module for missing-model detection/verification. - `runMissingModelPipeline(...)` now owns the pipeline previously embedded in `ComfyApp`: - candidate scan and enrichment - active ancestor filtering for muted/bypassed subgraph containers - pending warning cache updates - OSS folder path and file-size follow-up work - cloud asset verification follow-up work - surfaced missing-model errors via the existing execution error store - `refreshMissingModelPipeline(...)` handles the refresh-specific flow: - calls the injected `reloadNodeDefs()` first - serializes the current live graph - preserves model metadata by preferring active workflow `models`, then falling back to current missing-model candidate metadata - delegates back into the same pipeline used during workflow load - Kept `ComfyApp` as the compatibility caller instead of the owner of the pipeline. - `loadGraphData(...)` now calls `runMissingModelPipeline(...)` with `graph`, `graphData`, `missingNodeTypes`, and `silent` options. - `refreshMissingModels(...)` is now a thin wrapper around `refreshMissingModelPipeline(...)` and keeps the existing default `silent: true` refresh behavior. - The new pipeline module does not import `@/scripts/app`; app-owned data/actions are passed in as inputs. - Moved the workflow node-flattening helpers out of `workflowSchema.ts` and into `src/platform/workflow/core/utils/workflowFlattening.ts`. - This includes `flattenWorkflowNodes`, `buildSubgraphExecutionPaths`, and `isSubgraphDefinition`. - The move is intentional: these helpers are not zod schema definitions or workflow validation logic. They are core workflow traversal utilities used to flatten root workflow nodes plus nested subgraph definition nodes into the execution-shaped node list needed by missing-model scanning. - The refresh path receives data from `LGraph.serialize()`, whose return type is serialized LiteGraph data rather than validated `ComfyWorkflowJSON`. Previously this forced unsafe typing like `graph.serialize() as unknown as ComfyWorkflowJSON`. - The new `FlattenableWorkflowGraph` / `FlattenableWorkflowNode` structural contract describes only what flattening actually needs: `nodes`, `definitions.subgraphs`, node `id`, `type`, `mode`, `widgets_values`, and `properties`. - This lets both normal workflow-load data (`ComfyWorkflowJSON`) and refresh-time live graph serialization (`LGraph.serialize()`) flow into the same scan/enrichment path without pretending serialized LiteGraph output is a fully validated workflow schema document. - Updated `missingModelScan.ts` to consume that minimal flattenable workflow shape via `MissingModelWorkflowData`. - `MissingModelWorkflowData` extends the flattenable workflow contract with optional workflow-level `models` metadata. - Removed now-unnecessary casts around execution IDs, flattened nodes, and `widgets_values` object access. - Updated `getSelectedModelsMetadata(...)` to accept readonly widget value arrays so flattened workflow data can stay read-only. - Reduced the exported surface of the new pipeline module after `knip` flagged unused exported internal option/store interfaces. - Kept `workflowSchema.ts` focused on validation schemas. The flattening helpers are not re-exported from the schema module because they are internal workflow core utilities, not public schema API. - **Breaking**: None intended. - Internal imports were updated to the new core utility path. - This repo is not exposing these flattening helpers as a public package API, so the old schema-local helper location is treated as an internal implementation detail. - **Dependencies**: None. ## Review Focus - **Pipeline extraction / dependency direction**: - Please verify that `missingModelPipeline.ts` stays independent from `@/scripts/app`. - `ComfyApp` should remain the caller/adapter, not the owner of missing-model pipeline orchestration. - **Workflow flattening type boundary**: - The main type-cleanup goal is removing the refresh-time `graph.serialize() as unknown as ComfyWorkflowJSON` lie. - `LGraph.serialize()` and validated workflow JSON are not the same contract. The new flattenable workflow contract is deliberately smaller and structural because the missing-model enrichment path only needs enough data to flatten nodes and read embedded model metadata. - This is why the flattening helpers moved from `workflowSchema.ts` to `workflow/core/utils`: the logic is reusable workflow traversal, not validation schema. - **Behavior preservation**: - The PR is intended to preserve existing user-facing missing-model behavior while moving ownership out of `app.ts`. - Existing async follow-up behavior remains intentionally fire-and-forget: - cloud asset verification still surfaces after verification completes - OSS folder paths still update asynchronously before surfacing confirmed missing models - file-size metadata fetching remains asynchronous - More invasive behavior changes, such as adding non-cloud post-fetch `isMissingCandidateActive(...)` re-verification or redesigning the fire-and-forget result contract, are intentionally left for follow-up work because they are not pure extraction. - **Downloadable model metadata**: - `missingModels` returned for download metadata now requires both `url` and `directory`. - Candidates without a directory still remain in `confirmedCandidates`, but they are not exposed as downloadable model metadata. This keeps the returned downloadable list aligned with what the download flow can actually use. - **Test ownership**: - Complex missing-model pipeline behavior tests moved out of `src/scripts/app.test.ts` and into `src/platform/missingModel/missingModelPipeline.test.ts`. - `app.test.ts` now only covers thin delegation for `app.refreshMissingModels(...)`. - Workflow flattening tests moved with the helper from schema tests into `src/platform/workflow/core/utils/workflowFlattening.test.ts`. - **Deferred follow-ups**: - Broader function decomposition for cognitive complexity. - Wider dependency-injection/port cleanup for stores and services beyond the app boundary. - Cloud-specific pipeline unit tests, which need a separate `isCloud` mocking strategy. - Additional E2E coverage expansion beyond the existing OSS refresh path. - More general workflow serialization/type-boundary cleanup outside the missing-model refresh path. ## Validation - `pnpm format` - `pnpm lint` - Passed. Existing lint output included a pre-existing `no-misused-spread` warning and icon-name logs, but the command exited successfully. - `pnpm typecheck` - `pnpm test:unit` - `714 passed`, `9514 passed | 8 skipped` - Pre-push `pnpm knip` - Passed after reducing the exported surface of the new pipeline module. ## Screenshots (if applicable) Not applicable. This PR is a pipeline/type-boundary refactor with no UI changes. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11751-refactor-extract-missing-model-refresh-pipeline-3516d73d3650816d9245d4b1324b71c9) by [Unito](https://www.unito.io) --------- Co-authored-by: DrJKL <DrJKL0424@gmail.com> Co-authored-by: Alexander Brown <drjkl@comfy.org>
This commit is contained in:
@@ -1,9 +1,6 @@
|
||||
import type {
|
||||
ComfyWorkflowJSON,
|
||||
ModelFile,
|
||||
NodeId
|
||||
} from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import { flattenWorkflowNodes } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import type { ModelFile } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import type { FlattenableWorkflowGraph } from '@/platform/workflow/core/utils/workflowFlattening'
|
||||
import { flattenWorkflowNodes } from '@/platform/workflow/core/utils/workflowFlattening'
|
||||
import type {
|
||||
MissingModelCandidate,
|
||||
MissingModelViewModel,
|
||||
@@ -28,6 +25,10 @@ import {
|
||||
import { LGraphEventMode } from '@/lib/litegraph/src/types/globalEnums'
|
||||
import { resolveComboValues } from '@/utils/litegraphUtil'
|
||||
|
||||
export type MissingModelWorkflowData = FlattenableWorkflowGraph & {
|
||||
models?: ModelFile[]
|
||||
}
|
||||
|
||||
function isComboWidget(widget: IBaseWidget): widget is IComboWidget {
|
||||
return widget.type === 'combo'
|
||||
}
|
||||
@@ -180,7 +181,7 @@ function scanAssetWidget(
|
||||
if (!isModelFileName(value)) return null
|
||||
|
||||
return {
|
||||
nodeId: executionId as NodeId,
|
||||
nodeId: executionId,
|
||||
nodeType: node.type,
|
||||
widgetName: widget.name,
|
||||
isAssetSupported: true,
|
||||
@@ -206,7 +207,7 @@ function scanComboWidget(
|
||||
const inOptions = options.includes(value)
|
||||
|
||||
return {
|
||||
nodeId: executionId as NodeId,
|
||||
nodeId: executionId,
|
||||
nodeType: node.type,
|
||||
widgetName: widget.name,
|
||||
isAssetSupported: nodeIsAssetSupported,
|
||||
@@ -218,7 +219,7 @@ function scanComboWidget(
|
||||
|
||||
export async function enrichWithEmbeddedMetadata(
|
||||
candidates: readonly MissingModelCandidate[],
|
||||
graphData: ComfyWorkflowJSON,
|
||||
graphData: MissingModelWorkflowData,
|
||||
checkModelInstalled: (name: string, directory: string) => Promise<boolean>,
|
||||
isAssetSupported?: (nodeType: string, widgetName: string) => boolean
|
||||
): Promise<MissingModelCandidate[]> {
|
||||
@@ -388,7 +389,7 @@ function isAncestorPathActiveInFlattened(
|
||||
|
||||
function collectEmbeddedModelsWithSource(
|
||||
allNodes: ReturnType<typeof flattenWorkflowNodes>,
|
||||
graphData: ComfyWorkflowJSON
|
||||
graphData: MissingModelWorkflowData
|
||||
): EmbeddedModelWithSource[] {
|
||||
const result: EmbeddedModelWithSource[] = []
|
||||
|
||||
@@ -399,9 +400,7 @@ function collectEmbeddedModelsWithSource(
|
||||
)
|
||||
continue
|
||||
|
||||
const selected = getSelectedModelsMetadata(
|
||||
node as Parameters<typeof getSelectedModelsMetadata>[0]
|
||||
)
|
||||
const selected = getSelectedModelsMetadata(node)
|
||||
if (!selected?.length) continue
|
||||
|
||||
for (const model of selected) {
|
||||
@@ -435,8 +434,7 @@ function findWidgetNameForModel(
|
||||
modelName: string
|
||||
): string {
|
||||
if (Array.isArray(node.widgets_values) || !node.widgets_values) return ''
|
||||
const wv = node.widgets_values as Record<string, unknown>
|
||||
for (const [key, val] of Object.entries(wv)) {
|
||||
for (const [key, val] of Object.entries(node.widgets_values)) {
|
||||
if (val === modelName) return key
|
||||
}
|
||||
return ''
|
||||
|
||||
Reference in New Issue
Block a user