From 4bacc9c7d396b1e847a90b9533dd1a2604b423ad Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sun, 23 Aug 2026 05:24:50 +0300 Subject: [PATCH] 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 --- CHANGELOG.md | 11 +++++------ gguf_support.py | 16 +++++++++++++--- loaders.py | 24 ++++++++++++++++++++++++ tests/test_gguf_support.py | 16 +++++++++++++++- 4 files changed, 57 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ed00b5d..88ee67da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/gguf_support.py b/gguf_support.py index 0d6d244d..860bf434 100644 --- a/gguf_support.py +++ b/gguf_support.py @@ -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]: diff --git a/loaders.py b/loaders.py index df335bef..b42d7f78 100644 --- a/loaders.py +++ b/loaders.py @@ -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 ": " 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) diff --git a/tests/test_gguf_support.py b/tests/test_gguf_support.py index a2bf2e6e..82eec412 100644 --- a/tests/test_gguf_support.py +++ b/tests/test_gguf_support.py @@ -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")