diff --git a/browser_tests/assets/subgraphs/nested-pack-promoted-values.json b/browser_tests/assets/subgraphs/nested-pack-promoted-values.json new file mode 100644 index 0000000000..f4f2665680 --- /dev/null +++ b/browser_tests/assets/subgraphs/nested-pack-promoted-values.json @@ -0,0 +1,817 @@ +{ + "id": "9ae6082b-c7f4-433c-9971-7a8f65a3ea65", + "revision": 0, + "last_node_id": 61, + "last_link_id": 70, + "nodes": [ + { + "id": 35, + "type": "MarkdownNote", + "pos": [-424.0076397768001, 199.99406275798367], + "size": [510, 774], + "flags": { + "collapsed": false + }, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [], + "title": "Model link", + "properties": {}, + "widgets_values": [ + "## Report workflow issue\n\nIf you found any issues when running this workflow, [report template issue here](https://github.com/Comfy-Org/workflow_templates/issues)\n\n\n## Model links\n\n**text_encoders**\n\n- [qwen_3_4b.safetensors](https://huggingface.co/Comfy-Org/z_image_turbo/resolve/main/split_files/text_encoders/qwen_3_4b.safetensors)\n\n**loras**\n\n- [pixel_art_style_z_image_turbo.safetensors](https://huggingface.co/tarn59/pixel_art_style_lora_z_image_turbo/resolve/main/pixel_art_style_z_image_turbo.safetensors)\n\n**diffusion_models**\n\n- [z_image_turbo_bf16.safetensors](https://huggingface.co/Comfy-Org/z_image_turbo/resolve/main/split_files/diffusion_models/z_image_turbo_bf16.safetensors)\n\n**vae**\n\n- [ae.safetensors](https://huggingface.co/Comfy-Org/z_image_turbo/resolve/main/split_files/vae/ae.safetensors)\n\n\nModel Storage Location\n\n```\nšŸ“‚ ComfyUI/\nā”œā”€ā”€ šŸ“‚ models/\nā”‚ ā”œā”€ā”€ šŸ“‚ text_encoders/\nā”‚ ā”‚ ā””ā”€ā”€ qwen_3_4b.safetensors\nā”‚ ā”œā”€ā”€ šŸ“‚ loras/\nā”‚ ā”‚ ā””ā”€ā”€ pixel_art_style_z_image_turbo.safetensors\nā”‚ ā”œā”€ā”€ šŸ“‚ diffusion_models/\nā”‚ ā”‚ ā””ā”€ā”€ z_image_turbo_bf16.safetensors\nā”‚ ā””ā”€ā”€ šŸ“‚ vae/\nā”‚ ā””ā”€ā”€ ae.safetensors\n```\n" + ], + "color": "#432", + "bgcolor": "#000" + }, + { + "id": 9, + "type": "SaveImage", + "pos": [569.9875743118757, 199.99406275798367], + "size": [780, 660], + "flags": {}, + "order": 2, + "mode": 0, + "inputs": [ + { + "name": "images", + "type": "IMAGE", + "link": 62 + } + ], + "outputs": [], + "properties": { + "Node name for S&R": "SaveImage", + "cnr_id": "comfy-core", + "ver": "0.3.64", + "enableTabs": false, + "tabWidth": 65, + "tabXOffset": 10, + "hasSecondTab": false, + "secondTabText": "Send Back", + "secondTabOffset": 80, + "secondTabWidth": 65 + }, + "widgets_values": ["z-image-turbo"] + }, + { + "id": 57, + "type": "f2fdebf6-dfaf-43b6-9eb2-7f70613cfdc1", + "pos": [128.01215102992103, 199.99406275798367], + "size": [400, 470], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { + "label": "prompt", + "name": "text", + "type": "STRING", + "widget": { + "name": "text" + }, + "link": null + } + ], + "outputs": [ + { + "name": "IMAGE", + "type": "IMAGE", + "links": [62] + } + ], + "properties": { + "proxyWidgets": [ + ["27", "text"], + ["13", "width"], + ["13", "height"], + ["28", "unet_name"], + ["30", "clip_name"], + ["29", "vae_name"], + ["3", "steps"], + ["3", "control_after_generate"] + ], + "cnr_id": "comfy-core", + "ver": "0.3.73", + "enableTabs": false, + "tabWidth": 65, + "tabXOffset": 10, + "hasSecondTab": false, + "secondTabText": "Send Back", + "secondTabOffset": 80, + "secondTabWidth": 65 + }, + "widgets_values": [] + } + ], + "links": [[62, 57, 0, 9, 0, "IMAGE"]], + "groups": [], + "definitions": { + "subgraphs": [ + { + "id": "f2fdebf6-dfaf-43b6-9eb2-7f70613cfdc1", + "version": 1, + "state": { + "lastGroupId": 4, + "lastNodeId": 61, + "lastLinkId": 70, + "lastRerouteId": 0 + }, + "revision": 0, + "config": {}, + "name": "Text to Image (Z-Image-Turbo)", + "inputNode": { + "id": -10, + "bounding": [-80, 425, 120, 180] + }, + "outputNode": { + "id": -20, + "bounding": [1490, 415, 120, 60] + }, + "inputs": [ + { + "id": "fb178669-e742-4a53-8a69-7df59834dfd8", + "name": "text", + "type": "STRING", + "linkIds": [34], + "label": "prompt", + "pos": [20, 445] + }, + { + "id": "dd780b3c-23e9-46ff-8469-156008f42e5a", + "name": "width", + "type": "INT", + "linkIds": [35], + "pos": [20, 465] + }, + { + "id": "7b08d546-6bb0-4ef9-82e9-ffae5e1ee6bc", + "name": "height", + "type": "INT", + "linkIds": [36], + "pos": [20, 485] + }, + { + "id": "8ed4eb73-a2bf-4766-8bf4-c5890b560596", + "name": "unet_name", + "type": "COMBO", + "linkIds": [38], + "pos": [20, 505] + }, + { + "id": "f362d639-d412-4b5d-8490-1e9995dc5f82", + "name": "clip_name", + "type": "COMBO", + "linkIds": [39], + "pos": [20, 525] + }, + { + "id": "ee25ac16-de63-4b74-bbbb-5b29fdc1efcf", + "name": "vae_name", + "type": "COMBO", + "linkIds": [40], + "pos": [20, 545] + }, + { + "id": "51cbcd61-9218-4bcb-89ac-ecdfb1ef8892", + "name": "steps", + "type": "INT", + "linkIds": [70], + "pos": [20, 565] + } + ], + "outputs": [ + { + "id": "1fa72a21-ce00-4952-814e-1f2ffbe87d1d", + "name": "IMAGE", + "type": "IMAGE", + "linkIds": [16], + "localized_name": "IMAGE", + "pos": [1510, 435] + } + ], + "widgets": [], + "nodes": [ + { + "id": 30, + "type": "CLIPLoader", + "pos": [110, 330], + "size": [270, 106], + "flags": {}, + "order": 7, + "mode": 0, + "inputs": [ + { + "localized_name": "clip_name", + "name": "clip_name", + "type": "COMBO", + "widget": { + "name": "clip_name" + }, + "link": 39 + } + ], + "outputs": [ + { + "localized_name": "CLIP", + "name": "CLIP", + "type": "CLIP", + "links": [28] + } + ], + "properties": { + "Node name for S&R": "CLIPLoader", + "cnr_id": "comfy-core", + "ver": "0.3.73", + "models": [ + { + "name": "qwen_3_4b.safetensors", + "url": "https://huggingface.co/Comfy-Org/z_image_turbo/resolve/main/split_files/text_encoders/qwen_3_4b.safetensors", + "directory": "text_encoders" + } + ], + "enableTabs": false, + "tabWidth": 65, + "tabXOffset": 10, + "hasSecondTab": false, + "secondTabText": "Send Back", + "secondTabOffset": 80, + "secondTabWidth": 65 + }, + "widgets_values": ["qwen_3_4b.safetensors", "lumina2", "default"] + }, + { + "id": 29, + "type": "VAELoader", + "pos": [110, 480], + "size": [270, 58], + "flags": {}, + "order": 6, + "mode": 0, + "inputs": [ + { + "localized_name": "vae_name", + "name": "vae_name", + "type": "COMBO", + "widget": { + "name": "vae_name" + }, + "link": 40 + } + ], + "outputs": [ + { + "localized_name": "VAE", + "name": "VAE", + "type": "VAE", + "links": [27] + } + ], + "properties": { + "Node name for S&R": "VAELoader", + "cnr_id": "comfy-core", + "ver": "0.3.73", + "models": [ + { + "name": "ae.safetensors", + "url": "https://huggingface.co/Comfy-Org/z_image_turbo/resolve/main/split_files/vae/ae.safetensors", + "directory": "vae" + } + ], + "enableTabs": false, + "tabWidth": 65, + "tabXOffset": 10, + "hasSecondTab": false, + "secondTabText": "Send Back", + "secondTabOffset": 80, + "secondTabWidth": 65 + }, + "widgets_values": ["ae.safetensors"] + }, + { + "id": 33, + "type": "ConditioningZeroOut", + "pos": [640, 620], + "size": [204.134765625, 26], + "flags": {}, + "order": 8, + "mode": 0, + "inputs": [ + { + "localized_name": "conditioning", + "name": "conditioning", + "type": "CONDITIONING", + "link": 32 + } + ], + "outputs": [ + { + "localized_name": "CONDITIONING", + "name": "CONDITIONING", + "type": "CONDITIONING", + "links": [33] + } + ], + "properties": { + "Node name for S&R": "ConditioningZeroOut", + "cnr_id": "comfy-core", + "ver": "0.3.73", + "enableTabs": false, + "tabWidth": 65, + "tabXOffset": 10, + "hasSecondTab": false, + "secondTabText": "Send Back", + "secondTabOffset": 80, + "secondTabWidth": 65 + }, + "widgets_values": [] + }, + { + "id": 8, + "type": "VAEDecode", + "pos": [1220, 160], + "size": [210, 46], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { + "localized_name": "samples", + "name": "samples", + "type": "LATENT", + "link": 14 + }, + { + "localized_name": "vae", + "name": "vae", + "type": "VAE", + "link": 27 + } + ], + "outputs": [ + { + "localized_name": "IMAGE", + "name": "IMAGE", + "type": "IMAGE", + "slot_index": 0, + "links": [16] + } + ], + "properties": { + "Node name for S&R": "VAEDecode", + "cnr_id": "comfy-core", + "ver": "0.3.64", + "enableTabs": false, + "tabWidth": 65, + "tabXOffset": 10, + "hasSecondTab": false, + "secondTabText": "Send Back", + "secondTabOffset": 80, + "secondTabWidth": 65 + }, + "widgets_values": [] + }, + { + "id": 28, + "type": "UNETLoader", + "pos": [110, 200], + "size": [270, 82], + "flags": {}, + "order": 5, + "mode": 0, + "inputs": [ + { + "localized_name": "unet_name", + "name": "unet_name", + "type": "COMBO", + "widget": { + "name": "unet_name" + }, + "link": 38 + } + ], + "outputs": [ + { + "localized_name": "MODEL", + "name": "MODEL", + "type": "MODEL", + "links": [26] + } + ], + "properties": { + "Node name for S&R": "UNETLoader", + "cnr_id": "comfy-core", + "ver": "0.3.73", + "models": [ + { + "name": "z_image_turbo_bf16.safetensors", + "url": "https://huggingface.co/Comfy-Org/z_image_turbo/resolve/main/split_files/diffusion_models/z_image_turbo_bf16.safetensors", + "directory": "diffusion_models" + } + ], + "enableTabs": false, + "tabWidth": 65, + "tabXOffset": 10, + "hasSecondTab": false, + "secondTabText": "Send Back", + "secondTabOffset": 80, + "secondTabWidth": 65 + }, + "widgets_values": ["z_image_turbo_bf16.safetensors", "default"] + }, + { + "id": 27, + "type": "CLIPTextEncode", + "pos": [430, 200], + "size": [410, 370], + "flags": {}, + "order": 4, + "mode": 0, + "inputs": [ + { + "localized_name": "clip", + "name": "clip", + "type": "CLIP", + "link": 28 + }, + { + "localized_name": "text", + "name": "text", + "type": "STRING", + "widget": { + "name": "text" + }, + "link": 34 + } + ], + "outputs": [ + { + "localized_name": "CONDITIONING", + "name": "CONDITIONING", + "type": "CONDITIONING", + "links": [30, 32] + } + ], + "properties": { + "Node name for S&R": "CLIPTextEncode", + "cnr_id": "comfy-core", + "ver": "0.3.73", + "enableTabs": false, + "tabWidth": 65, + "tabXOffset": 10, + "hasSecondTab": false, + "secondTabText": "Send Back", + "secondTabOffset": 80, + "secondTabWidth": 65 + }, + "widgets_values": [ + "Latina female with thick wavy hair, harbor boats and pastel houses behind. Breezy seaside light, warm tones, cinematic close-up. " + ] + }, + { + "id": 13, + "type": "EmptySD3LatentImage", + "pos": [110, 630], + "size": [260, 110], + "flags": {}, + "order": 3, + "mode": 0, + "inputs": [ + { + "localized_name": "width", + "name": "width", + "type": "INT", + "widget": { + "name": "width" + }, + "link": 35 + }, + { + "localized_name": "height", + "name": "height", + "type": "INT", + "widget": { + "name": "height" + }, + "link": 36 + } + ], + "outputs": [ + { + "localized_name": "LATENT", + "name": "LATENT", + "type": "LATENT", + "slot_index": 0, + "links": [17] + } + ], + "properties": { + "Node name for S&R": "EmptySD3LatentImage", + "cnr_id": "comfy-core", + "ver": "0.3.64", + "enableTabs": false, + "tabWidth": 65, + "tabXOffset": 10, + "hasSecondTab": false, + "secondTabText": "Send Back", + "secondTabOffset": 80, + "secondTabWidth": 65 + }, + "widgets_values": [1024, 1024, 1] + }, + { + "id": 11, + "type": "ModelSamplingAuraFlow", + "pos": [880, 160], + "size": [310, 60], + "flags": {}, + "order": 2, + "mode": 0, + "inputs": [ + { + "localized_name": "model", + "name": "model", + "type": "MODEL", + "link": 26 + } + ], + "outputs": [ + { + "localized_name": "MODEL", + "name": "MODEL", + "type": "MODEL", + "slot_index": 0, + "links": [13] + } + ], + "properties": { + "Node name for S&R": "ModelSamplingAuraFlow", + "cnr_id": "comfy-core", + "ver": "0.3.64", + "enableTabs": false, + "tabWidth": 65, + "tabXOffset": 10, + "hasSecondTab": false, + "secondTabText": "Send Back", + "secondTabOffset": 80, + "secondTabWidth": 65 + }, + "widgets_values": [3] + }, + { + "id": 3, + "type": "KSampler", + "pos": [880, 270], + "size": [315, 262], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [ + { + "localized_name": "model", + "name": "model", + "type": "MODEL", + "link": 13 + }, + { + "localized_name": "positive", + "name": "positive", + "type": "CONDITIONING", + "link": 30 + }, + { + "localized_name": "negative", + "name": "negative", + "type": "CONDITIONING", + "link": 33 + }, + { + "localized_name": "latent_image", + "name": "latent_image", + "type": "LATENT", + "link": 17 + }, + { + "localized_name": "steps", + "name": "steps", + "type": "INT", + "widget": { + "name": "steps" + }, + "link": 70 + } + ], + "outputs": [ + { + "localized_name": "LATENT", + "name": "LATENT", + "type": "LATENT", + "slot_index": 0, + "links": [14] + } + ], + "properties": { + "Node name for S&R": "KSampler", + "cnr_id": "comfy-core", + "ver": "0.3.64", + "enableTabs": false, + "tabWidth": 65, + "tabXOffset": 10, + "hasSecondTab": false, + "secondTabText": "Send Back", + "secondTabOffset": 80, + "secondTabWidth": 65 + }, + "widgets_values": [ + 0, + "randomize", + 8, + 1, + "res_multistep", + "simple", + 1 + ] + } + ], + "groups": [ + { + "id": 2, + "title": "Step2 - Image size", + "bounding": [100, 560, 290, 200], + "color": "#3f789e", + "flags": {} + }, + { + "id": 3, + "title": "Step3 - Prompt", + "bounding": [410, 130, 450, 540], + "color": "#3f789e", + "flags": {} + }, + { + "id": 4, + "title": "Step1 - Load models", + "bounding": [100, 130, 290, 413.6], + "color": "#3f789e", + "flags": {} + } + ], + "links": [ + { + "id": 32, + "origin_id": 27, + "origin_slot": 0, + "target_id": 33, + "target_slot": 0, + "type": "CONDITIONING" + }, + { + "id": 26, + "origin_id": 28, + "origin_slot": 0, + "target_id": 11, + "target_slot": 0, + "type": "MODEL" + }, + { + "id": 14, + "origin_id": 3, + "origin_slot": 0, + "target_id": 8, + "target_slot": 0, + "type": "LATENT" + }, + { + "id": 27, + "origin_id": 29, + "origin_slot": 0, + "target_id": 8, + "target_slot": 1, + "type": "VAE" + }, + { + "id": 13, + "origin_id": 11, + "origin_slot": 0, + "target_id": 3, + "target_slot": 0, + "type": "MODEL" + }, + { + "id": 30, + "origin_id": 27, + "origin_slot": 0, + "target_id": 3, + "target_slot": 1, + "type": "CONDITIONING" + }, + { + "id": 33, + "origin_id": 33, + "origin_slot": 0, + "target_id": 3, + "target_slot": 2, + "type": "CONDITIONING" + }, + { + "id": 17, + "origin_id": 13, + "origin_slot": 0, + "target_id": 3, + "target_slot": 3, + "type": "LATENT" + }, + { + "id": 28, + "origin_id": 30, + "origin_slot": 0, + "target_id": 27, + "target_slot": 0, + "type": "CLIP" + }, + { + "id": 16, + "origin_id": 8, + "origin_slot": 0, + "target_id": -20, + "target_slot": 0, + "type": "IMAGE" + }, + { + "id": 34, + "origin_id": -10, + "origin_slot": 0, + "target_id": 27, + "target_slot": 1, + "type": "STRING" + }, + { + "id": 35, + "origin_id": -10, + "origin_slot": 1, + "target_id": 13, + "target_slot": 0, + "type": "INT" + }, + { + "id": 36, + "origin_id": -10, + "origin_slot": 2, + "target_id": 13, + "target_slot": 1, + "type": "INT" + }, + { + "id": 38, + "origin_id": -10, + "origin_slot": 3, + "target_id": 28, + "target_slot": 0, + "type": "COMBO" + }, + { + "id": 39, + "origin_id": -10, + "origin_slot": 4, + "target_id": 30, + "target_slot": 0, + "type": "COMBO" + }, + { + "id": 40, + "origin_id": -10, + "origin_slot": 5, + "target_id": 29, + "target_slot": 0, + "type": "COMBO" + }, + { + "id": 70, + "origin_id": -10, + "origin_slot": 6, + "target_id": 3, + "target_slot": 4, + "type": "INT" + } + ], + "extra": { + "workflowRendererVersion": "LG" + } + } + ] + }, + "config": {}, + "extra": { + "ds": { + "scale": 0.6488294314381271, + "offset": [733, 392.7886597938144] + }, + "frontendVersion": "1.43.4", + "workflowRendererVersion": "LG", + "VHS_latentpreview": false, + "VHS_latentpreviewrate": 0, + "VHS_MetadataImage": true, + "VHS_KeepIntermediate": true + }, + "version": 0.4 +} diff --git a/browser_tests/tests/subgraphLifecycle.spec.ts b/browser_tests/tests/subgraphLifecycle.spec.ts index c8bbe28eb2..50aa7f4a79 100644 --- a/browser_tests/tests/subgraphLifecycle.spec.ts +++ b/browser_tests/tests/subgraphLifecycle.spec.ts @@ -142,12 +142,12 @@ test.describe( }) }) - test.describe('Placeholder Behavior After Promoted Source Removal', () => { + test.describe('Cleanup Behavior After Promoted Source Removal', () => { test.beforeEach(async ({ comfyPage }) => { await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top') }) - test('Removing promoted source node inside subgraph falls back to disconnected placeholder on exterior', async ({ + test('Removing promoted source node inside subgraph cleans up exterior proxyWidgets', async ({ comfyPage }) => { await comfyPage.workflow.loadWorkflow( @@ -182,8 +182,8 @@ test.describe( }) }) .toEqual({ - proxyWidgetCount: initialWidgets.length, - firstWidgetType: 'button' + proxyWidgetCount: 0, + firstWidgetType: undefined }) }) diff --git a/browser_tests/tests/subgraphNestedPackValues.spec.ts b/browser_tests/tests/subgraphNestedPackValues.spec.ts new file mode 100644 index 0000000000..114b8d47d1 --- /dev/null +++ b/browser_tests/tests/subgraphNestedPackValues.spec.ts @@ -0,0 +1,195 @@ +import { + comfyPageFixture as test, + comfyExpect as expect +} from '../fixtures/ComfyPage' + +/** + * Regression test for PR #10532: + * Packing all nodes inside a subgraph into a nested subgraph was causing + * the parent subgraph node's promoted widget values to go blank. + * + * Root cause: SubgraphNode had two sets of PromotedWidgetView references ā€” + * node.widgets (rebuilt from the promotion store) vs input._widget (cached + * at promotion time). After repointing, input._widget still pointed to + * removed node IDs, causing missing-node failures and blank values on the + * next checkState cycle. + */ +test.describe( + 'Nested subgraph pack preserves promoted widget values', + { tag: ['@subgraph', '@widget'] }, + () => { + const WORKFLOW = 'subgraphs/nested-pack-promoted-values' + const HOST_NODE_ID = '57' + + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', true) + }) + + test('Promoted widget values persist after packing interior nodes into nested subgraph', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow(WORKFLOW) + await comfyPage.vueNodes.waitForNodes() + + const nodeLocator = comfyPage.vueNodes.getNodeLocator(HOST_NODE_ID) + await expect(nodeLocator).toBeVisible() + + // 1. Verify initial promoted widget values via Vue node DOM + const widthWidget = nodeLocator + .getByLabel('width', { exact: true }) + .first() + const heightWidget = nodeLocator + .getByLabel('height', { exact: true }) + .first() + const stepsWidget = nodeLocator + .getByLabel('steps', { exact: true }) + .first() + const textWidget = nodeLocator.getByRole('textbox', { name: 'prompt' }) + + const widthControls = + comfyPage.vueNodes.getInputNumberControls(widthWidget) + const heightControls = + comfyPage.vueNodes.getInputNumberControls(heightWidget) + const stepsControls = + comfyPage.vueNodes.getInputNumberControls(stepsWidget) + + await expect(async () => { + await expect(widthControls.input).toHaveValue('1024') + await expect(heightControls.input).toHaveValue('1024') + await expect(stepsControls.input).toHaveValue('8') + await expect(textWidget).toHaveValue(/Latina female/) + }).toPass({ timeout: 5000 }) + + // 2. Enter the subgraph via Vue node button + await comfyPage.vueNodes.enterSubgraph(HOST_NODE_ID) + expect(await comfyPage.subgraph.isInSubgraph()).toBe(true) + + // 3. Disable Vue nodes for canvas operations (select all + convert) + await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', false) + await comfyPage.nextFrame() + + // 4. Select all interior nodes and convert to nested subgraph + await comfyPage.canvas.click() + await comfyPage.canvas.press('Control+a') + await comfyPage.nextFrame() + + await comfyPage.page.evaluate(() => { + const canvas = window.app!.canvas + canvas.graph!.convertToSubgraph(canvas.selectedItems) + }) + await comfyPage.nextFrame() + + // 5. Navigate back to root graph and trigger a checkState cycle + await comfyPage.subgraph.exitViaBreadcrumb() + await comfyPage.canvas.click() + await comfyPage.nextFrame() + + // 6. Re-enable Vue nodes and verify values are preserved + await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', true) + await comfyPage.vueNodes.waitForNodes() + + const nodeAfter = comfyPage.vueNodes.getNodeLocator(HOST_NODE_ID) + await expect(nodeAfter).toBeVisible() + + const widthAfter = nodeAfter.getByLabel('width', { exact: true }).first() + const heightAfter = nodeAfter + .getByLabel('height', { exact: true }) + .first() + const stepsAfter = nodeAfter.getByLabel('steps', { exact: true }).first() + const textAfter = nodeAfter.getByRole('textbox', { name: 'prompt' }) + + const widthControlsAfter = + comfyPage.vueNodes.getInputNumberControls(widthAfter) + const heightControlsAfter = + comfyPage.vueNodes.getInputNumberControls(heightAfter) + const stepsControlsAfter = + comfyPage.vueNodes.getInputNumberControls(stepsAfter) + + await expect(async () => { + await expect(widthControlsAfter.input).toHaveValue('1024') + await expect(heightControlsAfter.input).toHaveValue('1024') + await expect(stepsControlsAfter.input).toHaveValue('8') + await expect(textAfter).toHaveValue(/Latina female/) + }).toPass({ timeout: 5000 }) + }) + + test('proxyWidgets entries resolve to valid interior nodes after packing', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow(WORKFLOW) + await comfyPage.vueNodes.waitForNodes() + + // Verify the host node is visible + const nodeLocator = comfyPage.vueNodes.getNodeLocator(HOST_NODE_ID) + await expect(nodeLocator).toBeVisible() + + // Enter the subgraph via Vue node button, then disable for canvas ops + await comfyPage.vueNodes.enterSubgraph(HOST_NODE_ID) + await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', false) + await comfyPage.nextFrame() + + await comfyPage.canvas.click() + await comfyPage.canvas.press('Control+a') + await comfyPage.nextFrame() + + await comfyPage.page.evaluate(() => { + const canvas = window.app!.canvas + canvas.graph!.convertToSubgraph(canvas.selectedItems) + }) + await comfyPage.nextFrame() + + await comfyPage.subgraph.exitViaBreadcrumb() + await comfyPage.canvas.click() + await comfyPage.nextFrame() + + // Verify all proxyWidgets entries resolve + await expect(async () => { + const result = await comfyPage.page.evaluate((hostId) => { + const graph = window.app!.graph! + const hostNode = graph.getNodeById(hostId) + if ( + !hostNode || + typeof hostNode.isSubgraphNode !== 'function' || + !hostNode.isSubgraphNode() + ) { + return { error: 'Host node not found or not a subgraph node' } + } + + const proxyWidgets = hostNode.properties?.proxyWidgets ?? [] + const entries = (proxyWidgets as unknown[]) + .filter( + (e): e is [string, string] => + Array.isArray(e) && + e.length >= 2 && + typeof e[0] === 'string' && + typeof e[1] === 'string' && + !e[1].startsWith('$$') + ) + .map(([nodeId, widgetName]) => { + const interiorNode = hostNode.subgraph.getNodeById(Number(nodeId)) + return { + nodeId, + widgetName, + resolved: interiorNode !== null && interiorNode !== undefined + } + }) + + return { entries, count: entries.length } + }, HOST_NODE_ID) + + expect(result).not.toHaveProperty('error') + const { entries, count } = result as { + entries: { nodeId: string; widgetName: string; resolved: boolean }[] + count: number + } + expect(count).toBeGreaterThan(0) + for (const entry of entries) { + expect( + entry.resolved, + `Widget "${entry.widgetName}" (node ${entry.nodeId}) should resolve` + ).toBe(true) + } + }).toPass({ timeout: 5000 }) + }) + } +) diff --git a/package.json b/package.json index c5b280ebf0..b38d269e44 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "stylelint:fix": "stylelint --cache --fix '{apps,packages,src}/**/*.{css,vue}'", "stylelint": "stylelint --cache '{apps,packages,src}/**/*.{css,vue}'", "test:browser": "pnpm exec nx e2e", - "test:browser:local": "cross-env PLAYWRIGHT_LOCAL=1 pnpm test:browser", + "test:browser:local": "cross-env PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:5173 pnpm test:browser", "test:unit": "nx run test", "typecheck": "vue-tsc --noEmit", "typecheck:browser": "vue-tsc --project browser_tests/tsconfig.json", diff --git a/src/lib/litegraph/src/LGraph.repointAncestorPromotions.test.ts b/src/lib/litegraph/src/LGraph.repointAncestorPromotions.test.ts new file mode 100644 index 0000000000..edd142aa64 --- /dev/null +++ b/src/lib/litegraph/src/LGraph.repointAncestorPromotions.test.ts @@ -0,0 +1,287 @@ +import { createTestingPinia } from '@pinia/testing' +import { setActivePinia } from 'pinia' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { + LGraphNode, + LiteGraph, + SubgraphNode +} from '@/lib/litegraph/src/litegraph' +import type { + ExportedSubgraphInstance, + Positionable, + Subgraph +} from '@/lib/litegraph/src/litegraph' +import { + createTestSubgraph, + createTestSubgraphNode, + resetSubgraphFixtureState +} from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers' +import { usePromotionStore } from '@/stores/promotionStore' + +/** + * Registers a minimal SubgraphNode class for a subgraph definition + * so that `LiteGraph.createNode(subgraphId)` works in tests. + */ +function registerSubgraphNodeType(subgraph: Subgraph): void { + const instanceData: ExportedSubgraphInstance = { + id: -1, + type: subgraph.id, + pos: [0, 0], + size: [100, 100], + inputs: [], + outputs: [], + flags: {}, + order: 0, + mode: 0 + } + + const node = class extends SubgraphNode { + constructor() { + super(subgraph.rootGraph, subgraph, instanceData) + } + } + Object.defineProperty(node, 'title', { value: subgraph.name }) + LiteGraph.registerNodeType(subgraph.id, node) +} + +const registeredTypes: string[] = [] + +afterEach(() => { + for (const type of registeredTypes) { + LiteGraph.unregisterNodeType(type) + } + registeredTypes.length = 0 +}) + +beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) + resetSubgraphFixtureState() +}) + +describe('_repointAncestorPromotions', () => { + function setupParentSubgraphWithWidgets() { + const parentSubgraph = createTestSubgraph({ + name: 'Parent Subgraph', + inputs: [{ name: 'input', type: '*' }], + outputs: [{ name: 'output', type: '*' }] + }) + const rootGraph = parentSubgraph.rootGraph + + // We need to listen for new subgraph registrations so + // LiteGraph.createNode works during convertToSubgraph + rootGraph.events.addEventListener('subgraph-created', (e) => { + const { subgraph } = e.detail + registerSubgraphNodeType(subgraph) + registeredTypes.push(subgraph.id) + }) + + const interiorNode = new LGraphNode('Interior Node') + interiorNode.addInput('in', '*') + interiorNode.addOutput('out', '*') + interiorNode.addWidget('text', 'prompt', 'hello world', () => {}) + parentSubgraph.add(interiorNode) + + // Create host SubgraphNode in root graph + registerSubgraphNodeType(parentSubgraph) + registeredTypes.push(parentSubgraph.id) + const hostNode = createTestSubgraphNode(parentSubgraph) + rootGraph.add(hostNode) + + return { rootGraph, parentSubgraph, interiorNode, hostNode } + } + + it('repoints parent promotions when interior nodes are packed into a nested subgraph', () => { + const { rootGraph, parentSubgraph, interiorNode, hostNode } = + setupParentSubgraphWithWidgets() + + // Promote the interior node's widget on the host + const store = usePromotionStore() + store.promote(rootGraph.id, hostNode.id, { + sourceNodeId: String(interiorNode.id), + sourceWidgetName: 'prompt' + }) + + const beforeEntries = store.getPromotions(rootGraph.id, hostNode.id) + expect(beforeEntries).toHaveLength(1) + expect(beforeEntries[0].sourceNodeId).toBe(String(interiorNode.id)) + + // Pack the interior node into a nested subgraph + const { node: nestedSubgraphNode } = parentSubgraph.convertToSubgraph( + new Set([interiorNode]) + ) + + // After conversion, the host's promotion should be repointed + const afterEntries = store.getPromotions(rootGraph.id, hostNode.id) + expect(afterEntries).toHaveLength(1) + expect(afterEntries[0].sourceNodeId).toBe(String(nestedSubgraphNode.id)) + expect(afterEntries[0].sourceWidgetName).toBe('prompt') + expect(afterEntries[0].disambiguatingSourceNodeId).toBe( + String(interiorNode.id) + ) + + // The nested subgraph node should also have the promotion + const nestedEntries = store.getPromotions( + rootGraph.id, + nestedSubgraphNode.id + ) + expect(nestedEntries).toHaveLength(1) + expect(nestedEntries[0].sourceNodeId).toBe(String(interiorNode.id)) + expect(nestedEntries[0].sourceWidgetName).toBe('prompt') + }) + + it('preserves promotions that reference non-moved nodes', () => { + const { rootGraph, parentSubgraph, interiorNode, hostNode } = + setupParentSubgraphWithWidgets() + + const remainingNode = new LGraphNode('Remaining Node') + remainingNode.addWidget('text', 'widget_b', 'b', () => {}) + parentSubgraph.add(remainingNode) + + const store = usePromotionStore() + store.promote(rootGraph.id, hostNode.id, { + sourceNodeId: String(interiorNode.id), + sourceWidgetName: 'prompt' + }) + store.promote(rootGraph.id, hostNode.id, { + sourceNodeId: String(remainingNode.id), + sourceWidgetName: 'widget_b' + }) + + // Pack only the interiorNode + parentSubgraph.convertToSubgraph(new Set([interiorNode])) + + const afterEntries = store.getPromotions(rootGraph.id, hostNode.id) + expect(afterEntries).toHaveLength(2) + + // The remaining node's promotion should be unchanged + const remainingEntry = afterEntries.find( + (e) => e.sourceWidgetName === 'widget_b' + ) + expect(remainingEntry?.sourceNodeId).toBe(String(remainingNode.id)) + expect(remainingEntry?.disambiguatingSourceNodeId).toBeUndefined() + + // The moved node's promotion should be repointed + const movedEntry = afterEntries.find((e) => e.sourceWidgetName === 'prompt') + expect(movedEntry?.sourceNodeId).not.toBe(String(interiorNode.id)) + expect(movedEntry?.disambiguatingSourceNodeId).toBe(String(interiorNode.id)) + }) + + it('does not modify promotions when converting in root graph', () => { + const parentSubgraph = createTestSubgraph({ name: 'Dummy' }) + const rootGraph = parentSubgraph.rootGraph + + rootGraph.events.addEventListener('subgraph-created', (e) => { + const { subgraph } = e.detail + registerSubgraphNodeType(subgraph) + registeredTypes.push(subgraph.id) + }) + + const node = new LGraphNode('Root Node') + node.addInput('in', '*') + node.addOutput('out', '*') + node.addWidget('text', 'value', 'test', () => {}) + rootGraph.add(node) + + // Converting in root graph should not throw + rootGraph.convertToSubgraph(new Set([node])) + }) + + it('uses existing disambiguatingSourceNodeId as fallback on repeat packing', () => { + const { rootGraph, parentSubgraph, interiorNode, hostNode } = + setupParentSubgraphWithWidgets() + + const store = usePromotionStore() + store.promote(rootGraph.id, hostNode.id, { + sourceNodeId: String(interiorNode.id), + sourceWidgetName: 'prompt' + }) + + // First pack: interior node ā†’ nested subgraph + const { node: firstNestedNode } = parentSubgraph.convertToSubgraph( + new Set([interiorNode]) + ) + + const afterFirstPack = store.getPromotions(rootGraph.id, hostNode.id) + expect(afterFirstPack).toHaveLength(1) + expect(afterFirstPack[0].sourceNodeId).toBe(String(firstNestedNode.id)) + expect(afterFirstPack[0].disambiguatingSourceNodeId).toBe( + String(interiorNode.id) + ) + + // Second pack: nested subgraph ā†’ another level of nesting + const { node: secondNestedNode } = parentSubgraph.convertToSubgraph( + new Set([firstNestedNode]) + ) + + // After second pack, promotion should use the disambiguatingSourceNodeId + // as fallback and point to the new nested node + const afterSecondPack = store.getPromotions(rootGraph.id, hostNode.id) + expect(afterSecondPack).toHaveLength(1) + expect(afterSecondPack[0].sourceNodeId).toBe(String(secondNestedNode.id)) + expect(afterSecondPack[0].disambiguatingSourceNodeId).toBe( + String(interiorNode.id) + ) + }) + + it('repoints promotions for multiple host instances of the same subgraph', () => { + const parentSubgraph = createTestSubgraph({ + name: 'Shared Parent Subgraph', + inputs: [{ name: 'input', type: '*' }], + outputs: [{ name: 'output', type: '*' }] + }) + const rootGraph = parentSubgraph.rootGraph + + rootGraph.events.addEventListener('subgraph-created', (e) => { + const { subgraph } = e.detail + registerSubgraphNodeType(subgraph) + registeredTypes.push(subgraph.id) + }) + + const interiorNode = new LGraphNode('Interior Node') + interiorNode.addInput('in', '*') + interiorNode.addOutput('out', '*') + interiorNode.addWidget('text', 'prompt', 'shared', () => {}) + parentSubgraph.add(interiorNode) + + // Create TWO host SubgraphNodes pointing to the same subgraph + registerSubgraphNodeType(parentSubgraph) + registeredTypes.push(parentSubgraph.id) + + const hostNode1 = createTestSubgraphNode(parentSubgraph) + const hostNode2 = createTestSubgraphNode(parentSubgraph) + rootGraph.add(hostNode1) + rootGraph.add(hostNode2) + + // Promote on both hosts + const store = usePromotionStore() + store.promote(rootGraph.id, hostNode1.id, { + sourceNodeId: String(interiorNode.id), + sourceWidgetName: 'prompt' + }) + store.promote(rootGraph.id, hostNode2.id, { + sourceNodeId: String(interiorNode.id), + sourceWidgetName: 'prompt' + }) + + // Pack the interior node + const { node: nestedNode } = parentSubgraph.convertToSubgraph( + new Set([interiorNode]) + ) + + // Both hosts' promotions should be repointed to the nested node + const host1Promotions = store.getPromotions(rootGraph.id, hostNode1.id) + expect(host1Promotions).toHaveLength(1) + expect(host1Promotions[0].sourceNodeId).toBe(String(nestedNode.id)) + expect(host1Promotions[0].disambiguatingSourceNodeId).toBe( + String(interiorNode.id) + ) + + const host2Promotions = store.getPromotions(rootGraph.id, hostNode2.id) + expect(host2Promotions).toHaveLength(1) + expect(host2Promotions[0].sourceNodeId).toBe(String(nestedNode.id)) + expect(host2Promotions[0].disambiguatingSourceNodeId).toBe( + String(interiorNode.id) + ) + }) +}) diff --git a/src/lib/litegraph/src/LGraph.ts b/src/lib/litegraph/src/LGraph.ts index 68b80d9dc8..ce3522e83d 100644 --- a/src/lib/litegraph/src/LGraph.ts +++ b/src/lib/litegraph/src/LGraph.ts @@ -1,5 +1,6 @@ import { toString } from 'es-toolkit/compat' +import type { PromotedWidgetSource } from '@/core/graph/subgraph/promotedWidgetTypes' import { SUBGRAPH_INPUT_ID, SUBGRAPH_OUTPUT_ID @@ -9,7 +10,10 @@ import type { UUID } from '@/lib/litegraph/src/utils/uuid' import { createUuidv4, zeroUuid } from '@/lib/litegraph/src/utils/uuid' import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMutations' import { LayoutSource } from '@/renderer/core/layout/types' -import { usePromotionStore } from '@/stores/promotionStore' +import { + makePromotionEntryKey, + usePromotionStore +} from '@/stores/promotionStore' import { useWidgetValueStore } from '@/stores/widgetValueStore' import { forEachNode } from '@/utils/graphTraversalUtil' @@ -1952,6 +1956,13 @@ export class LGraph subgraphNode._setConcreteSlots() subgraphNode.arrange() + // Repair ancestor promotions: when nodes are packed into a nested + // subgraph, any host SubgraphNode whose proxyWidgets referenced the + // moved nodes must be repointed to chain through the new nested node. + if (!this.isRootGraph) { + this._repointAncestorPromotions(nodes, subgraphNode as SubgraphNode) + } + this.canvasAction((c) => c.canvas.dispatchEvent( new CustomEvent('subgraph-converted', { @@ -1964,6 +1975,75 @@ export class LGraph return { subgraph, node: subgraphNode as SubgraphNode } } + /** + * After packing nodes into a nested subgraph, repoint any ancestor + * SubgraphNode promotions that referenced the moved nodes so they + * chain through the newly created nested SubgraphNode. + */ + private _repointAncestorPromotions( + movedNodes: Set, + nestedSubgraphNode: SubgraphNode + ): void { + const movedNodeIds = new Set([...movedNodes].map((n) => String(n.id))) + const store = usePromotionStore() + const nestedNodeId = String(nestedSubgraphNode.id) + const graphId = this.rootGraph.id + const nestedEntries = store.getPromotions(graphId, nestedSubgraphNode.id) + const nextNestedEntries = [...nestedEntries] + const nestedEntryKeys = new Set( + nestedEntries.map((entry) => makePromotionEntryKey(entry)) + ) + const hostUpdates: Array<{ + node: SubgraphNode + entries: PromotedWidgetSource[] + }> = [] + + // Find all SubgraphNode instances that host `this` subgraph. + // They live in any graph and have `type === this.id`. + const allGraphs: LGraph[] = [ + this.rootGraph, + ...this.rootGraph._subgraphs.values() + ] + for (const graph of allGraphs) { + for (const node of graph._nodes) { + if (!node.isSubgraphNode() || node.type !== this.id) continue + + const entries = store.getPromotions(graphId, node.id) + const movedEntries = entries.filter((entry) => + movedNodeIds.has(entry.sourceNodeId) + ) + if (movedEntries.length === 0) continue + + for (const entry of movedEntries) { + const key = makePromotionEntryKey(entry) + if (nestedEntryKeys.has(key)) continue + nestedEntryKeys.add(key) + nextNestedEntries.push(entry) + } + + const nextEntries = entries.map((entry) => { + if (!movedNodeIds.has(entry.sourceNodeId)) return entry + return { + sourceNodeId: nestedNodeId, + sourceWidgetName: entry.sourceWidgetName, + disambiguatingSourceNodeId: + entry.disambiguatingSourceNodeId ?? entry.sourceNodeId + } + }) + + hostUpdates.push({ node, entries: nextEntries }) + } + } + + if (nextNestedEntries.length !== nestedEntries.length) + store.setPromotions(graphId, nestedSubgraphNode.id, nextNestedEntries) + + for (const { node, entries } of hostUpdates) { + store.setPromotions(graphId, node.id, entries) + node.rebuildInputWidgetBindings() + } + } + unpackSubgraph( subgraphNode: SubgraphNode, options?: { skipMissingNodes?: boolean } diff --git a/src/lib/litegraph/src/subgraph/SubgraphInputNode.ts b/src/lib/litegraph/src/subgraph/SubgraphInputNode.ts index fe0dc25c02..3687fd32e2 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphInputNode.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphInputNode.ts @@ -209,6 +209,14 @@ export class SubgraphInputNode link.id ) } + + if (subgraphInput.linkIds.length === 0) { + subgraphInput._widget = undefined + } + subgraphInput.events.dispatch('input-disconnected', { + input: subgraphInput + }) + const slotIndex = node.inputs.findIndex((inp) => inp === input) if (slotIndex !== -1) { node.onConnectionsChange?.( diff --git a/src/lib/litegraph/src/subgraph/SubgraphNode.ts b/src/lib/litegraph/src/subgraph/SubgraphNode.ts index f1dda6c476..f97b2a2bd3 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphNode.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphNode.ts @@ -882,8 +882,20 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph { usePromotionStore().demote(this.rootGraph.id, this.id, source) } - const didSetWidgetFromEvent = !input._widget - if (didSetWidgetFromEvent) + const boundWidget = + input._widget && isPromotedWidgetView(input._widget) + ? input._widget + : undefined + const hasStaleBoundWidget = + boundWidget && + this.subgraph + .getNodeById(boundWidget.sourceNodeId) + ?.widgets?.some( + (widget) => widget.name === boundWidget.sourceWidgetName + ) !== true + + const shouldSetWidgetFromEvent = !input._widget || hasStaleBoundWidget + if (shouldSetWidgetFromEvent) this._setWidget( subgraphInput, input, @@ -1107,6 +1119,27 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph { } } + /** + * Clears all cached promoted widget views and re-resolves `input._widget` + * bindings from the current subgraph connections. Called after ancestor + * promotions are repointed during nested subgraph packing. + */ + rebuildInputWidgetBindings(): void { + this._promotedViewManager.clear() + this._invalidatePromotedViewsCache() + + for (const input of this.inputs) { + delete input.widget + delete input.pos + input._widget = undefined + const subgraphInput = input._subgraphSlot + if (!subgraphInput) continue + this._resolveInputWidget(subgraphInput, input) + } + + this._syncPromotions() + } + private _resolveInputWidget( subgraphInput: SubgraphInput, input: INodeInputSlot diff --git a/src/lib/litegraph/src/subgraph/SubgraphWidgetPromotion.test.ts b/src/lib/litegraph/src/subgraph/SubgraphWidgetPromotion.test.ts index f1e360af00..52096a16d7 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphWidgetPromotion.test.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphWidgetPromotion.test.ts @@ -298,13 +298,14 @@ describe('SubgraphWidgetPromotion', () => { subgraph.add(vaeNode) const outerNode = createTestSubgraphNode(subgraph) + const keptSamplerNodeId = String(samplerNode.id) // Inject stale proxyWidgets referencing nodes that don't exist in // this subgraph (they were packed into a nested subgraph) outerNode.properties.proxyWidgets = [ ['999', 'text'], ['998', 'text'], - [String(samplerNode.id), 'widget'] + [keptSamplerNodeId, 'widget'] ] outerNode.configure(outerNode.serialize()) @@ -317,6 +318,7 @@ describe('SubgraphWidgetPromotion', () => { expect(widgetSourceIds).not.toContain('999') expect(widgetSourceIds).not.toContain('998') + expect(widgetSourceIds).toContain(keptSamplerNodeId) }) })