mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-09-20 18:51:26 -03:00
fix(other-models): stop warning about legacy folder keys that alias
Enabling Other Models logged two warnings on a stock ComfyUI install:
Detected the same folder '.../clip' under multiple other-model categories
('.../clip' is already mapped). Keeping the first category; please fix
your path configuration.
Nothing was wrong with the configuration. ComfyUI's folder_paths rewrites
legacy names before every access (map_legacy: clip -> text_encoders,
unet -> diffusion_models) and registers both legacy directories under the
canonical key, so get_folder_paths("clip") returns exactly the same list as
get_folder_paths("text_encoders"). Both keys are in the enabled allow-list,
so the second pass hit the overlap guard for every text-encoder folder and
printed advice the user cannot act on. The path list itself was correct
(deduped), only the message was wrong.
- Config._collapse_legacy_folder_keys() drops a key when the host exposes
map_legacy and resolves it to another queried key. That is provably
lossless: an empty canonical list implies an empty alias list. The
standalone MockFolderPaths has no map_legacy and its keys are independent
settings.json entries, so every key is still queried there.
- _prepare_other_paths() now tracks the claiming sub_type alongside the
business path and downgrades a same-sub_type duplicate to debug, keeping
the warning for a genuine cross-category collision (and naming the other
category in the message).
Regression tests cover the aliased-key layout (no warning, no redundant
query, both folders still managed) and the same-sub_type duplicate, and the
opt-in test is parametrized over controlnet and clip_vision.
This commit is contained in:
+72
-11
@@ -1169,6 +1169,46 @@ class Config:
|
|||||||
if sub_type in allowed
|
if sub_type in allowed
|
||||||
]
|
]
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _collapse_legacy_folder_keys(keys: List[str]) -> List[str]:
|
||||||
|
"""Drop folder keys the host already normalizes onto another queried key.
|
||||||
|
|
||||||
|
ComfyUI's ``folder_paths`` rewrites legacy names before every access
|
||||||
|
(``clip`` -> ``text_encoders``, ``unet`` -> ``diffusion_models``), and
|
||||||
|
registers both legacy directories under the canonical key, so
|
||||||
|
``get_folder_paths("clip")`` returns exactly the same list as
|
||||||
|
``get_folder_paths("text_encoders")``. Querying both therefore reports
|
||||||
|
every text-encoder folder twice and trips the overlap guard with a
|
||||||
|
conflict the user cannot fix.
|
||||||
|
|
||||||
|
When the host exposes ``map_legacy`` the alias is provably redundant and
|
||||||
|
is skipped (an empty canonical list implies an empty alias list).
|
||||||
|
Without it - the standalone mock, whose keys are independent
|
||||||
|
``settings.json`` entries - every key is kept, because a ``clip``-only
|
||||||
|
configuration is then genuinely distinct.
|
||||||
|
"""
|
||||||
|
map_legacy = getattr(folder_paths, "map_legacy", None)
|
||||||
|
if not callable(map_legacy):
|
||||||
|
return list(keys)
|
||||||
|
|
||||||
|
queried = set(keys)
|
||||||
|
collapsed: List[str] = []
|
||||||
|
for key in keys:
|
||||||
|
try:
|
||||||
|
canonical = map_legacy(key)
|
||||||
|
except Exception:
|
||||||
|
canonical = key
|
||||||
|
if canonical != key and canonical in queried:
|
||||||
|
logger.debug(
|
||||||
|
"Skipping legacy folder key '%s'; the host resolves it to "
|
||||||
|
"'%s', which is queried as well.",
|
||||||
|
key,
|
||||||
|
canonical,
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
collapsed.append(key)
|
||||||
|
return collapsed
|
||||||
|
|
||||||
def _prepare_other_paths(
|
def _prepare_other_paths(
|
||||||
self, folder_path_map: Mapping[str, Iterable[str]]
|
self, folder_path_map: Mapping[str, Iterable[str]]
|
||||||
) -> Tuple[List[str], Dict[str, str], Dict[str, List[str]]]:
|
) -> Tuple[List[str], Dict[str, str], Dict[str, List[str]]]:
|
||||||
@@ -1182,7 +1222,8 @@ class Config:
|
|||||||
unique_paths: List[str] = []
|
unique_paths: List[str] = []
|
||||||
sub_type_map: Dict[str, str] = {}
|
sub_type_map: Dict[str, str] = {}
|
||||||
per_key_roots: Dict[str, List[str]] = {}
|
per_key_roots: Dict[str, List[str]] = {}
|
||||||
seen_real_paths: Dict[str, str] = {} # real path -> business path
|
# real path -> (business path, sub_type) of the category that claimed it
|
||||||
|
seen_real_paths: Dict[str, Tuple[str, str]] = {}
|
||||||
|
|
||||||
# Cross-scanner overlap detection: warn when an "other" root is
|
# Cross-scanner overlap detection: warn when an "other" root is
|
||||||
# already covered by the checkpoints/unet or embeddings scanners.
|
# already covered by the checkpoints/unet or embeddings scanners.
|
||||||
@@ -1206,16 +1247,31 @@ class Config:
|
|||||||
for real_path, business_path in sorted(
|
for real_path, business_path in sorted(
|
||||||
path_map.items(), key=lambda item: item[1].lower()
|
path_map.items(), key=lambda item: item[1].lower()
|
||||||
):
|
):
|
||||||
if real_path in seen_real_paths:
|
seen = seen_real_paths.get(real_path)
|
||||||
logger.warning(
|
if seen is not None:
|
||||||
"Detected the same folder '%s' under multiple other-model "
|
seen_business_path, seen_sub_type = seen
|
||||||
"categories ('%s' is already mapped). Keeping the first "
|
if seen_sub_type == sub_type:
|
||||||
"category; please fix your path configuration.",
|
# Same category reached through a second folder_paths
|
||||||
business_path,
|
# key (legacy alias, or a sub_type spanning two keys).
|
||||||
seen_real_paths[real_path],
|
# Expected, so never a "fix your configuration" warning.
|
||||||
)
|
logger.debug(
|
||||||
|
"Ignoring duplicate folder '%s' for category '%s' "
|
||||||
|
"(already covered by '%s').",
|
||||||
|
business_path,
|
||||||
|
sub_type,
|
||||||
|
seen_business_path,
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
logger.warning(
|
||||||
|
"Detected the same folder '%s' under multiple other-model "
|
||||||
|
"categories ('%s' is already mapped as '%s'). Keeping the "
|
||||||
|
"first category; please fix your path configuration.",
|
||||||
|
business_path,
|
||||||
|
seen_business_path,
|
||||||
|
seen_sub_type,
|
||||||
|
)
|
||||||
continue
|
continue
|
||||||
seen_real_paths[real_path] = business_path
|
seen_real_paths[real_path] = (business_path, sub_type)
|
||||||
unique_paths.append(business_path)
|
unique_paths.append(business_path)
|
||||||
key_roots.append(business_path)
|
key_roots.append(business_path)
|
||||||
sub_type_map[business_path] = sub_type
|
sub_type_map[business_path] = sub_type
|
||||||
@@ -1394,10 +1450,15 @@ class Config:
|
|||||||
Iterates the enabled OTHER_MODEL_FOLDER_SUBTYPES keys and pulls each
|
Iterates the enabled OTHER_MODEL_FOLDER_SUBTYPES keys and pulls each
|
||||||
from ``folder_paths.get_folder_paths(key)`` (in standalone mode the
|
from ``folder_paths.get_folder_paths(key)`` (in standalone mode the
|
||||||
mock serves arbitrary keys from ``settings.json.folder_paths``).
|
mock serves arbitrary keys from ``settings.json.folder_paths``).
|
||||||
|
Legacy aliases the host normalizes onto a canonical key (``clip`` ->
|
||||||
|
``text_encoders``) are collapsed first so the same folders are not
|
||||||
|
reported twice.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
folder_path_map: Dict[str, List[str]] = {}
|
folder_path_map: Dict[str, List[str]] = {}
|
||||||
for key in self._get_enabled_other_folder_keys():
|
for key in self._collapse_legacy_folder_keys(
|
||||||
|
self._get_enabled_other_folder_keys()
|
||||||
|
):
|
||||||
try:
|
try:
|
||||||
folder_path_map[key] = folder_paths.get_folder_paths(key)
|
folder_path_map[key] = folder_paths.get_folder_paths(key)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
|
|||||||
@@ -116,6 +116,50 @@ class TestPrepareOtherPaths:
|
|||||||
]
|
]
|
||||||
assert len(warnings) == 1
|
assert len(warnings) == 1
|
||||||
|
|
||||||
|
def test_same_sub_type_duplicate_is_debug_not_warning(self, tmp_path, caplog):
|
||||||
|
"""A sub_type spanning two folder keys legitimately sees a folder twice.
|
||||||
|
|
||||||
|
``clip`` and ``text_encoders`` both map to ``text_encoder``, so a folder
|
||||||
|
reachable through both is expected and must not tell the user to fix a
|
||||||
|
configuration they cannot fix.
|
||||||
|
"""
|
||||||
|
text_encoders_dir = tmp_path / "text_encoders"
|
||||||
|
legacy_clip_dir = tmp_path / "clip"
|
||||||
|
text_encoders_dir.mkdir()
|
||||||
|
legacy_clip_dir.mkdir()
|
||||||
|
|
||||||
|
config = _make_config()
|
||||||
|
with caplog.at_level(logging.DEBUG, logger=config_module.logger.name):
|
||||||
|
unique, sub_type_map, per_key = config._prepare_other_paths(
|
||||||
|
{
|
||||||
|
"text_encoders": [str(text_encoders_dir)],
|
||||||
|
"clip": [str(legacy_clip_dir), str(text_encoders_dir)],
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
assert set(unique) == {
|
||||||
|
_normalize(str(text_encoders_dir)),
|
||||||
|
_normalize(str(legacy_clip_dir)),
|
||||||
|
}
|
||||||
|
assert sub_type_map[_normalize(str(legacy_clip_dir))] == "text_encoder"
|
||||||
|
assert per_key["clip"] == [_normalize(str(legacy_clip_dir))]
|
||||||
|
|
||||||
|
warnings = [
|
||||||
|
record.message
|
||||||
|
for record in caplog.records
|
||||||
|
if record.levelname == "WARNING"
|
||||||
|
and "multiple other-model categories" in record.message
|
||||||
|
]
|
||||||
|
assert warnings == []
|
||||||
|
|
||||||
|
debug_messages = [
|
||||||
|
record.message
|
||||||
|
for record in caplog.records
|
||||||
|
if record.levelname == "DEBUG"
|
||||||
|
and "Ignoring duplicate folder" in record.message
|
||||||
|
]
|
||||||
|
assert len(debug_messages) == 1
|
||||||
|
|
||||||
def test_cross_scanner_overlap_warns_but_keeps_path(self, tmp_path, caplog):
|
def test_cross_scanner_overlap_warns_but_keeps_path(self, tmp_path, caplog):
|
||||||
"""An other root overlapping a checkpoint root warns but stays managed."""
|
"""An other root overlapping a checkpoint root warns but stays managed."""
|
||||||
shared = tmp_path / "shared_models"
|
shared = tmp_path / "shared_models"
|
||||||
@@ -175,7 +219,7 @@ class TestInitOtherPaths:
|
|||||||
config_module.folder_paths, "get_folder_paths", get_folder_paths
|
config_module.folder_paths, "get_folder_paths", get_folder_paths
|
||||||
)
|
)
|
||||||
|
|
||||||
def test_default_enabled_keys_exclude_controlnet(self, monkeypatch, tmp_path):
|
def test_default_enabled_keys_exclude_opt_in_types(self, monkeypatch, tmp_path):
|
||||||
dirs = {}
|
dirs = {}
|
||||||
for key in (
|
for key in (
|
||||||
"vae",
|
"vae",
|
||||||
@@ -194,25 +238,79 @@ class TestInitOtherPaths:
|
|||||||
config = _make_config()
|
config = _make_config()
|
||||||
roots = config._init_other_paths()
|
roots = config._init_other_paths()
|
||||||
|
|
||||||
assert _normalize(dirs["controlnet"]) not in roots
|
# clip_vision and controlnet are workflow-driven categories and stay
|
||||||
assert _normalize(dirs["controlnet"]) not in config.other_root_subtypes
|
# opt-in; only VAE / upscaler / text encoder are managed by default.
|
||||||
for key in ("vae", "upscale_models", "text_encoders", "clip", "clip_vision"):
|
for key in ("clip_vision", "controlnet"):
|
||||||
|
assert _normalize(dirs[key]) not in roots
|
||||||
|
assert _normalize(dirs[key]) not in config.other_root_subtypes
|
||||||
|
# This stub has no map_legacy (standalone-shaped), so the legacy clip
|
||||||
|
# key is queried on its own and its folder lands under text_encoder.
|
||||||
|
for key in ("vae", "upscale_models", "text_encoders", "clip"):
|
||||||
assert _normalize(dirs[key]) in roots
|
assert _normalize(dirs[key]) in roots
|
||||||
|
|
||||||
def test_controlnet_opt_in_via_setting(self, monkeypatch, tmp_path):
|
def test_legacy_key_is_not_queried_when_host_aliases_it(
|
||||||
controlnet_dir = tmp_path / "controlnet"
|
self, monkeypatch, tmp_path, caplog
|
||||||
controlnet_dir.mkdir()
|
):
|
||||||
|
"""ComfyUI resolves clip -> text_encoders, so only the canonical key is
|
||||||
|
queried: its folder list already contains the legacy directory."""
|
||||||
|
canonical_dir = tmp_path / "text_encoders"
|
||||||
|
legacy_dir = tmp_path / "clip"
|
||||||
|
canonical_dir.mkdir()
|
||||||
|
legacy_dir.mkdir()
|
||||||
|
|
||||||
self._stub_folder_paths(monkeypatch, {"controlnet": str(controlnet_dir)})
|
queried = []
|
||||||
get_settings_manager().set("enabled_other_sub_types", ["controlnet"])
|
|
||||||
|
def get_folder_paths(key):
|
||||||
|
queried.append(key)
|
||||||
|
if key == "text_encoders":
|
||||||
|
# Mirrors ComfyUI folder_paths: both directories are registered
|
||||||
|
# under the canonical key.
|
||||||
|
return [str(canonical_dir), str(legacy_dir)]
|
||||||
|
return []
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
config_module.folder_paths, "get_folder_paths", get_folder_paths
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
config_module.folder_paths,
|
||||||
|
"map_legacy",
|
||||||
|
lambda key: {"clip": "text_encoders"}.get(key, key),
|
||||||
|
raising=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
config = _make_config()
|
||||||
|
with caplog.at_level(logging.DEBUG, logger=config_module.logger.name):
|
||||||
|
roots = config._init_other_paths()
|
||||||
|
|
||||||
|
assert "clip" not in queried
|
||||||
|
assert _normalize(str(canonical_dir)) in roots
|
||||||
|
assert _normalize(str(legacy_dir)) in roots
|
||||||
|
assert (
|
||||||
|
config.other_root_subtypes[_normalize(str(legacy_dir))]
|
||||||
|
== "text_encoder"
|
||||||
|
)
|
||||||
|
# The reported bug: this layout used to log "please fix your path
|
||||||
|
# configuration" twice for aliased keys the user cannot separate.
|
||||||
|
assert [
|
||||||
|
record.message
|
||||||
|
for record in caplog.records
|
||||||
|
if record.levelname == "WARNING"
|
||||||
|
] == []
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("opt_in_key", ["controlnet", "clip_vision"])
|
||||||
|
def test_opt_in_sub_type_via_setting(self, monkeypatch, tmp_path, opt_in_key):
|
||||||
|
opt_in_dir = tmp_path / opt_in_key
|
||||||
|
opt_in_dir.mkdir()
|
||||||
|
|
||||||
|
self._stub_folder_paths(monkeypatch, {opt_in_key: str(opt_in_dir)})
|
||||||
|
get_settings_manager().set("enabled_other_sub_types", [opt_in_key])
|
||||||
|
|
||||||
config = _make_config()
|
config = _make_config()
|
||||||
roots = config._init_other_paths()
|
roots = config._init_other_paths()
|
||||||
|
|
||||||
assert _normalize(str(controlnet_dir)) in roots
|
assert _normalize(str(opt_in_dir)) in roots
|
||||||
assert (
|
assert (
|
||||||
config.other_root_subtypes[_normalize(str(controlnet_dir))]
|
config.other_root_subtypes[_normalize(str(opt_in_dir))] == opt_in_key
|
||||||
== "controlnet"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
def test_disabled_sub_type_is_not_scanned(self, monkeypatch, tmp_path):
|
def test_disabled_sub_type_is_not_scanned(self, monkeypatch, tmp_path):
|
||||||
|
|||||||
Reference in New Issue
Block a user