mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-07-03 13:48:49 +00:00
## Summary This PR intentionally narrows workflow-embedded model metadata handling so root-level `models[]` and node-level embedded model metadata can enrich existing missing-model candidates, but can no longer create new candidates by themselves. ## Why this PR exists ADR 0009, **Subgraph promoted widgets use linked inputs**, changes promoted value ownership for subgraphs. That design was implemented by [#12197](https://github.com/Comfy-Org/ComfyUI_frontend/pull/12197), **Subgraph Link Only Promotion (ADR 0009)**. Under ADR 0009, a promoted widget is represented as a standard linked `SubgraphInput` on the host `SubgraphNode`. The host boundary owns the promoted value identity through the host node locator plus `SubgraphInput.name`. The interior source widget remains the provider of schema, type, options, tooltip, defaults, diagnostics, and migration metadata, but it is not the persistence owner of the promoted value. This PR is a preparatory cleanup discovered while working on the missing model detection follow-up required by that ADR 0009 / [#12197](https://github.com/Comfy-Org/ComfyUI_frontend/pull/12197) behavior. The follow-up needs missing model detection to respect the new subgraph promoted-widget ownership model. While reviewing that path, we found that the existing embedded model metadata fallback in `enrichWithEmbeddedMetadata` was doing more than metadata enrichment. The important finding was that this fallback was not just attaching metadata to candidates that had already been detected from live node widgets. It could also synthesize brand-new `MissingModelCandidate` entries from workflow JSON metadata, including root-level `models[]` entries, when no live candidate existed. That behavior is inaccurate for the missing model system for two reasons. First, the normal missing model lifecycle is anchored to a real node/widget binding. A candidate found from a COMBO or asset widget has a concrete `nodeId + widgetName` reference. That reference is what lets the UI surface the error, cache it as a pending warning, and later clear or resolve it when the underlying node/widget value is fixed. A root-level `models[]` entry does not reliably provide that anchor. If metadata-only fallback creates a candidate without a real live widget reference, the resulting error can be detected but cannot reliably travel through the existing clearing path. In practice, that can become an effectively unremovable missing model warning unless the user downloads exactly the same model referenced by the stale metadata. Second, a missing model error is meant to mean that a model-selecting widget on an active node references a value that is not available. Workflow JSON metadata by itself is not the same source of truth. If a model only appears in root workflow metadata, or appears in node metadata that is not represented by an active COMBO or asset widget candidate, that is a different kind of state from the existing missing model error model. Treating that metadata as a candidate creates a second, less reliable detector that is not aligned with the scan/clear lifecycle. This is especially important before the ADR 0009 missing-model follow-up. With linked-input promoted widgets, the host promoted value is the value that matters. The interior source widget may still carry stale or default metadata, and it must not become a second source of truth for missing model errors. A detection path that can create candidates directly from workflow metadata would make it harder to reason about which value actually produced the warning. For those reasons, this PR removes metadata-only candidate synthesis and keeps embedded metadata in the role it can perform safely: metadata enrichment. If the live widget/asset scan produces a candidate, embedded metadata may fill in `directory`, `url`, `hash`, and `hashType`. If no live candidate exists, the metadata is not enough to create a missing model warning. This PR is intended to land before the child PR that updates runtime missing model detection for ADR 0009 linked-input promoted widgets. ## Changes - **What**: Restrict `enrichWithEmbeddedMetadata` to enriching existing candidates instead of creating fallback candidates from unmatched root `models[]` or embedded model metadata. - **What**: Remove the now-unused installed-model check callback and asset-support callback from `enrichWithEmbeddedMetadata`. - **What**: Remove the now-unnecessary `modelStore.loadModelFolders()` path from the missing model pipeline, since embedded metadata no longer performs installed-model fallback detection. - **What**: Remove dead source-tracking metadata (`EmbeddedModelWithSource`, source node/widget fields, and widget-name lookup) that only existed to support metadata-only synthesis. - **What**: Update missing model tests so they assert the new contract: metadata enriches live candidates, but does not create candidates without a live scan result. - **What**: Delete obsolete fixtures that only covered the removed metadata-only synthesis path. - **Breaking**: None expected. This is an intentional narrowing of an inaccurate fallback detector, not a public API change. - **Dependencies**: None. ## Review Focus Please focus on whether the candidate lifecycle now has a single source of truth: live COMBO/asset widget scanning creates candidates, while workflow metadata only enriches those candidates. The intended behavioral change is that a model present only in workflow-level metadata, with no active node widget candidate referencing it, no longer appears as a missing model. This avoids surfacing warnings that cannot be cleared through the normal `nodeId + widgetName` path. The expected retained behavior is that active widget-referenced missing models are still detected by `scanAllModelCandidates`, and metadata from root `models[]` or node `properties.models` still supplies download-related fields for those live candidates. ## Screenshots (if applicable) Not applicable. This is a detection/pipeline behavior change covered by unit tests. ## Validation - `pnpm test:unit src/platform/missingModel/missingModelScan.test.ts src/platform/missingModel/missingModelPipeline.test.ts` - `pnpm exec eslint src/platform/missingModel/missingModelScan.ts src/platform/missingModel/missingModelScan.test.ts src/platform/missingModel/missingModelPipeline.ts src/platform/missingModel/missingModelPipeline.test.ts src/platform/missingModel/types.ts` - `pnpm exec oxfmt --check src/platform/missingModel/missingModelScan.ts src/platform/missingModel/missingModelScan.test.ts src/platform/missingModel/missingModelPipeline.ts src/platform/missingModel/missingModelPipeline.test.ts src/platform/missingModel/types.ts` - `pnpm typecheck` - pre-push hook: `knip --cache`
165 lines
5.4 KiB
TypeScript
165 lines
5.4 KiB
TypeScript
import { expect } from '@playwright/test'
|
|
import type { Locator } from '@playwright/test'
|
|
|
|
import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage'
|
|
import { TestIds } from '@e2e/fixtures/selectors'
|
|
import {
|
|
interceptClipboardWrite,
|
|
getClipboardText
|
|
} from '@e2e/fixtures/utils/clipboardSpy'
|
|
import {
|
|
cleanupFakeModel,
|
|
loadWorkflowAndOpenErrorsTab
|
|
} from '@e2e/fixtures/helpers/ErrorsTabHelper'
|
|
|
|
const FAKE_MODEL_NAME = 'fake_model.safetensors'
|
|
|
|
function getModelLabel(group: Locator, modelName: string = FAKE_MODEL_NAME) {
|
|
return group.getByRole('button', { name: modelName, exact: true })
|
|
}
|
|
|
|
async function expectReferenceBadge(group: Locator, count: number) {
|
|
await expect(
|
|
group.getByTestId(TestIds.dialogs.missingModelReferenceCount)
|
|
).toHaveText(String(count))
|
|
}
|
|
|
|
test.describe('Errors tab - Missing models', { tag: '@ui' }, () => {
|
|
test.beforeEach(async ({ comfyPage }) => {
|
|
await comfyPage.settings.setSetting(
|
|
'Comfy.RightSidePanel.ShowErrorsTab',
|
|
true
|
|
)
|
|
await cleanupFakeModel(comfyPage)
|
|
})
|
|
|
|
test('Should show missing models group in errors tab', async ({
|
|
comfyPage
|
|
}) => {
|
|
await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models')
|
|
|
|
const missingModelsGroup = comfyPage.page.getByTestId(
|
|
TestIds.dialogs.missingModelsGroup
|
|
)
|
|
await expect(missingModelsGroup).toBeVisible()
|
|
await expect(
|
|
missingModelsGroup.getByTestId(TestIds.dialogs.errorGroupDisplayMessage)
|
|
).toHaveText(/\S/)
|
|
})
|
|
|
|
test('Should display model name and metadata', async ({ comfyPage }) => {
|
|
await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models')
|
|
|
|
const modelsGroup = comfyPage.page.getByTestId(
|
|
TestIds.dialogs.missingModelsGroup
|
|
)
|
|
await expect(getModelLabel(modelsGroup)).toBeVisible()
|
|
await expect(modelsGroup.getByText('checkpoints')).toBeVisible()
|
|
})
|
|
|
|
test('Should expand model row to show referencing nodes', async ({
|
|
comfyPage
|
|
}) => {
|
|
await loadWorkflowAndOpenErrorsTab(
|
|
comfyPage,
|
|
'missing/missing_models_with_nodes'
|
|
)
|
|
|
|
const modelsGroup = comfyPage.page.getByTestId(
|
|
TestIds.dialogs.missingModelsGroup
|
|
)
|
|
const expandButton = modelsGroup.getByTestId(
|
|
TestIds.dialogs.missingModelExpand
|
|
)
|
|
await expect(expandButton.first()).toBeVisible()
|
|
await expectReferenceBadge(modelsGroup, 2)
|
|
await expandButton.first().click()
|
|
|
|
await expect(
|
|
modelsGroup.getByTestId(TestIds.dialogs.missingModelLocate)
|
|
).toHaveCount(2)
|
|
})
|
|
|
|
test('Should copy model URL to clipboard', async ({ comfyPage }) => {
|
|
await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models')
|
|
await interceptClipboardWrite(comfyPage.page)
|
|
|
|
const copyButton = comfyPage.page.getByRole('button', {
|
|
name: 'Copy URL'
|
|
})
|
|
await expect(copyButton.first()).toBeVisible()
|
|
await copyButton.first().dispatchEvent('click')
|
|
|
|
const copiedText = await getClipboardText(comfyPage.page)
|
|
expect(copiedText).toContain('/api/devtools/')
|
|
})
|
|
|
|
test.describe('OSS-specific', { tag: '@oss' }, () => {
|
|
test('Should show Copy URL button for non-asset models', async ({
|
|
comfyPage
|
|
}) => {
|
|
await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models')
|
|
|
|
const copyUrlButton = comfyPage.page.getByRole('button', {
|
|
name: 'Copy URL'
|
|
})
|
|
await expect(copyUrlButton.first()).toBeVisible()
|
|
})
|
|
|
|
test('Should show Download button for downloadable models', async ({
|
|
comfyPage
|
|
}) => {
|
|
await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models')
|
|
|
|
const downloadButton = comfyPage.page.getByTestId(
|
|
TestIds.dialogs.missingModelDownload
|
|
)
|
|
await expect(downloadButton.first()).toBeVisible()
|
|
await expect(downloadButton.first()).toHaveText('Download')
|
|
})
|
|
|
|
test('Should render Download all and Refresh actions for one downloadable model', async ({
|
|
comfyPage
|
|
}) => {
|
|
await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models')
|
|
|
|
await expect(
|
|
comfyPage.page.getByTestId(TestIds.dialogs.missingModelActions)
|
|
).toBeVisible()
|
|
await expect(
|
|
comfyPage.page.getByTestId(TestIds.dialogs.missingModelDownloadAll)
|
|
).toBeVisible()
|
|
await expect(
|
|
comfyPage.page.getByTestId(TestIds.dialogs.missingModelRefresh)
|
|
).toBeVisible()
|
|
})
|
|
|
|
test('Should clear resolved missing model when Refresh is clicked', async ({
|
|
comfyPage
|
|
}) => {
|
|
await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models')
|
|
await comfyPage.page.route(/\/object_info$/, async (route) => {
|
|
const response = await route.fetch()
|
|
const objectInfo = await response.json()
|
|
const ckptName =
|
|
objectInfo.CheckpointLoaderSimple.input.required.ckpt_name
|
|
ckptName[0] = [...ckptName[0], FAKE_MODEL_NAME]
|
|
await route.fulfill({ response, json: objectInfo })
|
|
})
|
|
|
|
const objectInfoResponse = comfyPage.page.waitForResponse((response) => {
|
|
const url = new URL(response.url())
|
|
return url.pathname.endsWith('/object_info') && response.ok()
|
|
})
|
|
const refreshButton = comfyPage.page.getByTestId(
|
|
TestIds.dialogs.missingModelRefresh
|
|
)
|
|
|
|
await Promise.all([objectInfoResponse, refreshButton.click()])
|
|
await expect(
|
|
comfyPage.page.getByTestId(TestIds.dialogs.missingModelsGroup)
|
|
).toBeHidden()
|
|
})
|
|
})
|
|
})
|