From c9e5e784fcbd37773759019c759385cd7cd2196c Mon Sep 17 00:00:00 2001 From: Will Miao Date: Tue, 28 Jul 2026 23:13:00 +0800 Subject: [PATCH] fix(metadata-overwrite): use sentinel default for clip_skip to accept wired 0 --- py/metadata_collector/constants.py | 6 ++++ py/metadata_collector/metadata_processor.py | 7 +++- py/metadata_collector/node_extractors.py | 7 ++-- py/nodes/metadata_overwrite.py | 33 +++++++++++++------ py/nodes/save_image.py | 6 ++-- .../test_metadata_collector.py | 13 ++++++-- 6 files changed, 52 insertions(+), 20 deletions(-) diff --git a/py/metadata_collector/constants.py b/py/metadata_collector/constants.py index d417e0d1..d62c5976 100644 --- a/py/metadata_collector/constants.py +++ b/py/metadata_collector/constants.py @@ -1,5 +1,11 @@ """Constants used by the metadata collector""" +# Sentinel value for clip_skip to distinguish "unconnected / widget default" +# from "user wired value 0". Both ComfyUI CLIPSetLastLayer (-24..-1) and +# A1111 conventions treat 0 as meaningless for clip skipping, but users may +# explicitly wire 0 to the overwrite node to express "no clip skip / default". +CLIP_SKIP_SENTINEL = -25 + # Metadata categories MODELS = "models" PROMPTS = "prompts" diff --git a/py/metadata_collector/metadata_processor.py b/py/metadata_collector/metadata_processor.py index a509d036..a7675f7c 100644 --- a/py/metadata_collector/metadata_processor.py +++ b/py/metadata_collector/metadata_processor.py @@ -678,7 +678,12 @@ class MetadataProcessor: for overwrite_info in metadata.get(OVERWRITE, {}).values(): overwrite_params = overwrite_info.get("parameters", {}) for key, value in overwrite_params.items(): - if value: # truthy check — only overwrite when user provided a real value + if key == "clip_skip": + # Accept any value from overwrite node (sentinel -25 already + # filtered upstream). Needed because falsy check treats 0 + # as "not set" even though 0 is a valid wired input here. + params[key] = value + elif value: # truthy check — only overwrite when user provided a real value params[key] = value # Bridge: the overwrite node exposes the field as "model" (more accurate), diff --git a/py/metadata_collector/node_extractors.py b/py/metadata_collector/node_extractors.py index d75e0d90..5421c306 100644 --- a/py/metadata_collector/node_extractors.py +++ b/py/metadata_collector/node_extractors.py @@ -2,7 +2,7 @@ import json import os import re -from .constants import MODELS, PROMPTS, SAMPLING, LORAS, SIZE, IMAGES, IS_SAMPLER, OVERWRITE, METADATA_OVERWRITE_FIELDS +from .constants import CLIP_SKIP_SENTINEL, MODELS, PROMPTS, SAMPLING, LORAS, SIZE, IMAGES, IS_SAMPLER, OVERWRITE, METADATA_OVERWRITE_FIELDS def _store_checkpoint_metadata(metadata, node_id, model_name): @@ -1236,7 +1236,10 @@ class MetadataOverwriteExtractor(NodeMetadataExtractor): overwrite_params = {} for key in METADATA_OVERWRITE_FIELDS: value = inputs.get(key) - if value: # truthy — only overwrite when user provided a real value + if key == "clip_skip": + if value != CLIP_SKIP_SENTINEL: + overwrite_params[key] = value + elif value: # truthy — only overwrite when user provided a real value overwrite_params[key] = value if overwrite_params: diff --git a/py/nodes/metadata_overwrite.py b/py/nodes/metadata_overwrite.py index b29eb3ca..c36613a5 100644 --- a/py/nodes/metadata_overwrite.py +++ b/py/nodes/metadata_overwrite.py @@ -1,13 +1,18 @@ """Metadata Overwrite node — allows users to manually specify generation parameters that override the automatically collected/inferred metadata. -All inputs have falsy defaults: only truthy (non-empty / non-zero) values -will overwrite the corresponding field in the final metadata. +Most inputs have falsy defaults (empty string / 0) which are skipped. +clip_skip uses a sentinel default (-25) so that a wired value of 0 is +preserved — both ComfyUI and A1111 conventions have no meaningful 0 value, +but users may wire 0 to express "no clip skip / default". """ from typing import Any -from ..metadata_collector.constants import METADATA_OVERWRITE_FIELDS +from ..metadata_collector.constants import ( + CLIP_SKIP_SENTINEL as _CLIP_SKIP_SENTINEL, + METADATA_OVERWRITE_FIELDS, +) class MetadataOverwriteLM: @@ -116,10 +121,14 @@ class MetadataOverwriteLM: "clip_skip": ( "INT", { - "default": 0, - "min": -24, + "default": _CLIP_SKIP_SENTINEL, + "min": -25, "max": 24, - "tooltip": "Clip skip. Only overwrites when non-zero.", + "tooltip": ( + "Clip skip (ComfyUI: -24..-1, A1111: 1+). " + "Default -25 means not set — any other value " + "overwrites." + ), }, ), "additional_data": ( @@ -144,14 +153,18 @@ class MetadataOverwriteLM: OUTPUT_NODE = True def collect_metadata(self, **kwargs: Any) -> tuple[dict[str, Any]]: - """Collect non-falsy input values into a metadata dict. + """Collect non-default input values into a metadata dict. - Only values that are truthy (non-empty string, non-zero number) - are included — matching the overwrite logic in the metadata pipeline. + For most fields, a falsy value (empty string, 0) means "not set" + and is skipped. clip_skip uses a dedicated sentinel (-25) so that + a wired value of 0 is preserved and reaches the metadata pipeline. """ result: dict[str, Any] = {} for key in METADATA_OVERWRITE_FIELDS: value = kwargs.get(key) - if value: + if key == "clip_skip": + if value != _CLIP_SKIP_SENTINEL: + result[key] = value + elif value: result[key] = value return (result,) diff --git a/py/nodes/save_image.py b/py/nodes/save_image.py index 8b0532a4..92d5ad72 100644 --- a/py/nodes/save_image.py +++ b/py/nodes/save_image.py @@ -476,11 +476,9 @@ class SaveImageLM: params.append(f"Seed: {seed}") if size: params.append(f"Size: {size}") - if clip_skip: + if clip_skip is not None: try: - cs = int(clip_skip) - if cs != 0: - params.append(f"Clip skip: {abs(cs)}") + params.append(f"Clip skip: {abs(int(clip_skip))}") except (ValueError, TypeError): pass additional_data = metadata_dict.get("additional_data", "") diff --git a/tests/metadata_collector/test_metadata_collector.py b/tests/metadata_collector/test_metadata_collector.py index b3c49435..22b14788 100644 --- a/tests/metadata_collector/test_metadata_collector.py +++ b/tests/metadata_collector/test_metadata_collector.py @@ -870,7 +870,8 @@ def test_metadata_overwrite_extractor_stores_truthy_values(metadata_registry): assert "steps" not in params assert "sampler" not in params assert "scheduler" not in params - assert "clip_skip" not in params + # clip_skip=0 is now stored (0 != sentinel -25) — wired 0 is valid + assert params["clip_skip"] == 0 metadata_registry.clear_metadata() @@ -880,8 +881,10 @@ def test_metadata_overwrite_extractor_empty_inputs(metadata_registry): metadata_registry.start_collection("prompt-ow2") metadata = metadata_registry.prompt_metadata["prompt-ow2"] + from py.metadata_collector.constants import CLIP_SKIP_SENTINEL + inputs = {key: "" for key in METADATA_OVERWRITE_FIELDS} - inputs.update({"seed": 0, "steps": 0, "cfg_scale": 0.0, "clip_skip": 0}) + inputs.update({"seed": 0, "steps": 0, "cfg_scale": 0.0, "clip_skip": CLIP_SKIP_SENTINEL}) MetadataOverwriteExtractor.extract("ow-2", inputs, None, metadata) @@ -950,7 +953,8 @@ def test_extract_generation_params_overwrite_falsy_skipped(metadata_registry, po registry_obj.set_current_prompt(populated_registry["prompt"]) metadata2 = registry_obj.prompt_metadata["promptA"] - # Inject overwrite with falsy values + # Inject overwrite with falsy values (except clip_skip=0 which is now + # treated as a valid wired input thanks to the -25 sentinel) metadata2[OVERWRITE] = { "ow-1": { "parameters": { @@ -974,6 +978,9 @@ def test_extract_generation_params_overwrite_falsy_skipped(metadata_registry, po assert params["prompt"] == "A castle on a hill" assert params["cfg_scale"] == 7.5 + # clip_skip=0 is a valid wired value (not the -25 sentinel) — should be applied + assert params["clip_skip"] == 0 + registry_obj.clear_metadata()