fix: SLA's H3 gate now matches the qualified class, not a bare name

Round 1 of extensive_testing on 79ca8cf found is_h3_model() matched on
diffusion_model's bare class name ("MiniMaxH3Model"), inherited
unchanged from v4. This codebase's own core/traits.py.has_block()
documents exactly why that's unsafe for a name this generic: a
same-named class from an unrelated custom node or a future FunPack
model would be silently treated as H3 and sparsified at 0.90 with
H3-specific audio-prefix pinning -- the identical LTX-collision risk
this file's own docstring already warns about, just not guarded
against strongly enough. modules/models/minimax_h3 already qualifies
its own identical check (MODEL_CLASS = the dotted path); this port
carried over the private, weaker duplicate instead of reusing that
pattern.

Switched to core/traits.py's has_block() with the same qualified dotted
name, kept as a local string (not an import of modules/models/minimax_h3
itself, which pulls in comfy_extras at import time -- this file
deliberately stays importable without ComfyUI on the path). Added a
test exercising the exact collision the bare match would have missed:
a class named MiniMaxH3Model from an unrelated module.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
digital-garbage
2026-09-15 15:20:51 +03:00
co-authored by Claude Sonnet 5
parent 79ca8cfb6b
commit 548d99a39b
2 changed files with 36 additions and 6 deletions
+18 -4
View File
@@ -24,10 +24,22 @@ below exists and why a run that never sparsified says so in the log.
"""
import logging
from .._core import traits as _traits
SLA_NAME = "sla_h3"
_H3_HEAD_DIM = 128
_H3_MODEL_NAMES = ("MiniMaxH3Model",)
# Qualified on purpose, per core/traits.py's has_block: "a bare name matches on
# the class name alone... actively wrong" for a false positive that silently
# sparsifies a model this was never validated on -- exactly the LTX-collision
# risk this file's own docstring already warns about, just not guarded against
# strongly enough before this fix (v4 matched the bare name; found in extensive_
# testing round 1 on this port). Kept as a local string matching modules/models/
# minimax_h3's own MODEL_CLASS rather than importing that module -- it pulls in
# comfy_extras at import time, which this file deliberately does not require
# just to be importable on a machine with no ComfyUI on the path. Keep the two
# in sync if the H3 model class is ever renamed.
_H3_MODEL_CLASS = "comfy.ldm.minimax.model.MiniMaxH3Model"
# Validated on an RTX 5090 at 768p/15s (ComfyUI-H3-SLA-Attention's own measurements):
# 3.7x the attention throughput of stock ComfyUI at sparsity 0.90 / block 64.
@@ -67,10 +79,12 @@ def is_h3_model(model):
SLA is only sound on H3: the sparsity ratio it runs at is the one the turbo LoRA
was distilled to tolerate, and H3's packed [text | cond | audio | video] sequence
is what the prefix pinning is for. Head shape alone would also match LTX, and
silently sparsifying LTX attention is a quality loss with no LoRA compensating it.
silently sparsifying LTX attention is a quality loss with no LoRA compensating it
-- which is also why the class name is matched qualified (module + name), not
bare: a bare name is exactly as blind to an unrelated same-named class as it is
to LTX's different head shape.
"""
diffusion = getattr(getattr(model, "model", None), "diffusion_model", None)
return type(diffusion).__name__ in _H3_MODEL_NAMES
return _traits.has_block(model, _H3_MODEL_CLASS)
def new_state():
+18 -2
View File
@@ -190,8 +190,12 @@ def test_a_non_h3_model_is_not_handed_a_minimax_kwarg():
# ── the H3 gate ───────────────────────────────────────────────────────────────
class _Patcher:
def __init__(self, class_name):
inner = type(class_name, (), {})()
def __init__(self, class_name, module="comfy.ldm.minimax.model"):
# has_block (core/traits.py) matches a QUALIFIED class name -- module and
# name both -- so the stub needs a real __module__ and a modules() walk
# the way an nn.Module provides, not just a bare type() with a name.
cls = type(class_name, (), {"__module__": module, "modules": lambda self: [self]})
inner = cls()
self.model = types.SimpleNamespace(diffusion_model=inner)
self.model_options = {}
self.wrappers = []
@@ -214,6 +218,18 @@ def test_sla_refuses_a_model_that_is_not_h3_and_says_so():
assert "not a MiniMax H3 model" in note
def test_a_same_named_class_from_a_different_module_is_not_treated_as_h3():
"""A bare class-name match would treat any unrelated MiniMaxH3Model -- built by a
different custom node, or a future FunPack model reusing the name by accident --
as the real thing, and sparsify it at H3's ratio with H3's prefix-pinning logic
on a model that was never validated for either. The qualified check requires the
DEFINING module too, not just the name."""
model = _Patcher("MiniMaxH3Model", module="some_other_package.model")
out, note, installed = sla.install_sla(model)
assert out is model and installed is False
assert "not a MiniMax H3 model" in note
def test_sla_reports_when_the_machine_cannot_run_it():
model = _Patcher("MiniMaxH3Model")
out, note, installed = sla.install_sla(model)