fix(loaders): name the stage a load failed at; gate the arch override

A bare "'dict' object has no attribute 'startswith'" names neither the
step nor the file. Each stage now labels itself in the re-raise. The
ComfyUI-GGUF architecture override is opt-in, not default.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
digital-garbage
2026-08-23 05:24:50 +03:00
co-authored by Claude Opus 5
parent 78c9625a1a
commit 4bacc9c7d3
4 changed files with 57 additions and 10 deletions
+5 -6
View File
@@ -8,12 +8,11 @@
minute of dequantizing a GGUF to alter a sparsity ratio. The weights depend only on the
file (identity, size, mtime) and the dtypes; everything else is applied to a clone. One
entry, replaced whenever the key changes.
- **A GGUF architecture ComfyUI-GGUF has not been told about now loads quantized anyway.**
The pack refuses anything off its tested list — `Unexpected architecture type in GGUF file:
'minimax_h3'` — which cost the quantized path entirely and fell back to expanding the whole
checkpoint. The name is now added to its IMAGE list (never the text one) for a file whose
header actually declares it, and the status line says so, because it is an override of
another project's own guard.
- **`FUNPACK_GGUF_ALLOW_ANY_ARCH=1` lets ComfyUI-GGUF read an architecture off its tested
list** — `Unexpected architecture type in GGUF file: 'minimax_h3'` — so the file loads
quantized instead of being expanded. OFF by default: overriding another project's guard
turns a clean refusal into an untested path, which can fail deeper or produce a subtly
wrong state dict. The status line names the variable when it is in effect.
- **When it does have to expand a GGUF, it expands ONCE.** The dequantized weights are
written beside the file as an ordinary safetensors and reused on every later load, which
then runs at safetensors speed. It costs the model's dequantized size in disk — the same
+13 -3
View File
@@ -314,6 +314,10 @@ def _load_native_cached(path: str) -> tuple[dict, dict, str]:
return sd, options, note
ARCH_OVERRIDE_VAR = "FUNPACK_GGUF_ALLOW_ANY_ARCH"
_ARCH_ON = {"1", "true", "yes", "on"}
def read_architecture(path: str) -> Optional[str]:
"""`general.architecture` out of a GGUF header, or None. Header only — no tensor data."""
try:
@@ -342,6 +346,11 @@ def _allow_architectures(loader, path: str) -> str:
declares it. Announced, because it is an override of another project's own guard and a
wrong result here should be traceable to this line.
"""
# OPT-IN. Overriding another project's safety guard changes a clean refusal (which falls
# back to a slower path that works) into an untested code path that may produce a subtly
# wrong state dict — a worse failure, and a harder one to attribute. Off unless asked.
if str(os.environ.get(ARCH_OVERRIDE_VAR, "")).strip().lower() not in _ARCH_ON:
return ""
arch = read_architecture(path)
if not arch:
return ""
@@ -364,9 +373,10 @@ def _allow_architectures(loader, path: str) -> str:
continue
if not added:
return ""
return (f"architecture {arch!r} is not on ComfyUI-GGUF's tested list and FunPack added it "
f"so the file loads quantized instead of being expanded — if the weights come out "
f"wrong, this is why")
return (f"architecture {arch!r} is not on ComfyUI-GGUF's tested list and "
f"{ARCH_OVERRIDE_VAR} added it so the file loads quantized instead of being "
f"expanded — if the weights come out wrong, or the load fails somewhere inside "
f"the pack, this is why: unset it to go back to the refusal and the slower path")
def _load_native(path: str) -> tuple[dict, dict, str]:
+24
View File
@@ -255,6 +255,24 @@ class FunPackDiffusionModelLoader:
fp16_accumulation=False, sla_sparsity=None, sla_block_size=None,
sla_protect_audio=None, sla_min_seq_len=None, sla_dense_last_steps=None,
sla=False):
# ComfyUI reports a node failure as "<node>: <message>" with the traceback going to
# the console. A bare "'dict' object has no attribute 'startswith'" names neither the
# step it came from nor the file it was reading, which is most of what you need. Each
# stage sets `stage`, and the re-raise says which one.
stage = {"at": "starting"}
try:
return self._load_model(stage, model_name, weight_dtype, compute_dtype, attention,
fp16_accumulation, sla_sparsity, sla_block_size,
sla_protect_audio, sla_min_seq_len, sla_dense_last_steps,
sla)
except Exception as e:
raise type(e)(f"{e} [while {stage['at']}: {model_name}]").with_traceback(
e.__traceback__) from e
def _load_model(self, stage, model_name, weight_dtype, compute_dtype, attention,
fp16_accumulation=False, sla_sparsity=None, sla_block_size=None,
sla_protect_audio=None, sla_min_seq_len=None, sla_dense_last_steps=None,
sla=False):
notes = [f"FunPack Diffusion Model Loader | {model_name}"]
accum = set_fp16_accumulation(fp16_accumulation)
@@ -272,6 +290,7 @@ class FunPackDiffusionModelLoader:
# depend on the file and the dtypes, so those are the cache key and everything else
# is applied to a clone. One entry, replaced whenever the key changes: this holds no
# more than ComfyUI's own node-output cache already did.
stage["at"] = "checking the weight cache"
cache_key = _weight_cache_key(model_name, weight_dtype, compute_dtype)
cached = _WEIGHT_CACHE.get("model") if _WEIGHT_CACHE.get("key") == cache_key else None
if cached is not None:
@@ -280,6 +299,7 @@ class FunPackDiffusionModelLoader:
return self._finish(model, notes, attention, sla, sla_sparsity, sla_block_size,
sla_min_seq_len, sla_dense_last_steps, sla_protect_audio)
stage["at"] = "locating the file"
path = None
misnamed = False
if gguf_support.is_gguf(model_name):
@@ -296,6 +316,7 @@ class FunPackDiffusionModelLoader:
if misnamed:
notes.append(f"{model_name} is named .safetensors but is a GGUF container — "
f"loaded as GGUF")
stage["at"] = "reading the GGUF container"
state_dict, gguf_options, gguf_note = gguf_support.load_state_dict(path)
# The quantized path needs its own torch operations, and they must not be lost to
# the dtype options merged above — a GGUF loaded with stock ops would try to matmul
@@ -304,7 +325,9 @@ class FunPackDiffusionModelLoader:
metadata = None
notes.append(gguf_note)
else:
stage["at"] = "reading the safetensors file"
state_dict, metadata = comfy.utils.load_torch_file(path, return_metadata=True)
stage["at"] = "handing the state dict to ComfyUI"
model = comfy.sd.load_diffusion_model_state_dict(
state_dict, model_options=model_options, metadata=metadata)
if model is None:
@@ -324,6 +347,7 @@ class FunPackDiffusionModelLoader:
# below is applied to a clone, which is what lets a sparsity change skip the disk.
_WEIGHT_CACHE["key"] = cache_key
_WEIGHT_CACHE["model"] = model
stage["at"] = "applying attention / SLA"
return self._finish(model.clone(), notes, attention, sla, sla_sparsity,
sla_block_size, sla_min_seq_len, sla_dense_last_steps,
sla_protect_audio)
+15 -1
View File
@@ -322,21 +322,34 @@ class _Loader:
self.IMG_ARCH_LIST = set(arches)
def test_the_override_is_off_unless_asked(monkeypatch):
"""Turning another project's clean refusal into an untested code path is a WORSE failure
than the slow fallback it avoids — it can produce a subtly wrong state dict instead."""
monkeypatch.delenv(gguf_support.ARCH_OVERRIDE_VAR, raising=False)
monkeypatch.setattr(gguf_support, "read_architecture", lambda p: "minimax_h3")
loader = _Loader({"flux"})
assert gguf_support._allow_architectures(loader, "x.gguf") == ""
assert loader.IMG_ARCH_LIST == {"flux"}
def test_an_unknown_architecture_is_added_so_the_file_loads_quantized(monkeypatch):
monkeypatch.setenv(gguf_support.ARCH_OVERRIDE_VAR, "1")
monkeypatch.setattr(gguf_support, "read_architecture", lambda p: "minimax_h3")
loader = _Loader({"flux", "sd3"})
note = gguf_support._allow_architectures(loader, "x.gguf")
assert "minimax_h3" in loader.IMG_ARCH_LIST
assert "minimax_h3" in note and "FunPack added it" in note
assert "minimax_h3" in note and gguf_support.ARCH_OVERRIDE_VAR in note
def test_a_known_architecture_is_left_alone_and_says_nothing(monkeypatch):
monkeypatch.setenv(gguf_support.ARCH_OVERRIDE_VAR, "1")
monkeypatch.setattr(gguf_support, "read_architecture", lambda p: "flux")
loader = _Loader({"flux"})
assert gguf_support._allow_architectures(loader, "x.gguf") == ""
def test_an_unreadable_architecture_changes_nothing(monkeypatch):
monkeypatch.setenv(gguf_support.ARCH_OVERRIDE_VAR, "1")
monkeypatch.setattr(gguf_support, "read_architecture", lambda p: None)
loader = _Loader({"flux"})
assert gguf_support._allow_architectures(loader, "x.gguf") == ""
@@ -344,6 +357,7 @@ def test_an_unreadable_architecture_changes_nothing(monkeypatch):
def test_the_text_encoder_list_is_never_touched(monkeypatch):
monkeypatch.setenv(gguf_support.ARCH_OVERRIDE_VAR, "1")
"""Image and text architectures steer different key renaming in the pack; guessing a
text arch into the image list is one thing, the reverse is another."""
monkeypatch.setattr(gguf_support, "read_architecture", lambda p: "minimax_h3")