From fc977f4068496e30672db12841b970cf0b2d7896 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sat, 29 Aug 2026 13:05:28 +0300 Subject: [PATCH] sampling: check the values where they are used, not only where they are made The NaN guard was on one of two doors. A FUNPACK_SETTINGS socket is typed by a string tag, so a dict reaches the load node without necessarily having passed through the settings node -- a raw API prompt does exactly that. Through that door, until_sigma = NaN made ALG a permanent no-op: sigma > nan is False at every step, so the anchor was never loosened, and the node reported "1 modifier applied: alg". Nothing about the run looked wrong, which is the only reason this is worth a commit of its own. Values are now checked against the declaration at the point they are consumed. The producer still checks too; that is not redundant, it is where a person editing JSON gets told immediately. Also recorded ALG's standing assumption rather than leaving it implicit: the anchor is latent frame 0. True of everything it can currently be wired into, untrue in general, and the denoise_mask already carries what would be needed to do it properly. Left undone because nothing yet pins elsewhere, and a derivation with no consumer cannot be checked against reality. Co-Authored-By: Claude Opus 5 --- modules/sampling/alg/__init__.py | 9 +++++++ modules/sampling/alg/tests/test_alg.py | 9 +++++++ modules/sampling/modifiers/nodes.py | 18 ++++++++++++- modules/sampling/tests/test_modifiers.py | 34 ++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/modules/sampling/alg/__init__.py b/modules/sampling/alg/__init__.py index 3084bc9c..77bc84b9 100644 --- a/modules/sampling/alg/__init__.py +++ b/modules/sampling/alg/__init__.py @@ -13,6 +13,15 @@ of its own. It attaches to the MODEL, so it applies to whatever sampler is wired H3" -- it needs a latent with a time axis and a pinned anchor, and any model with both gets it, including ones nobody here has heard of. +KNOWN ASSUMPTION: the anchor is latent frame 0. That is true of every i2v setup +this can currently be wired into, and it is what v4 assumed. It is NOT true in +general -- H3 can pin a keyframe at any frame -- so if an interior-pin mechanism +is ever ported, ALG must take the frame indices from the pin rather than assume +the first. The information to do it properly is already present in the +denoise_mask, which is what marks the locked frames; deriving from it is left +undone deliberately, because there is nothing yet that pins anywhere else and a +derivation with no consumer cannot be checked against reality. + What is NOT ported from v4: the packed-layout path, which reached into a model-specific arrangement of the latent to find the video stream. Where the anchor is not a plain [B, C, T, H, W] tensor this stands down and says so once, diff --git a/modules/sampling/alg/tests/test_alg.py b/modules/sampling/alg/tests/test_alg.py index c045100c..b01f2fcd 100644 --- a/modules/sampling/alg/tests/test_alg.py +++ b/modules/sampling/alg/tests/test_alg.py @@ -159,3 +159,12 @@ def test_the_proxy_is_transparent_for_everything_else(): assert proxy.inner_model == "the real model" proxy.something = 7 assert inner.something == 7, "an assignment did not reach the real denoiser" + + +def test_a_threshold_that_is_not_a_number_would_disable_it_everywhere(): + """Why the check at the load node matters, in ALG's own terms. NaN compares + False against every sigma, so the anchor is never loosened at any step -- + and nothing about the run looks wrong.""" + from modules.sampling.alg.blur import use_blurred + assert all(use_blurred(sigma, float("nan")) is False + for sigma in (0.99, 0.5, 0.01)) diff --git a/modules/sampling/modifiers/nodes.py b/modules/sampling/modifiers/nodes.py index 8f798fb8..e25f924c 100644 --- a/modules/sampling/modifiers/nodes.py +++ b/modules/sampling/modifiers/nodes.py @@ -163,8 +163,24 @@ class FunPackLoadModifiers(io.ComfyNode): if stripped: notes.append(f"replaced {stripped} modifier(s) already on this model") + # Checked HERE, not only where the payload was made. A FUNPACK_SETTINGS + # socket is typed by a string tag, so a dict can reach this node without + # ever passing through FunPackModifierSettings -- a raw API prompt, or a + # second producer of that type. Validating only at the producer is a + # guard on one of two doors, and the one left open let NaN through: + # `sigma > nan` is False at every step, so a modifier became a permanent + # no-op while reporting that it had been applied. + problems = [] + chosen = {} for spec in ordered: - values = (settings or {}).get(spec.id) or spec.defaults() + values, said = schema_mod.check_values(spec, (settings or {}).get(spec.id)) + chosen[spec.id] = values + problems.extend(said) + if problems: + raise RuntimeError("FunPack Load Modifiers:\n " + "\n ".join(problems)) + + for spec in ordered: + values = chosen[spec.id] install = spec.provides[CAPABILITY] try: note = install(patched, values, key=KEY_PREFIX + spec.id) diff --git a/modules/sampling/tests/test_modifiers.py b/modules/sampling/tests/test_modifiers.py index 2561b795..d5fd6c3d 100644 --- a/modules/sampling/tests/test_modifiers.py +++ b/modules/sampling/tests/test_modifiers.py @@ -252,3 +252,37 @@ def test_a_settings_payload_of_the_wrong_shape_is_refused_clearly(registry, patc with pytest.raises(RuntimeError, match="keyed by module id"): FunPackLoadModifiers.execute(patcher, settings=["not", "a", "dict"]) + + +def test_values_are_checked_where_they_are_used_not_only_where_they_were_made(registry, patcher): + """The settings socket is typed by a string tag, so a dict can reach this + node without passing through FunPackModifierSettings. Validating only at the + producer guards one of two doors, and NaN walked through the other: sigma > + nan is False at every step, so the modifier became a permanent no-op while + reporting that it had been applied.""" + from modules.sampling.modifiers.nodes import FunPackLoadModifiers + + registry.specs["alg"] = _spec("alg", lambda t, v, key: "on", settings={ + "until_sigma": {"type": "float", "default": 0.6, "min": 0.0, "max": 1.0, "label": "T"}, + }) + + with pytest.raises(RuntimeError, match="finite"): + FunPackLoadModifiers.execute(patcher, settings={"alg": {"until_sigma": float("nan")}}) + + +def test_an_out_of_range_value_is_refused_at_this_node_too(registry, patcher): + from modules.sampling.modifiers.nodes import FunPackLoadModifiers + registry.specs["alg"] = _spec("alg", lambda t, v, key: "on", settings={ + "strength": {"type": "float", "default": 4.0, "min": 1.0, "max": 16.0, "label": "S"}, + }) + with pytest.raises(RuntimeError, match="above its maximum"): + FunPackLoadModifiers.execute(patcher, settings={"alg": {"strength": 99.0}}) + + +def test_a_setting_no_module_declares_is_refused_at_this_node_too(registry, patcher): + from modules.sampling.modifiers.nodes import FunPackLoadModifiers + registry.specs["alg"] = _spec("alg", lambda t, v, key: "on", settings={ + "strength": {"type": "float", "default": 4.0, "min": 1.0, "max": 16.0, "label": "S"}, + }) + with pytest.raises(RuntimeError, match="no setting named"): + FunPackLoadModifiers.execute(patcher, settings={"alg": {"strenght": 4.0}})