From aa08b909f314dab73ea88df1f09db4510bcb7ed0 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Thu, 20 Aug 2026 19:09:33 +0300 Subject: [PATCH] feat(models): seed the LoRA loader; empty list passes the model through Empty is a working state now, logged as "No active LoRAs", so the hop every LoRA needs is already wired into a fresh pipeline. Co-Authored-By: Claude Opus 5 --- model_management.py | 13 +++++-- movie_editor/backend/pipeline_wiring.py | 42 ++++++++++++++++++++- movie_editor/frontend/models.js | 27 +++++++++++-- movie_editor/tests/test_builder.py | 22 ++++++++++- movie_editor/tests/test_pipeline_wiring.py | 44 ++++++++++++++++++++-- tests/test_loaders.py | 16 ++++++++ widgets.py | 11 +++++- 7 files changed, 160 insertions(+), 15 deletions(-) diff --git a/model_management.py b/model_management.py index fa7af3dc..532031da 100644 --- a/model_management.py +++ b/model_management.py @@ -199,8 +199,9 @@ def lora_row_fields(): ] -def lora_list_input(tooltip): - return list_widget("LoRA", lora_row_fields(), add_label="+ Add LoRA", tooltip=tooltip) +def lora_list_input(tooltip, allow_empty=False): + return list_widget("LoRA", lora_row_fields(), add_label="+ Add LoRA", tooltip=tooltip, + allow_empty=allow_empty) def stack_from_lora_list(value, per_block=False): @@ -571,7 +572,9 @@ class FunPackLoraLoader: "model": ("MODEL",), "lora_list": lora_list_input( "LoRAs to apply, top to bottom. Enough on its own — a stack is only " - "needed for prompt-specific trained weights."), + "needed for prompt-specific trained weights. Empty is fine: the model " + "passes straight through.", + allow_empty=True), }, "optional": { "clip": ("CLIP",), @@ -941,6 +944,10 @@ class FunPackLoraLoader: loras = list(stack.get("loras", [])) + own["loras"] per_block = coerce_bool(stack.get("per_block", False)) or coerce_bool(per_block) lora_stack = {**own, **stack, "per_block": per_block, "loras": loras} + if not loras: + # An empty loader is a wire, not an error: it hands the model on untouched so it + # can sit in the pipeline permanently, waiting for the run that wants a LoRA. + return (model, clip, lora_stack, "FunPack LoRA Loader | No active LoRAs") lines = [f"FunPack LoRA Loader | loading {len(loras)} LoRA(s)"] lines.append(f"Per-block application: {'enabled' if per_block else 'disabled'}") loaded_count = 0 diff --git a/movie_editor/backend/pipeline_wiring.py b/movie_editor/backend/pipeline_wiring.py index 01d96de5..03240554 100644 --- a/movie_editor/backend/pipeline_wiring.py +++ b/movie_editor/backend/pipeline_wiring.py @@ -398,11 +398,21 @@ def validate_models_wiring(models: Any) -> list[str]: # through DEFAULT_WIRES_BY_ROLE, the same table used when a node is added by hand. FUNPACK_DEFAULT_LOADERS: list[tuple[str, str, str, dict]] = [ ("unet", "FunPackDiffusionModelLoader", "Diffusion model", {}), + # Seeded empty, and empty means pass-through: the one node every LoRA needs is already + # in the chain, so using one is picking a file rather than adding and rewiring a node. + ("lora", "FunPackLoraLoader", "LoRAs", {"model": "out:fp_unet:MODEL"}), ("clip", "FunPackCLIPLoader", "Text encoder", {}), ("video_vae", "FunPackVAELoader", "Video VAE", {}), ("audio_vae", "FunPackVAELoader", "Audio VAE", {}), ] +# Wires for a seeded slot whose role has no default, or wants less than the role allows. +# The LoRA loader hands CLIP straight back untouched, so wiring it there would only add a +# hop — and a second source for the port the CLIP loader already feeds. +DEFAULT_SEED_WIRES: dict[str, dict[str, str]] = { + "lora": {"MODEL": "port:FunPackStudio.model"}, +} + # The rest of what a family needs before it can generate. Not loaders and not FunPack's, but # leaving them out would mean "the pipeline sets itself up" still ended in a required slot # the user has to find, add and wire by hand — which is the thing this is here to remove. @@ -460,7 +470,7 @@ def default_pipeline_slots(family: str = DEFAULT_FAMILY, node_def = (object_info or {}).get(cls) if node_def is None: continue # this ComfyUI does not have the node - role_wires = wires_by_role.get(role) or {} + role_wires = DEFAULT_SEED_WIRES.get(role) or wires_by_role.get(role) or {} # By output NAME first, then by type — the same rule the add-a-node path uses, and # necessary because the tables are keyed by type while a node names its own outputs. wires = {} @@ -472,10 +482,38 @@ def default_pipeline_slots(family: str = DEFAULT_FAMILY, continue # nothing to feed in this family: not a node, just clutter out.append({"id": f"fp_{role}", "role": role, "role_label": label, "node_class": cls, "inputs": declared_widget_defaults(node_def), - "input_sources": dict(sources), "wires": wires}) + "input_sources": dict(sources), "wires": wires, + "outputs": {o["name"]: o["type"] for o in node_outputs(node_def)}}) + for slot in out: + _claim_upstream_ports(slot, out) + for slot in out: + slot.pop("outputs", None) return out +def _claim_upstream_ports(slot: dict, slots: list[dict]) -> None: + """Move a port wire onto a seeded pass-through that sits in front of it. + + A slot that consumes type T from another seeded slot AND emits T is a link in that + chain, not a second branch off it: the producer must feed the pass-through, or both + would arrive at the same core port. Slots that merely consume T (an audio latent taking + a VAE) emit no T and so claim nothing. + """ + emits = set((slot.get("outputs") or {}).values()) + by_id = {s["id"]: s for s in slots} + for input_name, source in (slot.get("input_sources") or {}).items(): + parts = str(source).split(":") + if len(parts) != 3 or parts[0] != "out": + continue + producer = by_id.get(parts[1]) + if producer is None or producer is slot: + continue + out_type = (producer.get("outputs") or {}).get(parts[2]) + if out_type is None or out_type not in emits: + continue + producer["wires"][parts[2]] = [f"node:{slot['id']}:{input_name}"] + + def seed_default_pipeline(models: dict, object_info: Optional[dict] = None) -> dict: """Give a pipeline config that has never been set up FunPack's own loaders. diff --git a/movie_editor/frontend/models.js b/movie_editor/frontend/models.js index df1fbe6a..029dd65d 100644 --- a/movie_editor/frontend/models.js +++ b/movie_editor/frontend/models.js @@ -535,8 +535,9 @@ if (w.kind === "combo" && (!w.choices || !w.choices.length)) { issues.push({ level: "error", msg: `"${w.name}" has no installed options to pick from.` }); } else if (w.kind === "list") { - // "[]" passes the emptiness check below but means the node has nothing to load. - if (parseListValue(v != null ? v : w.default).length === 0) + // "[]" passes the emptiness check below but means the node has nothing to load — + // unless the node declares empty a working state, where it passes its input through. + if (!(w.list || {}).allow_empty && parseListValue(v != null ? v : w.default).length === 0) issues.push({ level: "error", msg: `"${w.name}" is empty — add at least one entry.` }); } else if (v == null || v === "") { // A linked input is FILLED at generate time from a project value, so the stored @@ -2173,7 +2174,10 @@ for (const recipe of missing) { const id = (config.slots || []).some((s) => s.id === recipe.id) ? recipe.id + "_" + Math.random().toString(36).slice(2, 7) : recipe.id; - config.slots.push({ ...recipe, id, inputs: { ...recipe.inputs } }); + const slot = { ...recipe, id, inputs: { ...recipe.inputs }, + input_sources: { ...(recipe.input_sources || {}) } }; + graftRecipeSources(slot); + config.slots.push(slot); } reconcileOpenPortWiring(); await persistNow(); @@ -2181,6 +2185,23 @@ render(); } + // A recipe's sources name the OTHER seeded slots by their fixed ids. Dropped into a + // pipeline that was not seeded, those ids may not be there — and where they are, the + // producer is already wired to the port this pass-through now takes over, which would + // leave the port with two sources. + function graftRecipeSources(slot) { + const outTypes = new Set(((specFor(slot) || {}).outputs || []).map((o) => o.type)); + Object.entries(slot.input_sources || {}).forEach(([input, src]) => { + const m = /^out:([^:]+):(.+)$/.exec(String(src)); + if (!m) return; + const producer = (config.slots || []).find((s) => s.id === m[1]); + if (!producer) { delete slot.input_sources[input]; return; } + const out = ((specFor(producer) || {}).outputs || []).find((o) => o.name === m[2]); + if (out && outTypes.has(out.type)) + (producer.wires = producer.wires || {})[m[2]] = [`node:${slot.id}:${input}`]; + }); + } + function loaderSection(slots, v) { const sec = el("div", "node-group node-group-pinned"); const head = el("div", "node-group-head"); diff --git a/movie_editor/tests/test_builder.py b/movie_editor/tests/test_builder.py index efe1f8bb..b65f311a 100644 --- a/movie_editor/tests/test_builder.py +++ b/movie_editor/tests/test_builder.py @@ -946,6 +946,14 @@ OI_DEFAULTS = dict(OI, **{ "frame_rate": ["FLOAT,INT", {"default": 25.0, "widgetType": "FLOAT"}], "audio_vae": ["VAE", {}]}}, "output": ["LATENT"], "output_name": ["Latent"]}, + "FunPackLoraLoader": { + "input": {"required": {"model": ["MODEL", {}], + "lora_list": ["STRING", {"default": "[]", + "funpack_list": {"allow_empty": True}}]}, + "optional": {"clip": ["CLIP", {}], + "per_block": ["BOOLEAN", {"default": False}]}}, + "output": ["MODEL", "CLIP", "FP_LORA_STACK", "STRING"], + "output_name": ["MODEL", "CLIP", "lora_stack", "status"]}, }) @@ -980,7 +988,7 @@ def test_a_freshly_seeded_pipeline_is_complete_for_i2v_too(): def test_the_seeded_loaders_reach_the_core_ports_they_were_wired_to(): graph, _ = builder.build(OI_DEFAULTS, _seeded_models(), PARAMS) by_class = {n["class_type"]: nid for nid, n in graph.items()} - assert graph["studio"]["inputs"]["model"] == [by_class["FunPackDiffusionModelLoader"], 0] + assert graph["studio"]["inputs"]["model"] == [by_class["FunPackLoraLoader"], 0] assert graph["studio"]["inputs"]["clip"] == [by_class["FunPackCLIPLoader"], 0] # two VAE loaders: the video one feeds the sampler, the audio one the audio decode video_vae = graph["sampler"]["inputs"]["vae"] @@ -990,6 +998,18 @@ def test_the_seeded_loaders_reach_the_core_ports_they_were_wired_to(): assert video_vae[0] != audio_vae[0] +def test_the_seeded_lora_loader_is_a_wire_the_model_passes_through(): + """It is seeded empty on purpose: the hop every LoRA needs is already in the graph, so + using one is picking a file — not adding a node and rewiring the model path.""" + graph, report = builder.build(OI_DEFAULTS, _seeded_models(), PARAMS) + by_class = {n["class_type"]: nid for nid, n in graph.items()} + lora = by_class["FunPackLoraLoader"] + assert graph[lora]["inputs"]["model"] == [by_class["FunPackDiffusionModelLoader"], 0] + assert graph[lora]["inputs"]["lora_list"] == "[]" + assert graph["studio"]["inputs"]["model"] == [lora, 0] + assert report["blocking"] == [] + + def test_the_seeded_audio_latent_follows_the_project_not_its_own_widgets(): graph, _ = builder.build(OI_DEFAULTS, _seeded_models(), PARAMS) audio = next(n for n in graph.values() if n["class_type"] == "LTXVEmptyLatentAudio") diff --git a/movie_editor/tests/test_pipeline_wiring.py b/movie_editor/tests/test_pipeline_wiring.py index 4806509b..79ff6229 100644 --- a/movie_editor/tests/test_pipeline_wiring.py +++ b/movie_editor/tests/test_pipeline_wiring.py @@ -231,19 +231,55 @@ OI_LOADERS = { "vae_name": [["v.safetensors"]], "dtype": [["default", "bf16"], {"default": "default"}]}}, "output": ["VAE"]}, + "FunPackLoraLoader": {"input": { + "required": {"model": ["MODEL", {}], + "lora_list": ["STRING", {"default": "[]", + "funpack_list": {"allow_empty": True}}]}, + "optional": {"clip": ["CLIP", {}]}}, + "output": ["MODEL", "CLIP", "FP_LORA_STACK", "STRING"], + "output_name": ["MODEL", "CLIP", "lora_stack", "status"]}, } def test_a_fresh_pipeline_is_funpacks_own_loaders_already_wired(): slots = pipeline_wiring.default_pipeline_slots("ltxav", OI_LOADERS) - assert [s["role"] for s in slots] == ["unet", "clip", "video_vae", "audio_vae"] + assert [s["role"] for s in slots] == ["unet", "lora", "clip", "video_vae", "audio_vae"] wired = {s["role"]: list(s["wires"].values())[0][0] for s in slots} - assert wired["unet"] == "port:FunPackStudio.model" + assert wired["lora"] == "port:FunPackStudio.model" assert wired["clip"] == "port:FunPackStudio.clip" assert wired["video_vae"] == "port:FunPackLTXAVSceneChainSampler.vae" assert wired["audio_vae"] == "port:LTXVAudioVAEDecode.audio_vae" +def test_the_seeded_lora_loader_sits_between_the_model_and_the_studio(): + """A LoRA has to be usable without adding and rewiring a node, so the hop is already + there. It is empty, and empty means the model passes straight through.""" + slots = {s["role"]: s for s in pipeline_wiring.default_pipeline_slots("ltxav", OI_LOADERS)} + assert slots["lora"]["input_sources"]["model"] == "out:fp_unet:MODEL" + assert slots["lora"]["inputs"]["lora_list"] == "[]" + # the diffusion loader now feeds the LoRA loader instead of the Studio directly, so the + # Studio's model port has exactly one source + assert slots["unet"]["wires"] == {"MODEL": ["node:fp_lora:model"]} + assert slots["lora"]["wires"] == {"MODEL": ["port:FunPackStudio.model"]} + + +def test_the_seeded_lora_loader_does_not_take_over_the_clip_path(): + """It hands CLIP back untouched, so wiring it there is a hop that only adds a second + source for the port the CLIP loader already feeds.""" + slots = {s["role"]: s for s in pipeline_wiring.default_pipeline_slots("ltxav", OI_LOADERS)} + assert "CLIP" not in slots["lora"]["wires"] + assert slots["clip"]["wires"] == {"CLIP": ["port:FunPackStudio.clip"]} + assert "clip" not in slots["lora"]["input_sources"] + + +def test_a_consumer_that_does_not_re_emit_the_type_claims_nothing(): + """The audio latent takes the audio VAE, but emits LATENT — the VAE still goes to the + decode port as well.""" + slots = {s["role"]: s for s in + pipeline_wiring.default_pipeline_slots("ltxav", OI_WITH_PLUMBING)} + assert slots["audio_vae"]["wires"] == {"VAE": ["port:LTXVAudioVAEDecode.audio_vae"]} + + def test_the_default_pipeline_follows_the_family(): """H3 decodes audio with core's node, so the audio VAE lands somewhere else entirely.""" slots = pipeline_wiring.default_pipeline_slots("minimax_h3", OI_LOADERS) @@ -266,7 +302,7 @@ def test_loaders_this_comfyui_does_not_have_are_not_seeded(): def test_seeding_happens_once_and_is_recorded(): models = {"slots": []} pipeline_wiring.seed_default_pipeline(models, OI_LOADERS) - assert models["defaults_seeded"] is True and len(models["slots"]) == 4 + assert models["defaults_seeded"] is True and len(models["slots"]) == 5 models["slots"] = [] pipeline_wiring.seed_default_pipeline(models, OI_LOADERS) assert models["slots"] == [] # emptied on purpose stays empty @@ -291,7 +327,7 @@ def test_seeding_is_deferred_when_the_node_schema_is_unavailable(): pipeline_wiring.seed_default_pipeline(models, None) assert "defaults_seeded" not in models pipeline_wiring.seed_default_pipeline(models, OI_LOADERS) - assert len(models["slots"]) == 4 + assert len(models["slots"]) == 5 OI_WITH_PLUMBING = dict(OI_LOADERS, **{ diff --git a/tests/test_loaders.py b/tests/test_loaders.py index 05bc277f..57a7d448 100644 --- a/tests/test_loaders.py +++ b/tests/test_loaders.py @@ -199,3 +199,19 @@ def test_a_list_becomes_the_same_stack_shape_apply_lora_weights_emits(): assert entry["type"] == "style" assert entry["model_weight"] == entry["base_model_weight"] == 0.8 assert entry["id"] == mm.lora_state_id("a.safetensors", "style") + + +def test_an_empty_lora_loader_is_a_wire_not_an_error(): + """It is seeded into every new pipeline, so its resting state has to be a working one: + the model goes straight through and the log says so.""" + model, clip, stack, status = mm.FunPackLoraLoader().load_loras( + model="MODEL-SENTINEL", lora_list="[]", clip="CLIP-SENTINEL") + assert model == "MODEL-SENTINEL" and clip == "CLIP-SENTINEL" + assert stack["loras"] == [] + assert status == "FunPack LoRA Loader | No active LoRAs" + + +def test_the_empty_lora_list_is_declared_a_working_state(): + """The Models panel flags an empty list as something to fix — except this one.""" + spec = mm.FunPackLoraLoader.INPUT_TYPES() + assert spec["required"]["lora_list"][1]["funpack_list"]["allow_empty"] is True diff --git a/widgets.py b/widgets.py index fe39357e..735f4637 100644 --- a/widgets.py +++ b/widgets.py @@ -30,11 +30,18 @@ def field(name, kind, *, label=None, choices=None, default=None, min=None, max=N return spec -def list_widget(item, fields, *, default=None, add_label=None, max_rows=0, tooltip=None): - """An INPUT_TYPES entry for a list input. Returns the ("STRING", {...}) tuple.""" +def list_widget(item, fields, *, default=None, add_label=None, max_rows=0, tooltip=None, + allow_empty=False): + """An INPUT_TYPES entry for a list input. Returns the ("STRING", {...}) tuple. + + `allow_empty` marks a list whose empty state is a working state — the node passes its + input through — so a frontend does not flag it as something to fix. + """ spec = {"item": item, "fields": list(fields), "add_label": add_label or f"+ Add {item}"} if max_rows: spec["max_rows"] = int(max_rows) + if allow_empty: + spec["allow_empty"] = True opts = { "default": json.dumps(default or []), "multiline": False,