diff --git a/movie_editor/backend/builder.py b/movie_editor/backend/builder.py index 7fb384a3..20f2614a 100644 --- a/movie_editor/backend/builder.py +++ b/movie_editor/backend/builder.py @@ -539,6 +539,7 @@ def build(object_info: dict, models_config: dict, params: dict, media: dict | No sid = slot_node_id[s["id"]] nd_s = slot_def[s["id"]] for ci_name, source in (s.get("input_sources") or {}).items(): + ci_name = _canonical_input(nd_s, ci_name) if not source or source == "auto": continue if source == "timeline": @@ -633,6 +634,9 @@ def build(object_info: dict, models_config: dict, params: dict, media: dict | No report["unsatisfied"].append(f"{s.get('node_class')}.{out_name}: wire target '{t}' could not be resolved.") continue dnode, dinput = dst + # A wire authored into an autogrow entry names it the same way its own page + # does; canonicalise so both directions land on the dotted socket id. + dinput = _canonical_input(object_info.get((graph.get(dnode) or {}).get("class_type")), dinput) if dnode not in graph: # e.g. a core-port target while the built-in pipeline is disabled report["unsatisfied"].append(f"{s.get('node_class')}.{out_name}: target '{t}' node is not in the graph (built-in pipeline disabled?).") continue @@ -848,6 +852,22 @@ def _resolve_source(source: str, slot_node_id: dict, slot_def: dict, object_info return None +def _canonical_input(node_def: Optional[dict], name: str) -> str: + """The socket id ComfyUI expects for `name` on this node. + + Autogrow entries are addressed by their dotted path ("ref_images.ref_image_0"). An + editor config saved before that was known holds the bare template name, and sending + that reaches the node as an unexpected keyword argument — so map it back rather than + letting an old config keep failing the run. + """ + if not node_def or "." in name: + return name + for ci in connection_inputs(node_def): + if ci.get("autogrow") and ci["name"].rsplit(".", 1)[-1] == name: + return ci["name"] + return name + + def _resolve_target(target: str, port_to_core, slot_node_id) -> Optional[tuple[str, str]]: if target.startswith("port:"): return port_to_core.get(target[5:]) diff --git a/movie_editor/backend/nodes.py b/movie_editor/backend/nodes.py index 7fcf5307..d51addf6 100644 --- a/movie_editor/backend/nodes.py +++ b/movie_editor/backend/nodes.py @@ -141,7 +141,14 @@ def _autogrow_element_type(opts: dict) -> str | None: def _autogrow_children(name: str, opts: dict) -> list[dict]: """Expand an autogrow input into its indexed sockets. Empty when the template can't be - read — the caller then falls back to the raw (unwireable) input so nothing regresses.""" + read — the caller then falls back to the raw (unwireable) input so nothing regresses. + + The socket id ComfyUI expects is the PARENT id, a dot, then the template name + ("ref_images.ref_image_0") — that dotted path is what it splits to rebuild the node's + list. Sending the bare name is rejected as a missing input, and any that slips past + validation reaches execute() as an unexpected keyword argument. `display` carries the + short name for the UI, which has no reason to show the plumbing. + """ t = _autogrow_element_type(opts) names = _autogrow_names(opts) if not t or not names: @@ -149,7 +156,7 @@ def _autogrow_children(name: str, opts: dict) -> list[dict]: return [ # Always optional: the API graph carries only the indices that are actually wired, # and ComfyUI grows the schema to match, so an unwired index must never block. - {"name": nm, "type": _normalize_type(t), "required": False, + {"name": f"{name}.{nm}", "display": nm, "type": _normalize_type(t), "required": False, "autogrow": {"parent": name, "index": i}} for i, nm in enumerate(names) ] diff --git a/movie_editor/frontend/models.js b/movie_editor/frontend/models.js index 51903d87..9c41210e 100644 --- a/movie_editor/frontend/models.js +++ b/movie_editor/frontend/models.js @@ -524,7 +524,7 @@ sbox.append(el("div", "wire-title", "Input sources")); visibleConnectionInputs(slot, cand.connection_inputs).forEach((ci) => { const row = el("div", "wire-row"); - const lbl = el("span", "wire-out", `${ci.name} (${ci.type})`); + const lbl = el("span", "wire-out", `${ci.display || ci.name} (${ci.type})`); if (ci.autogrow) lbl.title = `Entry ${ci.autogrow.index + 1} of the node's "${ci.autogrow.parent}" list — fill one and the next appears.`; row.append(lbl); @@ -567,7 +567,7 @@ config.slots.filter((s) => s.id !== slot.id).forEach((s2) => { const c2 = specFor(s2); visibleConnectionInputs(s2, c2?.connection_inputs).filter((ci) => typeAccepts(ci.type, type)).forEach((ci) => - out.push({ value: `node:${s2.id}:${ci.name}`, label: `${slotFullLabel(s2)} · ${ci.name}` })); + out.push({ value: `node:${s2.id}:${ci.name}`, label: `${slotFullLabel(s2)} · ${ci.display || ci.name}` })); // Widget inputs can also receive a connection (ComfyUI converts a widget to an // input when wired) — e.g. EmptyLatentVideo.width/height. Offer them as targets. (c2?.inputs || []).filter((w) => _KIND2T[w.kind] === type).forEach((w) => @@ -586,9 +586,43 @@ } // Autogrow list inputs (MiniMax H3's ref_images, ref_videos, …) reach us already expanded - // into one socket per index — ref_image0 … ref_image9. Showing all ten of each list would - // bury the node, so the list renders what's filled plus one empty slot and grows as it is - // used, the way ComfyUI's own canvas grows it. + // into one socket per index, addressed by their dotted path (ref_images.ref_image_0). + // Showing all ten of each list would bury the node, so the list renders what's filled plus + // one empty slot and grows as it is used, the way ComfyUI's own canvas grows it. + // Autogrow entries were first saved under the bare template name (ref_image_0) before the + // dotted socket id turned out to be what ComfyUI accepts. Rename them in place on load — + // the edge is the same edge, and leaving the old key in a config keeps failing the run. + function migrateAutogrowNames() { + let changed = false; + const canonical = (slot, name) => { + if (!name || name.includes(".")) return name; + const ci = ((specFor(slot) || {}).connection_inputs || []) + .find((c) => c.autogrow && c.display === name); + return ci ? ci.name : name; + }; + (config.slots || []).forEach((slot) => { + const srcs = slot.input_sources || {}; + Object.keys(srcs).forEach((k) => { + const nu = canonical(slot, k); + if (nu !== k) { srcs[nu] = srcs[k]; delete srcs[k]; changed = true; } + }); + }); + (config.slots || []).forEach((slot) => { + Object.entries(slot.wires || {}).forEach(([out, raw]) => { + const next = wireTargets(raw).map((t) => { + const parsed = _parseNodeTarget(t); + if (!parsed) return t; + const dest = slotById(parsed.slotId); + if (!dest) return t; + const nu = canonical(dest, parsed.input); + return nu === parsed.input ? t : `node:${parsed.slotId}:${nu}`; + }); + if (next.some((t, i) => t !== wireTargets(raw)[i])) { slot.wires[out] = next; changed = true; } + }); + }); + return changed; + } + function visibleConnectionInputs(slot, cis) { const lastFed = {}; (cis || []).forEach((ci) => { @@ -863,7 +897,7 @@ sbox.append(el("div", "wire-title", "Input sources")); visibleConnectionInputs(draftSlot, curCand.connection_inputs).forEach((ci) => { const row = el("div", "wire-row"); - row.append(el("span", "wire-out", `${ci.name} (${ci.type})`)); + row.append(el("span", "wire-out", `${ci.display || ci.name} (${ci.type})`)); row.append(el("span", "wire-arrow", "←")); const sel = el("select", "wire-select"); const cur = draft.input_sources[ci.name] || ""; @@ -1454,7 +1488,9 @@ // Backfill numeric bounds/step (and combo choices) onto controls exposed before this // metadata was captured, so opening Models once upgrades existing projects. persist() // dispatches funpack-models-changed → the store reloads and the inspector re-renders. - if (refreshExposedChoices()) { try { await persist(); } catch (_) {} } + let dirty = refreshExposedChoices(); + if (migrateAutogrowNames()) dirty = true; + if (dirty) { try { await persist(); } catch (_) {} } } function mount(body, ctx) { diff --git a/movie_editor/tests/test_builder.py b/movie_editor/tests/test_builder.py index 919bf58a..9c22ad8b 100644 --- a/movie_editor/tests/test_builder.py +++ b/movie_editor/tests/test_builder.py @@ -572,15 +572,16 @@ def test_autogrow_entries_are_wired_by_their_expanded_names(): models = {"slots": [ {"id": "li", "node_class": "LoadImage", "inputs": {}, "wires": {}}, {"id": "r", "node_class": "MiniMaxH3ReferenceToVideo", "inputs": {"prompt": "hi"}, - "wires": {}, "input_sources": {"ref_image0": "out:li:IMAGE", "ref_image1": "timeline"}}, + "wires": {}, + "input_sources": {"ref_images.ref_image0": "out:li:IMAGE", "ref_images.ref_image1": "timeline"}}, ]} graph, report = builder.build(REF_OI, models, PARAMS, media={"filename": "scene.png"}) ins = graph["slot_r"]["inputs"] - assert ins["ref_image0"] == ["slot_li", 0] - assert ins["ref_image1"] == ["media_load", 0] - assert "ref_images" not in ins # the template name is never sent + assert ins["ref_images.ref_image0"] == ["slot_li", 0] + assert ins["ref_images.ref_image1"] == ["media_load", 0] + assert "ref_images" not in ins # the template name itself is never sent # an unwired entry is simply absent — never blocking, never an empty placeholder - assert "ref_image2" not in ins and "ref_image3" not in ins + assert "ref_images.ref_image2" not in ins and "ref_images.ref_image3" not in ins assert not any("ref_image" in m for m in report["blocking"]) @@ -622,3 +623,19 @@ def test_a_core_combo_value_missing_on_this_machine_falls_back_instead_of_blocki graph, report = builder.build(oi, {"slots": []}, params) assert graph["sampler"]["inputs"]["identity_projector"] == "None" assert any("identity_projector" in m and "not installed" in m for m in report["unsatisfied"]) + + +def test_an_autogrow_entry_saved_under_its_bare_name_still_wires(): + """Configs written before the dotted socket id was known hold "ref_image0". Sending that + is what produced "execute() got an unexpected keyword argument" — map it back instead of + letting an old config keep failing every run.""" + models = {"slots": [ + {"id": "li", "node_class": "LoadImage", "inputs": {}, "wires": {"IMAGE": "node:r:ref_image1"}}, + {"id": "r", "node_class": "MiniMaxH3ReferenceToVideo", "inputs": {}, "wires": {}, + "input_sources": {"ref_image0": "out:li:IMAGE"}}, + ]} + graph, _ = builder.build(REF_OI, models, PARAMS) + ins = graph["slot_r"]["inputs"] + assert ins["ref_images.ref_image0"] == ["slot_li", 0] # via input_sources + assert ins["ref_images.ref_image1"] == ["slot_li", 0] # via the mirrored wire + assert "ref_image0" not in ins and "ref_image1" not in ins diff --git a/movie_editor/tests/test_nodes.py b/movie_editor/tests/test_nodes.py index eab1307d..b6b44b9c 100644 --- a/movie_editor/tests/test_nodes.py +++ b/movie_editor/tests/test_nodes.py @@ -223,11 +223,15 @@ def test_autogrow_list_input_expands_into_wireable_sockets(): desc = nodes.describe_node(_autogrow_oi(), "MiniMaxH3ReferenceToVideo") ci = {c["name"]: c for c in desc["connection_inputs"]} assert "ref_images" not in ci and "ref_audios" not in ci - assert [ci[n]["type"] for n in ("ref_image0", "ref_image1", "ref_image2")] == ["IMAGE"] * 3 - assert ci["ref_image0"]["autogrow"] == {"parent": "ref_images", "index": 0} + # the socket id is the dotted path ComfyUI splits to rebuild the list — the bare name + # is rejected as missing, and reaches execute() as an unexpected keyword argument + names = ("ref_images.ref_image0", "ref_images.ref_image1", "ref_images.ref_image2") + assert [ci[n]["type"] for n in names] == ["IMAGE"] * 3 + assert ci["ref_images.ref_image0"]["autogrow"] == {"parent": "ref_images", "index": 0} + assert ci["ref_images.ref_image0"]["display"] == "ref_image0" # UI shows the short name # explicit-name templates keep their own names, in order - assert ci["ref_audio_a"]["type"] == "AUDIO" - assert ci["ref_audio_b"]["autogrow"] == {"parent": "ref_audios", "index": 1} + assert ci["ref_audios.ref_audio_a"]["type"] == "AUDIO" + assert ci["ref_audios.ref_audio_b"]["autogrow"] == {"parent": "ref_audios", "index": 1} # never required: only the wired indices are sent, so an empty one must not block assert not any(ci[n]["required"] for n in ci if n.startswith("ref_")) # the list wrapper is not a user widget either