feat: video reference wins over image reference at the same slot index
For nodes with parallel image/video autogrow reference lists (MiniMax H3's ref_images/ref_videos and anything following the same naming convention): when both end up filled at the same index, only the video is sent -- letting you switch between image-as-reference and video-as-reference runs without manually clearing the one you're not using. Per-node opt-out toggle, on by default. Adversarial review (three rounds) found and fixed a real gap: dropping an image reference from the middle of a multi-reference list left a non-contiguous index sequence, which the V3 autogrow expansion this project already relies on cannot parse. Surviving entries are now re-indexed to close the gap. Also fixes an unrelated bug the same review surfaced in the Linked Inputs picker: clicking "Save link"/"Add" while an unrelated node-page edit sat buffered in the deferSave queue silently persisted that edit too, bypassing the normal unsaved-changes confirm. It now runs through the same confirm/discard path as leaving the page any other way. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
3022a341b0
commit
59f91a9e24
@@ -20,6 +20,7 @@ Output is a ComfyUI /prompt graph: {node_id: {class_type, inputs}}.
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import re
|
||||
import uuid
|
||||
from typing import Any, Optional
|
||||
|
||||
@@ -993,6 +994,35 @@ def build(object_info: dict, models_config: dict, params: dict, media: dict | No
|
||||
report["blocking"].append(msg)
|
||||
report["unsatisfied"].append(msg)
|
||||
|
||||
# 6. media-OR: when a node has a video reference AND its same-index image reference
|
||||
# both wired (e.g. you left an image-reference workflow's socket filled and then also
|
||||
# filled the matching video-reference one), the video wins and the image socket is
|
||||
# dropped — sending both is redundant at best and an unintended double reference at
|
||||
# worst. Opt-out per node (slot.media_or === False); on by default. Runs last, after
|
||||
# every wiring phase, so it only sees sockets that actually ended up filled.
|
||||
# Dropping ref_image_N alone would leave a gap if ref_image_(N+1) survives — ordinary
|
||||
# multi-reference use, not an edge case, for a list whose whole point is more than one
|
||||
# entry — and V3 autogrow expansion needs contiguous indices from 0. So each affected
|
||||
# list's survivors get re-indexed afterward to close any gap the drop left behind.
|
||||
reindex_groups: set[tuple[str, str, str]] = set()
|
||||
for s in slots:
|
||||
if s.get("media_or") is False:
|
||||
continue
|
||||
sid = slot_node_id.get(s["id"])
|
||||
if not sid or sid not in graph:
|
||||
continue
|
||||
node_inputs = graph[sid]["inputs"]
|
||||
for image_name, video_name in _media_or_pairs(slot_def.get(s["id"])):
|
||||
if isinstance(node_inputs.get(image_name), list) and isinstance(node_inputs.get(video_name), list):
|
||||
del node_inputs[image_name]
|
||||
report["wired"].append(
|
||||
f"{s.get('node_class')}.{image_name}: dropped — {video_name} is also "
|
||||
"filled and takes priority (video over image reference).")
|
||||
parent, child = image_name.split(".", 1)
|
||||
reindex_groups.add((sid, parent, re.sub(r"\d+$", "", child)))
|
||||
for sid, parent, prefix in reindex_groups:
|
||||
_reindex_autogrow_group(graph[sid]["inputs"], parent, prefix)
|
||||
|
||||
return graph, report
|
||||
|
||||
|
||||
@@ -1165,6 +1195,37 @@ def _canonical_input(node_def: Optional[dict], name: str) -> str:
|
||||
return name
|
||||
|
||||
|
||||
def _media_or_pairs(node_def: Optional[dict]) -> list[tuple[str, str]]:
|
||||
"""Same-index autogrow sockets that name the same reference slot once as IMAGE and
|
||||
once as VIDEO (MiniMax H3's ref_images.ref_image_0 / ref_videos.ref_video_0, and any
|
||||
node following the same convention) — found generically by swapping "image" for
|
||||
"video" in the dotted socket id, no per-node list to maintain."""
|
||||
names = {ci["name"] for ci in connection_inputs(node_def or {}) if ci.get("autogrow")}
|
||||
pairs = []
|
||||
for name in names:
|
||||
video_name = re.sub("image", "video", name, flags=re.IGNORECASE)
|
||||
if video_name != name and video_name in names:
|
||||
pairs.append((name, video_name))
|
||||
return pairs
|
||||
|
||||
|
||||
def _reindex_autogrow_group(node_inputs: dict, parent: str, prefix: str) -> None:
|
||||
"""Renumber `<parent>.<prefix><N>` entries still present in `node_inputs` to be
|
||||
contiguous from 0, preserving relative order — used after media-OR removes one entry
|
||||
from the middle of an autogrow list, which V3 expansion otherwise can't parse."""
|
||||
entries = []
|
||||
for key in node_inputs:
|
||||
if not key.startswith(parent + "."):
|
||||
continue
|
||||
suffix = key[len(parent) + 1:][len(prefix):]
|
||||
if key[len(parent) + 1:].startswith(prefix) and suffix.isdigit():
|
||||
entries.append((int(suffix), key))
|
||||
entries.sort()
|
||||
for new_idx, (old_idx, key) in enumerate(entries):
|
||||
if new_idx != old_idx:
|
||||
node_inputs[f"{parent}.{prefix}{new_idx}"] = node_inputs.pop(key)
|
||||
|
||||
|
||||
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:])
|
||||
|
||||
@@ -173,6 +173,20 @@
|
||||
}
|
||||
}
|
||||
|
||||
// Same-index autogrow sockets naming the same reference slot once as IMAGE and once as
|
||||
// VIDEO (MiniMax H3's ref_images.ref_image_0 / ref_videos.ref_video_0, and anything else
|
||||
// following the convention) — mirrors backend/builder.py's _media_or_pairs so the toggle
|
||||
// only appears where the backend will actually act on it.
|
||||
function mediaOrPairs(connectionInputs) {
|
||||
const names = new Set((connectionInputs || []).filter((ci) => ci.autogrow).map((ci) => ci.name));
|
||||
const pairs = [];
|
||||
names.forEach((name) => {
|
||||
const videoName = name.replace(/image/gi, "video");
|
||||
if (videoName !== name && names.has(videoName)) pairs.push([name, videoName]);
|
||||
});
|
||||
return pairs;
|
||||
}
|
||||
|
||||
function allowedSources(slot, ci) {
|
||||
const all = sources(slot, ci.type);
|
||||
if (!pipelineLocked()) return all;
|
||||
@@ -880,6 +894,16 @@
|
||||
});
|
||||
if (advBox) sbox.append(advBox);
|
||||
card.append(sbox);
|
||||
|
||||
if (mediaOrPairs(cand.connection_inputs).length) {
|
||||
const orRow = el("label", "field node-media-or-field");
|
||||
const cb = el("input"); cb.type = "checkbox"; cb.style.width = "auto";
|
||||
cb.checked = slot.media_or !== false;
|
||||
cb.onchange = async () => { slot.media_or = cb.checked ? undefined : false; await persist(); };
|
||||
orRow.append(cb);
|
||||
orRow.append(el("span", null, "When both an image and a video reference are filled at the same slot, send only the video"));
|
||||
card.append(orRow);
|
||||
}
|
||||
}
|
||||
|
||||
if (cand && cand.inputs.length) {
|
||||
@@ -1900,6 +1924,7 @@
|
||||
|
||||
async function saveLinkSelection() {
|
||||
if (linkSel.length < 1) return;
|
||||
if (!settleForegroundEdit()) return;
|
||||
if (linkTarget) return addLinkSelection();
|
||||
const def = "size " + ((config.links || []).length + 1);
|
||||
const nm = prompt("Link name:", def); if (nm == null) return;
|
||||
@@ -2183,6 +2208,24 @@
|
||||
_setView(v);
|
||||
}
|
||||
|
||||
// Link picking happens ON a node's own page (via beginEdit()'s deferSave buffer), which
|
||||
// means an unrelated edit to that node's own fields can be sitting buffered when the
|
||||
// user hits "Save link"/"Add". Previously that edit got persisted silently along with
|
||||
// the link — the same dirty-change decision setView() already confirms before leaving a
|
||||
// page, applied here since saving a link is also a page-leaving action. Discards the
|
||||
// stray edit (not the link, which the caller creates afterward) on confirm; returns
|
||||
// false to abort the save if the user declines.
|
||||
function settleForegroundEdit() {
|
||||
if (!deferDirty) return true;
|
||||
if (!confirm(
|
||||
"This node has unsaved changes that aren't part of the link.\n\n"
|
||||
+ "Discard them and save the link? Cancel to go back and save or discard them first."
|
||||
)) return false;
|
||||
restoreBaseline();
|
||||
deferDirty = false;
|
||||
return true;
|
||||
}
|
||||
|
||||
const mnItem = (opts) => window.SettingsWindow.navItem(opts);
|
||||
|
||||
function sideBar(v) {
|
||||
|
||||
@@ -649,6 +649,9 @@ REF_OI["MiniMaxH3ReferenceToVideo"] = {
|
||||
"ref_images": ["COMFY_AUTOGROW_V3", {
|
||||
"template": {"input": {"optional": {"ref_image": ["IMAGE", {}]}},
|
||||
"prefix": "ref_image", "min": 0, "max": 4}}],
|
||||
"ref_videos": ["COMFY_AUTOGROW_V3", {
|
||||
"template": {"input": {"optional": {"ref_video": ["VIDEO", {}]}},
|
||||
"prefix": "ref_video", "min": 0, "max": 4}}],
|
||||
}},
|
||||
"output": ["CONDITIONING"], "output_name": ["conditioning"]}
|
||||
|
||||
@@ -761,6 +764,71 @@ REF_OI["LoadVideo"] = {"input": {"required": {"file": [["a.mp4"]]}}, "output": [
|
||||
"output_name": ["VIDEO"]}
|
||||
|
||||
|
||||
def test_media_or_drops_the_image_when_its_paired_video_is_also_filled():
|
||||
"""Same reference index wired to both an IMAGE and a VIDEO autogrow socket: sending
|
||||
both is redundant at best, so the video wins and the image socket is dropped."""
|
||||
models = {"slots": [
|
||||
{"id": "li", "node_class": "LoadImage", "inputs": {}, "wires": {}},
|
||||
{"id": "lv", "node_class": "LoadVideo", "inputs": {}, "wires": {}},
|
||||
{"id": "r", "node_class": "MiniMaxH3ReferenceToVideo", "inputs": {"prompt": "hi"},
|
||||
"wires": {},
|
||||
"input_sources": {"ref_images.ref_image0": "out:li:IMAGE", "ref_videos.ref_video0": "out:lv:VIDEO"}},
|
||||
]}
|
||||
graph, report = builder.build(REF_OI, models, PARAMS, media={"filename": "scene.png"})
|
||||
ins = graph["slot_r"]["inputs"]
|
||||
assert "ref_images.ref_image0" not in ins
|
||||
assert ins["ref_videos.ref_video0"] == ["slot_lv", 0]
|
||||
assert any("ref_images.ref_image0" in m and "dropped" in m for m in report["wired"])
|
||||
|
||||
|
||||
def test_media_or_leaves_a_lone_image_reference_alone():
|
||||
"""No paired video wired at that index — the image reference is untouched."""
|
||||
models = {"slots": [
|
||||
{"id": "li", "node_class": "LoadImage", "inputs": {}, "wires": {}},
|
||||
{"id": "r", "node_class": "MiniMaxH3ReferenceToVideo", "inputs": {"prompt": "hi"},
|
||||
"wires": {}, "input_sources": {"ref_images.ref_image0": "out:li:IMAGE"}},
|
||||
]}
|
||||
graph, _ = builder.build(REF_OI, models, PARAMS, media={"filename": "scene.png"})
|
||||
assert graph["slot_r"]["inputs"]["ref_images.ref_image0"] == ["slot_li", 0]
|
||||
|
||||
|
||||
def test_media_or_reindexes_survivors_to_close_the_gap():
|
||||
"""Two image refs (0 and 1, ordinary multi-reference use) plus one video ref at index
|
||||
0: dropping ref_image0 must not leave ref_image1 stranded at a non-zero index — V3
|
||||
autogrow expansion wants the list contiguous from 0."""
|
||||
models = {"slots": [
|
||||
{"id": "li0", "node_class": "LoadImage", "inputs": {}, "wires": {}},
|
||||
{"id": "li1", "node_class": "LoadImage", "inputs": {}, "wires": {}},
|
||||
{"id": "lv", "node_class": "LoadVideo", "inputs": {}, "wires": {}},
|
||||
{"id": "r", "node_class": "MiniMaxH3ReferenceToVideo", "inputs": {"prompt": "hi"},
|
||||
"wires": {}, "input_sources": {
|
||||
"ref_images.ref_image0": "out:li0:IMAGE",
|
||||
"ref_images.ref_image1": "out:li1:IMAGE",
|
||||
"ref_videos.ref_video0": "out:lv:VIDEO"}},
|
||||
]}
|
||||
graph, _ = builder.build(REF_OI, models, PARAMS, media={"filename": "scene.png"})
|
||||
ins = graph["slot_r"]["inputs"]
|
||||
assert "ref_images.ref_image1" not in ins # no gap left behind
|
||||
assert ins["ref_images.ref_image0"] == ["slot_li1", 0] # survivor re-indexed down
|
||||
assert ins["ref_videos.ref_video0"] == ["slot_lv", 0] # untouched, its own list has no gap
|
||||
|
||||
|
||||
def test_media_or_can_be_switched_off_per_node():
|
||||
"""slot.media_or === False opts a node out — both sockets stay, letting the node see
|
||||
(and presumably reject or otherwise handle) both references itself."""
|
||||
models = {"slots": [
|
||||
{"id": "li", "node_class": "LoadImage", "inputs": {}, "wires": {}},
|
||||
{"id": "lv", "node_class": "LoadVideo", "inputs": {}, "wires": {}},
|
||||
{"id": "r", "node_class": "MiniMaxH3ReferenceToVideo", "inputs": {"prompt": "hi"},
|
||||
"wires": {}, "media_or": False,
|
||||
"input_sources": {"ref_images.ref_image0": "out:li:IMAGE", "ref_videos.ref_video0": "out:lv:VIDEO"}},
|
||||
]}
|
||||
graph, _ = builder.build(REF_OI, models, PARAMS, media={"filename": "scene.png"})
|
||||
ins = graph["slot_r"]["inputs"]
|
||||
assert ins["ref_images.ref_image0"] == ["slot_li", 0]
|
||||
assert ins["ref_videos.ref_video0"] == ["slot_lv", 0]
|
||||
|
||||
|
||||
def _ref_params(*refs):
|
||||
p = dict(PARAMS)
|
||||
p["references"] = [dict(r, index=i + 1) for i, r in enumerate(refs)]
|
||||
|
||||
Reference in New Issue
Block a user