fix: autogrow entries are addressed by their dotted socket id
ComfyUI expands an autogrow list into sockets named <parent>.<template name> — 'ref_images.ref_image_0' — and splits that path to rebuild the node's list. We sent the bare template name, which validation reports as a missing required input and, where it slips past, arrives at execute() as an unexpected keyword argument. Verified both ways against a live ComfyUI: the bare name is rejected, the dotted one queues and executes. The UI keeps showing the short name; only the wire key changed. Configs saved under the old bare name are renamed in place on load, and the builder maps any that remain, so an existing pipeline doesn't keep failing every run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
22607b0c14
commit
eb799743fe
@@ -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:])
|
||||
|
||||
@@ -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)
|
||||
]
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user