From 0ada32d0c7fbbc32c86b41f2e4be07ebdc09cbe8 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Wed, 23 Sep 2026 13:16:26 +0800 Subject: [PATCH] fix(organize): stop keyword-dump tags from becoming folder names (#1119) CivitAI tags are normally short single-concept labels, but some uploaders pack their entire keyword list into one tag. The model in #1119 carries "lora, character, rosie, irish, ... face" as a single 181-character tag. Priority resolution matches aliases by exact equality, so that tag matched nothing and resolve_priority_tag_for_model fell back to tags[0] -- the blob. With the default "{base_model}/{first_tag}" template the model was filed under "Krea 2/<181-character blob>/", and the full path plus the ".civitai.info" sidecar and the preview images next to it ran into the Windows MAX_PATH limit. Tags also bypassed sanitization on the way into a path: both calculate_relative_path_for_model and DownloadManager._calculate_relative_path sanitized model_name and version_name but interpolated {first_tag} verbatim, so a tag containing "/" or ":" silently produced nested or illegal folders. Two changes: - The fallback skips tags that cannot serve as a folder name. is_usable_path_tag rejects comma-separated keyword dumps and tags longer than MAX_PATH_TAG_LENGTH; the resolver returns "" when nothing usable is left, which callers already render as "no tags". Whole-tag priority matching is untouched, so existing priority configurations behave the same. - sanitize_folder_name gains an optional max_length, and every tag-derived segment now goes through it. Tags are capped at MAX_PATH_TAG_LENGTH, model and version names at MAX_FOLDER_NAME_LENGTH, and rendered filename stems at MAX_FILENAME_STEM_LENGTH. For the reported model the folder becomes "Krea 2/base model" instead of the blob, and the full path drops from 235 to 64 characters. Existing libraries are not migrated up front: a path is only recomputed on download, on an auto-organize run or when a filename template is applied, and values already inside the caps are left byte-identical. Models previously filed under a keyword-dump folder move on the next auto-organize run. --- docs/priority_tags_help.md | 6 +- py/services/download_manager.py | 16 ++- py/services/settings_manager.py | 8 +- py/utils/constants.py | 13 ++ py/utils/tag_priorities.py | 26 ++++ py/utils/utils.py | 56 ++++++-- tests/services/test_download_manager_basic.py | 41 ++++++ tests/services/test_settings_manager.py | 47 +++++++ tests/utils/test_utils.py | 121 ++++++++++++++++++ 9 files changed, 318 insertions(+), 16 deletions(-) diff --git a/docs/priority_tags_help.md b/docs/priority_tags_help.md index 155e7093..1a14cf35 100644 --- a/docs/priority_tags_help.md +++ b/docs/priority_tags_help.md @@ -30,7 +30,8 @@ Aliases live inside `()` and are separated with `|`. The canonical name is what When your path template contains `{first_tag}`, the app picks a folder name based on your priority list and the model’s own tags: - It checks the priority list from top to bottom. If a canonical tag or any of its aliases appear in the model tags, that canonical name becomes the folder name. -- If no priority tags are found but the model has tags, the very first model tag is used. +- If no priority tags are found but the model has tags, the first tag that can be used as a folder name is chosen. +- Tags that contain a comma, or that are longer than 50 characters, are treated as unusable and skipped: some uploaders pack their whole keyword list into a single tag. If every tag is unusable, the folder falls back to `no tags`. - If the model has no tags at all, the folder falls back to `no tags`. ### Example @@ -42,6 +43,7 @@ With a template like `/{model_type}/{first_tag}` and the priority entry list `ch | `["chars", "female"]` | `character` | `chars` matches the `character` alias, so the canonical wins. | | `["anime", "portrait"]` | `style` | `anime` hits the `style` entry, so its canonical label is used. | | `["portrait", "bw"]` | `portrait` | No priority match, so the first model tag is used. | +| `["lora, character, rosie, ... face"]` | `no tags` | The only tag is a keyword dump, so it is skipped. | | `[]` | `no tags` | Nothing to match, so the fallback is applied. | ## 3. Save the Settings @@ -61,10 +63,12 @@ After editing the entry list, press **Enter** to save. Use **Shift+Enter** whene - Keep canonical names short and meaningful—they become folder names. - Place the most important categories first; the first match wins. - Avoid duplicate canonical names within the same list; only the first instance is used. +- Folder names built from tags are sanitized for filesystem safety and truncated to 50 characters. ## Troubleshooting - **Unexpected folder name?** Check that the canonical name you want is placed before other matches. +- **Folder named `no tags`?** Every model tag was either missing or unusable (a comma-separated keyword dump, or longer than 50 characters). Add the tags you care about to your priority list so they match by name instead. - **Alias not working?** Ensure the alias is inside parentheses and separated with `|`, e.g. `character(char|chars)`. - **Validation error?** Look for missing parentheses or stray commas. Each entry must follow the `canonical(alias|alias)` pattern or just `canonical`. diff --git a/py/services/download_manager.py b/py/services/download_manager.py index d37bbf40..82164a5d 100644 --- a/py/services/download_manager.py +++ b/py/services/download_manager.py @@ -25,6 +25,8 @@ from ..utils.models import ( ) from ..utils.constants import ( CARD_PREVIEW_WIDTH, + MAX_FOLDER_NAME_LENGTH, + MAX_PATH_TAG_LENGTH, MODEL_WEIGHT_FILE_TYPES, SUPPORTED_DOWNLOAD_SKIP_BASE_MODELS, VALID_LORA_TYPES, @@ -2327,16 +2329,26 @@ class DownloadManager: if not first_tag: first_tag = "no tags" # Default if no tags available + # Tags come straight from CivitAI, so sanitize the value before it + # becomes a path segment and cap its length (#1119). + first_tag = sanitize_folder_name(first_tag, max_length=MAX_PATH_TAG_LENGTH) + # Format the template with available data formatted_path = path_template formatted_path = formatted_path.replace("{base_model}", mapped_base_model) formatted_path = formatted_path.replace("{first_tag}", first_tag) formatted_path = formatted_path.replace("{author}", author) formatted_path = formatted_path.replace( - "{model_name}", sanitize_folder_name(model_info.get("name", "")) + "{model_name}", + sanitize_folder_name( + model_info.get("name", ""), max_length=MAX_FOLDER_NAME_LENGTH + ), ) formatted_path = formatted_path.replace( - "{version_name}", sanitize_folder_name(version_info.get("name", "")) + "{version_name}", + sanitize_folder_name( + version_info.get("name", ""), max_length=MAX_FOLDER_NAME_LENGTH + ), ) if model_type == "embedding": diff --git a/py/services/settings_manager.py b/py/services/settings_manager.py index 54848adc..b4b913eb 100644 --- a/py/services/settings_manager.py +++ b/py/services/settings_manager.py @@ -47,6 +47,7 @@ from ..utils.settings_paths import ( from ..utils.tag_priorities import ( PriorityTagEntry, collect_canonical_tags, + is_usable_path_tag, parse_priority_tag_string, resolve_priority_tag, ) @@ -1569,9 +1570,12 @@ class SettingsManager: if resolved: return resolved + # Fall back to the first tag that is usable as a folder name. The raw + # tag list can contain keyword dumps that would become unusable folders + # and break path length limits, so skip those (#1119). for tag in tags: - if isinstance(tag, str) and tag: - return tag + if is_usable_path_tag(tag): + return tag.strip() return "" def get_priority_tag_suggestions(self) -> Dict[str, List[str]]: diff --git a/py/utils/constants.py b/py/utils/constants.py index 445d8e97..9f81e0d4 100644 --- a/py/utils/constants.py +++ b/py/utils/constants.py @@ -293,6 +293,19 @@ DEFAULT_DOWNLOAD_PATH_TEMPLATES: Dict[str, str] = { "other": "", } +# Length guards for template placeholders that end up in file and folder names. +# Windows enforces MAX_PATH (260 characters) on the full path and 255 on a +# single path component, and a model folder also has to leave room for the +# model file, its ".civitai.info"/".json" sidecars and preview images. Values +# stay well below those limits so the surrounding files still fit. +# +# Tags get a much tighter budget than other names: some CivitAI uploaders dump +# their whole keyword list into a single tag (see issue #1119), and such a tag +# is only useful as a folder name after truncation. +MAX_FOLDER_NAME_LENGTH = 100 +MAX_PATH_TAG_LENGTH = 50 +MAX_FILENAME_STEM_LENGTH = 150 + # baseModel values from CivitAI that should be treated as diffusion models (unet) # These model types are incorrectly labeled as "checkpoint" by CivitAI but are actually diffusion models DIFFUSION_MODEL_BASE_MODELS = frozenset( diff --git a/py/utils/tag_priorities.py b/py/utils/tag_priorities.py index 3bf2062a..6b85bc14 100644 --- a/py/utils/tag_priorities.py +++ b/py/utils/tag_priorities.py @@ -5,6 +5,8 @@ from __future__ import annotations from dataclasses import dataclass from typing import Dict, Iterable, List, Optional, Sequence, Set +from .constants import MAX_PATH_TAG_LENGTH + @dataclass(frozen=True) class PriorityTagEntry: @@ -102,3 +104,27 @@ def collect_canonical_tags(entries: Iterable[PriorityTagEntry]) -> List[str]: """Return the ordered list of canonical tags from the parsed entries.""" return [entry.canonical for entry in entries] + + +def is_usable_path_tag(tag: object) -> bool: + """Return True when a tag is a sane single-concept folder-name candidate. + + CivitAI tags are normally short labels ("character", "anime"), but some + uploaders dump their whole keyword list into a single tag, e.g. + ``"lora, character, rosie, irish, ... face"``. Using such a tag as a folder + name produces unwieldy and path-length-breaking directories (#1119), so + tag-derived path segments only accept single-concept tags. + """ + + if not isinstance(tag, str): + return False + + candidate = tag.strip() + if not candidate: + return False + + # Commas mean the tag is a keyword dump rather than one concept. + if "," in candidate: + return False + + return len(candidate) <= MAX_PATH_TAG_LENGTH diff --git a/py/utils/utils.py b/py/utils/utils.py index db60e3f8..d773b86b 100644 --- a/py/utils/utils.py +++ b/py/utils/utils.py @@ -6,6 +6,11 @@ from typing import Any, Dict, List, Optional from ..services.service_registry import ServiceRegistry from ..config import config from ..services.settings_manager import get_settings_manager +from .constants import ( + MAX_FILENAME_STEM_LENGTH, + MAX_FOLDER_NAME_LENGTH, + MAX_PATH_TAG_LENGTH, +) import asyncio logger = logging.getLogger(__name__) @@ -417,12 +422,17 @@ def fuzzy_match(text: str, pattern: str, threshold: float = 0.85) -> bool: return True -def sanitize_folder_name(name: str, replacement: str = "_") -> str: +def sanitize_folder_name( + name: str, replacement: str = "_", max_length: Optional[int] = None +) -> str: """Sanitize a folder name by removing or replacing invalid characters. Args: name: The original folder name. replacement: The character to use when replacing invalid characters. + max_length: Optional maximum length for the resulting name. Longer + names are truncated (and re-trimmed) so that a single untrusted + value cannot blow past filesystem path limits. Returns: A sanitized folder name safe to use across common filesystems. @@ -449,6 +459,15 @@ def sanitize_folder_name(name: str, replacement: str = "_") -> str: # If no replacement, just strip spaces and dots from right, spaces from left sanitized = sanitized.rstrip(" .").lstrip(" ") + if max_length is not None and max_length > 0 and len(sanitized) > max_length: + sanitized = sanitized[:max_length] + # Re-trim separators and spaces exposed by the cut so the truncated + # name stays filesystem-safe. + if replacement: + sanitized = sanitized.rstrip(" ." + replacement).lstrip(" " + replacement) + else: + sanitized = sanitized.rstrip(" .").lstrip(" ") + if not sanitized: return "unnamed" @@ -575,12 +594,20 @@ def calculate_relative_path_for_model( if not first_tag: first_tag = "no tags" # Default if no tags available + # Tags are user-generated on CivitAI, so sanitize the value before it + # becomes a path segment and cap its length (#1119). + first_tag = sanitize_folder_name(first_tag, max_length=MAX_PATH_TAG_LENGTH) + # Format the template with available data - model_name = sanitize_folder_name(model_data.get("model_name", "")) + model_name = sanitize_folder_name( + model_data.get("model_name", ""), max_length=MAX_FOLDER_NAME_LENGTH + ) version_name = "" if isinstance(civitai_data, dict): - version_name = sanitize_folder_name(civitai_data.get("name") or "") + version_name = sanitize_folder_name( + civitai_data.get("name") or "", max_length=MAX_FOLDER_NAME_LENGTH + ) formatted_path = path_template formatted_path = formatted_path.replace("{base_model}", mapped_base_model) @@ -667,20 +694,22 @@ def calculate_filename_for_model( else: original_name = os.path.splitext(str(model_data.get("file_name", "")))[0] - def _sanitize_value(value: Any) -> str: + def _sanitize_value(value: Any, max_length: Optional[int] = None) -> str: # sanitize_folder_name falls back to "unnamed" for empty input; for # templates an empty value must stay empty so segments collapse. text = str(value) if value else "" - return sanitize_folder_name(text) if text else "" + if not text: + return "" + return sanitize_folder_name(text, max_length=max_length) replacements = { - "{model_name}": _sanitize_value(model_name), - "{version_name}": _sanitize_value(version_name), - "{base_model}": _sanitize_value(mapped_base_model), - "{author}": _sanitize_value(author), - "{first_tag}": _sanitize_value(first_tag), + "{model_name}": _sanitize_value(model_name, MAX_FILENAME_STEM_LENGTH), + "{version_name}": _sanitize_value(version_name, MAX_FILENAME_STEM_LENGTH), + "{base_model}": _sanitize_value(mapped_base_model, MAX_FILENAME_STEM_LENGTH), + "{author}": _sanitize_value(author, MAX_FILENAME_STEM_LENGTH), + "{first_tag}": _sanitize_value(first_tag, MAX_PATH_TAG_LENGTH), "{hash_short}": hash_short, - "{original_name}": _sanitize_value(original_name), + "{original_name}": _sanitize_value(original_name, MAX_FILENAME_STEM_LENGTH), } result = template @@ -699,6 +728,11 @@ def calculate_filename_for_model( # A stem must not start or end with separators, spaces or dots. result = result.strip("-_. ") + # A template can concatenate several values, so cap the rendered stem as + # well and re-trim the cut. + if len(result) > MAX_FILENAME_STEM_LENGTH: + result = result[:MAX_FILENAME_STEM_LENGTH].strip("-_. ") + return result diff --git a/tests/services/test_download_manager_basic.py b/tests/services/test_download_manager_basic.py index 62b1d78a..69a9504d 100644 --- a/tests/services/test_download_manager_basic.py +++ b/tests/services/test_download_manager_basic.py @@ -238,6 +238,47 @@ async def test_successful_download_uses_defaults( assert captured["download_urls"] == ["https://example.invalid/file.safetensors"] +def test_calculate_relative_path_ignores_keyword_dump_tag(): + """The #1119 download flow: a keyword-dump tag must not become a folder.""" + keyword_dump = ( + "lora, character, rosie, irish, redhead, auburn, freckles, green eyes, " + "curly hair, woman, female, photorealistic, realistic, krea2, dark beast, " + "kreativity, nsfw, nude, portrait, face" + ) + manager = DownloadManager() + + relative_path = manager._calculate_relative_path( + { + "baseModel": "BaseModel", + "creator": {"username": "mad_macs"}, + "name": "v1.2", + "model": {"name": "Rosie", "tags": [keyword_dump, "base model"]}, + }, + "lora", + ) + + assert relative_path == "MappedModel/base model" + assert keyword_dump not in relative_path + assert len(relative_path) < 50 + + +def test_calculate_relative_path_sanitizes_tag_segment(): + """A tag with path separators must not create nested folders.""" + manager = DownloadManager() + + relative_path = manager._calculate_relative_path( + { + "baseModel": "BaseModel", + "creator": {"username": "author"}, + "name": "v1.2", + "model": {"name": "Rosie", "tags": ["a/b:c"]}, + }, + "lora", + ) + + assert relative_path == "MappedModel/a_b_c" + + @pytest.mark.asyncio async def test_download_accepts_enhancement_lora_primary_file( monkeypatch, scanners, metadata_provider, tmp_path diff --git a/tests/services/test_settings_manager.py b/tests/services/test_settings_manager.py index 9a2148e3..3c6aee04 100644 --- a/tests/services/test_settings_manager.py +++ b/tests/services/test_settings_manager.py @@ -365,6 +365,53 @@ def test_download_path_template_unknown_type_is_flat(manager): assert manager.get_download_path_template("not-a-model-type") == "" +# Real CivitAI data for the model reported in issue #1119: the uploader dumped +# a whole keyword list into a single tag. +KEYWORD_DUMP_TAG = ( + "lora, character, rosie, irish, redhead, auburn, freckles, green eyes, " + "curly hair, woman, female, photorealistic, realistic, krea2, dark beast, " + "kreativity, nsfw, nude, portrait, face" +) + + +def test_resolve_priority_tag_prefers_configured_priority(manager): + # Priority order from CIVITAI_MODEL_TAGS: "character" precedes "anime". + assert manager.resolve_priority_tag_for_model(["anime", "character"], "lora") == ( + "character" + ) + + +def test_resolve_priority_tag_falls_back_to_first_usable_tag(manager): + assert ( + manager.resolve_priority_tag_for_model(["portrait", "anime-ish"], "lora") + == "portrait" + ) + + +def test_resolve_priority_tag_skips_keyword_dump_tag(manager): + """A keyword-dump tag must not be used as a folder name (#1119).""" + assert manager.resolve_priority_tag_for_model([KEYWORD_DUMP_TAG], "lora") == "" + + +def test_resolve_priority_tag_skips_keyword_dump_and_uses_next_tag(manager): + assert ( + manager.resolve_priority_tag_for_model([KEYWORD_DUMP_TAG, "portrait"], "lora") + == "portrait" + ) + + +def test_resolve_priority_tag_skips_unusable_tags(manager): + overlong_tag = "x" * 51 + + assert manager.resolve_priority_tag_for_model([overlong_tag], "lora") == "" + assert manager.resolve_priority_tag_for_model([overlong_tag, " "], "lora") == "" + # Non-string entries never win the fallback. + assert manager.resolve_priority_tag_for_model([None, 42], "lora") == "" + # A tag at the length budget is still accepted and stripped. + assert manager.resolve_priority_tag_for_model(["x" * 50], "lora") == "x" * 50 + assert manager.resolve_priority_tag_for_model([" portrait "], "lora") == "portrait" + + def test_auto_set_default_roots(manager): # Clear any previously auto-set values to test fresh behavior manager.settings["default_lora_root"] = "" diff --git a/tests/utils/test_utils.py b/tests/utils/test_utils.py index a371e1c3..f7d89e7b 100644 --- a/tests/utils/test_utils.py +++ b/tests/utils/test_utils.py @@ -2,6 +2,11 @@ import pytest from py.services.settings_manager import SettingsManager, get_settings_manager from py.services.service_registry import ServiceRegistry +from py.utils.constants import ( + MAX_FILENAME_STEM_LENGTH, + MAX_FOLDER_NAME_LENGTH, + MAX_PATH_TAG_LENGTH, +) from py.utils.utils import ( calculate_filename_for_model, calculate_recipe_fingerprint, @@ -12,6 +17,15 @@ from py.utils.utils import ( ) +# Real CivitAI data for the model reported in issue #1119: the uploader dumped +# a whole keyword list into a single tag. +KEYWORD_DUMP_TAG = ( + "lora, character, rosie, irish, redhead, auburn, freckles, green eyes, " + "curly hair, woman, female, photorealistic, realistic, krea2, dark beast, " + "kreativity, nsfw, nude, portrait, face" +) + + class _FakeCache: def __init__(self, items): self.raw_data = list(items) @@ -147,6 +161,66 @@ def test_calculate_relative_path_sanitizes_double_slashes(isolated_settings): assert relative_path == "no tags/Author" +def test_calculate_relative_path_ignores_keyword_dump_tag(isolated_settings): + """A tag holding a whole keyword list must not become a folder name (#1119).""" + model_data = {"base_model": "Krea 2", "tags": [KEYWORD_DUMP_TAG]} + + relative_path = calculate_relative_path_for_model(model_data, "lora") + + assert relative_path == "Krea 2/no tags" + + +def test_calculate_relative_path_uses_next_usable_tag(isolated_settings): + """Unusable tags are skipped instead of hijacking the folder name (#1119).""" + model_data = {"base_model": "Krea 2", "tags": [KEYWORD_DUMP_TAG, "portrait"]} + + assert calculate_relative_path_for_model(model_data, "lora") == "Krea 2/portrait" + + +def test_calculate_relative_path_sanitizes_tag_segment(isolated_settings): + """A tag with path separators must not create nested folders.""" + model_data = {"base_model": "SDXL", "tags": ["a/b:c"]} + + assert calculate_relative_path_for_model(model_data, "lora") == "SDXL/a_b_c" + + +def test_calculate_relative_path_caps_tag_segment(isolated_settings): + """A long configured priority tag is truncated to the tag length budget.""" + long_tag = "y" * 80 + isolated_settings["priority_tags"] = {"lora": long_tag} + + model_data = {"base_model": "SDXL", "tags": [long_tag]} + + relative_path = calculate_relative_path_for_model(model_data, "lora") + + assert relative_path == "SDXL/" + "y" * MAX_PATH_TAG_LENGTH + + +def test_calculate_relative_path_keeps_tag_within_budget(isolated_settings): + model_data = {"base_model": "SDXL", "tags": ["t" * 40]} + + relative_path = calculate_relative_path_for_model(model_data, "lora") + + assert relative_path == "SDXL/" + "t" * 40 + + +def test_calculate_relative_path_caps_model_and_version_names(isolated_settings): + isolated_settings["download_path_templates"]["lora"] = "{model_name}/{version_name}" + + model_data = { + "model_name": "m" * 300, + "base_model": "SDXL", + "tags": [], + "civitai": {"id": 1, "name": "v" * 300, "creator": {"username": "Creator"}}, + } + + relative_path = calculate_relative_path_for_model(model_data, "lora") + + assert relative_path == ( + "m" * MAX_FOLDER_NAME_LENGTH + "/" + "v" * MAX_FOLDER_NAME_LENGTH + ) + + def test_calculate_recipe_fingerprint_filters_and_sorts(): loras = [ {"hash": "ABC", "strength": 0.1234}, @@ -304,6 +378,32 @@ def test_calculate_filename_original_name_falls_back_to_file_name(isolated_setti assert calculate_filename_for_model(model_data, "lora") == "legacy-name-0123456789" +def test_calculate_filename_drops_keyword_dump_tag(isolated_settings): + """The keyword-dump tag collapses instead of filling the filename (#1119).""" + _set_filename_templates(isolated_settings, "{base_model}-{first_tag}") + + model_data = { + "base_model": "Krea 2", + "tags": [KEYWORD_DUMP_TAG], + "file_path": "/models/V1.safetensors", + } + + assert calculate_filename_for_model(model_data, "lora") == "Krea 2" + + +def test_calculate_filename_caps_rendered_stem(isolated_settings): + _set_filename_templates(isolated_settings, "{model_name}") + + model_data = { + "model_name": "m" * 400, + "file_path": "/models/V1.safetensors", + } + + result = calculate_filename_for_model(model_data, "lora") + + assert len(result) == MAX_FILENAME_STEM_LENGTH + + @pytest.mark.parametrize( "original, expected", [ @@ -318,6 +418,27 @@ def test_sanitize_folder_name(original, expected): assert sanitize_folder_name(original) == expected +def test_sanitize_folder_name_without_max_length_is_unbounded(): + assert sanitize_folder_name("x" * 300) == "x" * 300 + + +@pytest.mark.parametrize( + "original, max_length, expected", + [ + ("abcdefghij", 4, "abcd"), + # Re-trim separators and spaces exposed by the cut. + ("abc...defg", 4, "abc"), + ("abcdefg hij", 8, "abcdefg"), + # Shorter than the cap is returned untouched. + ("short", 10, "short"), + # A cut that leaves only separators falls back to "unnamed". + ("...abcdef", 3, "unnamed"), + ], +) +def test_sanitize_folder_name_truncates_to_max_length(original, max_length, expected): + assert sanitize_folder_name(original, max_length=max_length) == expected + + def test_get_lora_info_absolute_bare_name(mock_lora_scanner): mock_lora_scanner([ {"file_name": "mylora", "folder": "SDXL", "file_path": "/models/Lora/SDXL/mylora.safetensors", "civitai": {"trainedWords": ["trigger1"]}},