diff --git a/app/boot.js b/app/boot.js index ed0fa9fd..e1739459 100644 --- a/app/boot.js +++ b/app/boot.js @@ -32,6 +32,40 @@ import { wireReferences } from "./shell/reference_wiring.js"; const root = document.querySelector("#app"); +// Loader nodes core knows how to point `/api/probe` at, and which of their +// inputs names the file. Core never names a model, but the app already has +// to name these two loaders elsewhere (widgets.js's "ckpt" combo) to draw +// their panels, so naming them again here to find "the chosen file" is not +// a new coupling. +const MODEL_FILE_INPUT = { + FunPackCheckpointLoader: "ckpt_name", + FunPackDiffusionModelLoader: "model_name", +}; + +/** + * Which traits the pipeline's chosen model has, or null when that is not + * knowable yet (no loader slot, no file chosen, or the probe could not + * place it). `null` must reach fetchManifest() as "do not filter" rather + * than as an empty list -- an empty list would hide every module that + * requires ANY trait at all, for every model this cheap, load-free probe + * simply has no opinion about yet ([[project_v5_slot_inputs_gap]]-shaped + * mistake: partial knowledge is not the same as "no traits"). + */ +async function probeModelTraits(slots) { + const slot = (slots || []).find((s) => MODEL_FILE_INPUT[s.node] && s.inputs); + if (!slot) return null; + const filename = slot.inputs[MODEL_FILE_INPUT[slot.node]]; + if (!filename) return null; + try { + const res = await fetch(`/funpack/api/probe?file=${encodeURIComponent(filename)}`); + if (!res.ok) return null; + const data = await res.json(); + return data.detected ? (data.traits || []) : null; + } catch { + return null; + } +} + async function start() { const id = clientId(); const run = createRun({ clientId: id, connect }); @@ -268,6 +302,10 @@ async function start() { describe, check, search, presets, ...ctx, onApply: (next) => { slots = next; + // A saved edit may have changed which file the model loader + // points at (or added/removed the loader entirely) -- re-probe + // rather than assume the traits computed at boot still hold. + probeModelTraits(next).then(remountFor); // The boxes on the main window are for inputs of THESE slots. A slot // that was removed takes its box with it, and a value saved in the // window is what its box now shows -- otherwise the two windows hold @@ -504,9 +542,20 @@ async function start() { project.flush(); }); + // Read once, ahead of the manifest, purely to name the chosen model before + // anything mounts -- separate from the `load()` call below that reads the + // pipeline's inputs for the prompt boxes, since that one runs after mount + // (a region has to exist first) and this one has to run before it. + let initialTraits = null; + try { + initialTraits = await probeModelTraits((await load()).slots); + } catch (err) { + console.warn(`[FunPack] could not read the pipeline's model before mounting: ${err.message}`); + } + let manifest; try { - manifest = await fetchManifest(); + manifest = await fetchManifest(initialTraits); } catch (err) { // The one failure the user must see: with no manifest there is no app, so // silence here would be an empty window with no explanation. @@ -518,7 +567,33 @@ async function start() { return; } - const { mounted, hidden } = await mountAll(manifest); + let { mounted, hidden } = await mountAll(manifest); + + /** + * Re-fetch the manifest for a newly-chosen model and swap the mounted + * panels for it. `mounted`'s own `destroy()` (panels.js) is what makes + * this safe to call more than once -- a stale listener from the old set + * of panels is exactly what dropping the teardown would leave behind. + */ + async function remountFor(traits) { + let fresh; + try { + fresh = await fetchManifest(traits); + } catch (err) { + console.warn(`[FunPack] could not refresh modules for the new model: ${err.message}`); + return; + } + for (const m of mounted) m.destroy(); + manifest = fresh; + ({ mounted, hidden } = await mountAll(manifest)); + settle(); + showGroups(); + if (window.FunPack) { + window.FunPack.manifest = manifest; + window.FunPack.hidden = hidden; + window.FunPack.mounted = mounted.map((m) => m.id); + } + } // After the modules, because a region has to exist before anything can be put // in it, and a module may be sharing the region a role names. diff --git a/app/legacy/pipeline_state.js b/app/legacy/pipeline_state.js index ea511699..7910226b 100644 --- a/app/legacy/pipeline_state.js +++ b/app/legacy/pipeline_state.js @@ -10,8 +10,55 @@ (function () { const API = window.MovieEditorAPI; + // Loader nodes core knows how to point /api/probe at, and which of their + // inputs names the file. Mirrors the same map api.js's probeFamily() is + // meant to be fed from -- nothing else in this app currently resolves + // "the file the pipeline's model loader points at" from raw slots. + const MODEL_FILE_INPUT = { + FunPackCheckpointLoader: "ckpt_name", + FunPackDiffusionModelLoader: "model_name", + }; + + function currentModelFile(pipelineSlots) { + const slot = (pipelineSlots || []).find((s) => MODEL_FILE_INPUT[s.node] && s.inputs); + return slot ? slot.inputs[MODEL_FILE_INPUT[slot.node]] || null : null; + } + + // Which traits the pipeline's chosen model has, or null when that is not + // knowable (no loader slot, no file chosen, or the probe found nothing). + // null must reach API.modules() as "do not filter" rather than as an + // empty list -- /api/modules?traits= with an empty traits set hides every + // module that requires ANY trait at all, for every model this cheap, + // load-free probe has no opinion about yet (this cheap path only knows + // MiniMax H3's own traits today -- see modules/models/minimax_h3). + async function probeModelTraits(pipelineSlots) { + const filename = currentModelFile(pipelineSlots); + if (!filename) return null; + try { + const data = await API.probeFamily(filename); + return data.detected ? (data.traits || []) : null; + } catch (_) { + return null; + } + } + let modulesById = {}; let slots = null; // null = never loaded this session; loaded exactly once + // The model file `modulesById` was last filtered for. A save() whose slots + // still name the same file has nothing new to learn -- re-probing on every + // unrelated edit (a prompt, a slider) would mean one extra round trip per + // keystroke-driven save for no reason. + let lastProbedFile = null; + + async function refreshManifest() { + const file = currentModelFile(slots); + if (file === lastProbedFile) return; + lastProbedFile = file; + const traits = await probeModelTraits(slots); + const manifest = await API.modules(traits ? traits.join(",") : undefined); + modulesById = {}; + (manifest.modules || []).forEach((m) => { modulesById[m.id] = m; }); + } let incomplete = []; let refused = []; let queueable = false; @@ -62,13 +109,16 @@ loading = true; loadError = null; loadPromise = (async () => { try { - const [manifest, pipe] = await Promise.all([API.modules(), API.pipeline()]); - modulesById = {}; - (manifest.modules || []).forEach((m) => { modulesById[m.id] = m; }); + // Sequential, not Promise.all: which modules are even compatible + // depends on which model the pipeline's loader slot names, so the + // manifest fetch has to know that before it can ask -- refreshManifest() + // reads `slots`, so it has to be set first. + const pipe = await API.pipeline(); slots = pipe.slots || []; incomplete = pipe.incomplete || []; refused = pipe.refused || []; queueable = !!pipe.queueable; + await refreshManifest(); } catch (e) { loadError = e && e.message ? e.message : String(e); } @@ -129,6 +179,17 @@ refused = (res && res.refused) || []; queueable = !!(res && res.queueable); saveNotes = (res && res.notes) || []; + // A no-op for the common case (same file as last time) -- see + // refreshManifest()'s own guard. Only a Models & Pipeline edit that + // actually changes the loader's file does a second round trip here. + // Its own try/catch: a failure here is not "could not save" -- the + // edit above already landed -- so it must not overwrite saveNotes + // with a message about the wrong failure. + try { + await refreshManifest(); + } catch (e) { + console.warn(`[FunPack] could not refresh modules for the new model: ${e && e.message ? e.message : e}`); + } } catch (e) { saveNotes = [`Could not save: ${e && e.message ? e.message : e}`]; } diff --git a/core/probe.py b/core/probe.py index b04653ec..fc11f914 100644 --- a/core/probe.py +++ b/core/probe.py @@ -79,24 +79,28 @@ def resolve_diffusion_model(filename: str) -> Optional[Path]: def detect(path: Path, registry=None) -> dict: """Inspect one checkpoint file. - Returns ``{"module", "title", "detected", "reason"}``. `module` is the id - of whichever model module claimed it, or None -- and None is a real - answer, not a failure to try hard enough: it means this install has no - module that recognises the file, and the caller should say so rather + Returns ``{"module", "title", "detected", "reason", "traits"}``. `module` + is the id of whichever model module claimed it, or None -- and None is a + real answer, not a failure to try hard enough: it means this install has + no module that recognises the file, and the caller should say so rather than guess a family. `reason` is plain text for a person, always - populated. + populated. `traits` is only ever populated when `detected` is true -- + an undetected file says nothing about what it is, so it must not narrow + what the UI shows via `/api/modules?traits=`; the caller should treat + `detected: false` as "do not filter", not as "filter to nothing". """ p = Path(path) if not p.is_file(): return {"module": None, "title": None, "detected": False, - "reason": f"{p.name}: file not found"} + "reason": f"{p.name}: file not found", "traits": []} if p.suffix.lower() != ".safetensors": return {"module": None, "title": None, "detected": False, - "reason": f"{p.name}: only .safetensors can be inspected without loading it"} + "reason": f"{p.name}: only .safetensors can be inspected without loading it", + "traits": []} keys = read_safetensors_keys(p) if keys is None: return {"module": None, "title": None, "detected": False, - "reason": f"{p.name}: not a readable safetensors header"} + "reason": f"{p.name}: not a readable safetensors header", "traits": []} keyset = set(keys) reg = registry or registry_mod.current() @@ -108,7 +112,16 @@ def detect(path: Path, registry=None) -> dict: log.failed(f"{spec.id}.detect", exc) continue if matched: + traits: list = [] + probe_traits = spec.provides.get("probe_traits") + if probe_traits is not None: + try: + traits = list(probe_traits(keyset)) + except Exception as exc: # noqa: BLE001 + from . import log + log.failed(f"{spec.id}.probe_traits", exc) return {"module": spec.id, "title": spec.title, "detected": True, - "reason": f"{p.name}: {spec.title}"} + "reason": f"{p.name}: {spec.title}", "traits": traits} return {"module": None, "title": None, "detected": False, - "reason": f"{p.name}: no installed model module recognises this file"} + "reason": f"{p.name}: no installed model module recognises this file", + "traits": []} diff --git a/core/tests/test_probe.py b/core/tests/test_probe.py index c874c32f..e470b184 100644 --- a/core/tests/test_probe.py +++ b/core/tests/test_probe.py @@ -83,7 +83,8 @@ def test_a_file_no_module_recognises_says_so_rather_than_guessing(tmp_path): _write_safetensors(p, ["some.unrelated.weight"]) out = probe.detect(p, registry=_registry(("m", "M", {"video_patch_proj.weight"}))) assert out == {"module": None, "title": None, "detected": False, - "reason": f"{p.name}: no installed model module recognises this file"} + "reason": f"{p.name}: no installed model module recognises this file", + "traits": []} def test_a_file_a_module_recognises_names_that_module(tmp_path): @@ -97,6 +98,52 @@ def test_a_file_a_module_recognises_names_that_module(tmp_path): assert "MiniMax H3" in out["reason"] +# --- traits, only ever alongside a real detection --------------------------- + +def test_an_undetected_file_carries_no_traits(tmp_path): + """`detected: false` must mean "the caller should not filter", never + "filter to nothing" -- an empty list here is that contract, checked + explicitly so it cannot regress into `None` (which reads the same to a + human but not to `/api/modules?traits=`).""" + p = tmp_path / "m.safetensors" + _write_safetensors(p, ["some.unrelated.weight"]) + out = probe.detect(p, registry=_registry(("m", "M", {"video_patch_proj.weight"}))) + assert out["traits"] == [] + + +def test_a_matched_module_with_no_probe_traits_provider_still_gets_a_list(tmp_path): + p = tmp_path / "m.safetensors" + _write_safetensors(p, ["shared.weight"]) + out = probe.detect(p, registry=_registry(("m", "M", {"shared.weight"}))) + assert out["traits"] == [] + + +def test_a_matched_modules_probe_traits_are_carried_through(tmp_path): + p = tmp_path / "m.safetensors" + _write_safetensors(p, ["shared.weight"]) + reg = Registry() + reg.add(ModuleSpec(id="m", title="M", mount="", + provides={"detect": lambda keys: "shared.weight" in keys, + "probe_traits": lambda keys: ["audio_stream", "temporal_latent"]})) + out = probe.detect(p, registry=reg) + assert out["traits"] == ["audio_stream", "temporal_latent"] + + +def test_a_probe_traits_that_raises_is_skipped_not_fatal(tmp_path): + p = tmp_path / "m.safetensors" + _write_safetensors(p, ["shared.weight"]) + def explodes(_keys): + raise RuntimeError("boom") + reg = Registry() + reg.add(ModuleSpec(id="m", title="M", mount="", + provides={"detect": lambda keys: "shared.weight" in keys, + "probe_traits": explodes})) + out = probe.detect(p, registry=reg) + assert out["module"] == "m" + assert out["detected"] is True + assert out["traits"] == [] + + def test_a_missing_file_is_not_found_not_undetected(tmp_path): out = probe.detect(tmp_path / "nope.safetensors", registry=_registry()) assert out["detected"] is False diff --git a/modules/models/minimax_h3/__init__.py b/modules/models/minimax_h3/__init__.py index 3b18f4a8..cce0390e 100644 --- a/modules/models/minimax_h3/__init__.py +++ b/modules/models/minimax_h3/__init__.py @@ -86,6 +86,34 @@ def detect(keys) -> bool: for prefix in _PREFIXES) +def probe_traits(keys) -> list: + """What `traits()` would say for a loaded H3 model, read before anything + loads. + + Sourced from H3's own model_config class, never from a checkpoint + instance -- `latent_dimensions` and `temporal_downscale_ratio` are fixed + on the class, so nothing a load would tell us here isn't already true of + every H3 file. Lets the UI hide LTX-only settings the moment a file is + picked in the pipeline window, not only once something is generated. + + Deliberately narrower than `traits()`: `adaln_modalities` depends on + which blocks an actual loaded module tree contains (full-form vs. + curve-form checkpoints), which the header alone cannot answer, so it is + left for the real load. + """ + if not detect(keys): + return [] + from comfy.latent_formats import MiniMaxH3AV + + found = ["audio_stream"] + rank = _traits.LATENT_RANK.get(MiniMaxH3AV.latent_dimensions) + if rank: + found.append(rank) + if getattr(MiniMaxH3AV, "temporal_downscale_ratio", 1) not in (None, 1): + found.append("temporal_compression") + return found + + def traits(model): """What core cannot read off the model for itself.""" if not is_h3(model): @@ -150,4 +178,4 @@ def empty_latent(model, width, height, length, batch_size=1): TRAITS = traits PROVIDES = {"empty_latent": empty_latent, "decode": decode, "detect": detect, - "pipeline_presets": _pipeline_presets} + "pipeline_presets": _pipeline_presets, "probe_traits": probe_traits} diff --git a/modules/models/minimax_h3/tests/test_probe_traits.py b/modules/models/minimax_h3/tests/test_probe_traits.py new file mode 100644 index 00000000..fbc09af5 --- /dev/null +++ b/modules/models/minimax_h3/tests/test_probe_traits.py @@ -0,0 +1,64 @@ +"""probe_traits(): what traits() would answer for a loaded H3 model, read +from a checkpoint's tensor names alone. + +Cross-checked against the real comfy.latent_formats.MiniMaxH3AV class rather +than hand-copied numbers, so this stays correct if upstream ever changes +those constants without anyone remembering to update a second copy here. +""" + +import pytest + + +@pytest.fixture(autouse=True) +def _needs_comfy(comfyui): + """probe_traits() imports comfy.latent_formats; detect() does not.""" + + +def _module(): + from modules.models import minimax_h3 + return minimax_h3 + + +def _h3_keys(): + return {"video_patch_proj.weight", "audio_patch_proj.weight"} + + +def test_an_undetected_file_gets_no_traits(): + m = _module() + assert m.probe_traits({"unrelated.weight"}) == [] + + +def test_an_empty_key_set_gets_no_traits(): + m = _module() + assert m.probe_traits(set()) == [] + + +def test_a_detected_file_always_gets_audio_stream(): + m = _module() + assert "audio_stream" in m.probe_traits(_h3_keys()) + + +def test_traits_match_the_real_latent_format_class(): + """Pinned against comfy's own class, not a hardcoded string -- if + upstream ever made H3's latent 2-D or dropped temporal compression, this + test (not just probe_traits) would need to change too.""" + from comfy.latent_formats import MiniMaxH3AV + from core import traits as core_traits + + m = _module() + found = m.probe_traits(_h3_keys()) + + expected_rank = core_traits.LATENT_RANK.get(MiniMaxH3AV.latent_dimensions) + assert expected_rank in found + + if getattr(MiniMaxH3AV, "temporal_downscale_ratio", 1) not in (None, 1): + assert "temporal_compression" in found + + +def test_does_not_claim_adaln_modalities_without_a_real_load(): + """The one trait probe_traits deliberately does not answer -- full-form + vs. curve-form checkpoints only differ in which blocks are actually + present, which the header's key NAMES alone (as read here) do not + distinguish reliably enough to assert either way.""" + m = _module() + assert "adaln_modalities" not in m.probe_traits(_h3_keys())