diff --git a/alembic_db/versions/0006_add_loader_path.py b/alembic_db/versions/0006_add_loader_path.py new file mode 100644 index 000000000..afa65312d --- /dev/null +++ b/alembic_db/versions/0006_add_loader_path.py @@ -0,0 +1,30 @@ +""" +Add loader_path column to asset_references. + +Stores the in-root loader path (path relative to the storage root with the +top-level model category dropped) derived from file_path at scan/ingest time, +so the assets API can return it without re-resolving against every registered +model-folder base on every request. + +Revision ID: 0006_add_loader_path +Revises: 0005_allow_case_sensitive_tags +Create Date: 2026-07-02 +""" + +from alembic import op +import sqlalchemy as sa + +revision = "0006_add_loader_path" +down_revision = "0005_allow_case_sensitive_tags" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + with op.batch_alter_table("asset_references") as batch_op: + batch_op.add_column(sa.Column("loader_path", sa.Text(), nullable=True)) + + +def downgrade() -> None: + with op.batch_alter_table("asset_references") as batch_op: + batch_op.drop_column("loader_path") diff --git a/app/assets/api/routes.py b/app/assets/api/routes.py index 6cc43c498..40dd35933 100644 --- a/app/assets/api/routes.py +++ b/app/assets/api/routes.py @@ -39,7 +39,7 @@ from app.assets.services import ( upload_from_temp_path, ) from app.assets.services.cursor import InvalidCursorError -from app.assets.services.path_utils import compute_asset_response_paths +from app.assets.services.path_utils import compute_display_name from app.assets.services.tagging import list_tag_histogram ROUTES = web.RouteTableDef() @@ -162,16 +162,17 @@ def _build_asset_response(result: schemas.AssetDetailResult | schemas.UploadResu else: preview_url = _build_preview_url_from_view(result.tags, result.ref.user_metadata) if result.ref.file_path: - paths = compute_asset_response_paths(result.ref.file_path) - file_path, display_name = paths if paths else (None, None) + display_name = compute_display_name(result.ref.file_path) + # In-root loader path (model category dropped): what model loaders consume. + loader_path = result.ref.loader_path else: - file_path, display_name = None, None + display_name, loader_path = None, None asset_content_hash = result.asset.hash if result.asset else None return schemas_out.Asset( id=result.ref.id, name=result.ref.name, hash=asset_content_hash, - file_path=file_path, + loader_path=loader_path, display_name=display_name, asset_hash=asset_content_hash, size=int(result.asset.size_bytes) if result.asset else None, diff --git a/app/assets/api/schemas_out.py b/app/assets/api/schemas_out.py index d4d2c699d..da8251499 100644 --- a/app/assets/api/schemas_out.py +++ b/app/assets/api/schemas_out.py @@ -12,16 +12,16 @@ class Asset(BaseModel): name: str = Field( ..., deprecated=True, - description="Reference label, often caller-provided or derived from the filename. Deprecated for storage path/display semantics; use `file_path` and `display_name` when present.", + description="Reference label, often caller-provided or derived from the filename. Deprecated for storage path/display semantics; use `loader_path` and `display_name` when present.", ) hash: str | None = None - file_path: str | None = Field( + loader_path: str | None = Field( default=None, - description="Runtime storage locator for filesystem-backed assets, using Comfy storage namespaces such as `input/`, `output/`, `temp/`, or `models/`. Not an absolute filesystem path, unique identity, or model loader path.", + description="The value a loader consumes to load this asset. `None` when no loader can resolve the file.", ) display_name: str | None = Field( default=None, - description="Human-facing label derived from `file_path`, usually the path below the top-level storage namespace. Not unique.", + description="Human-facing label for the asset. Not unique.", ) asset_hash: str | None = None size: int | None = None diff --git a/app/assets/database/models.py b/app/assets/database/models.py index 9b61d309a..329cd483d 100644 --- a/app/assets/database/models.py +++ b/app/assets/database/models.py @@ -76,6 +76,8 @@ class AssetReference(Base): # Cache state fields (from former AssetCacheState) file_path: Mapped[str | None] = mapped_column(Text, nullable=True) + # In-root loader path derived from file_path at scan/ingest time. + loader_path: Mapped[str | None] = mapped_column(Text, nullable=True) mtime_ns: Mapped[int | None] = mapped_column(BigInteger, nullable=True) needs_verify: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) is_missing: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) diff --git a/app/assets/database/queries/asset_reference.py b/app/assets/database/queries/asset_reference.py index 792411800..967b0e43a 100644 --- a/app/assets/database/queries/asset_reference.py +++ b/app/assets/database/queries/asset_reference.py @@ -650,6 +650,7 @@ def upsert_reference( name: str, mtime_ns: int, owner_id: str = "", + loader_path: str | None = None, ) -> tuple[bool, bool]: """Upsert a reference by file_path. Returns (created, updated). @@ -659,6 +660,7 @@ def upsert_reference( vals = { "asset_id": asset_id, "file_path": file_path, + "loader_path": loader_path, "name": name, "owner_id": owner_id, "mtime_ns": int(mtime_ns), @@ -686,13 +688,14 @@ def upsert_reference( AssetReference.asset_id != asset_id, AssetReference.mtime_ns.is_(None), AssetReference.mtime_ns != int(mtime_ns), + AssetReference.loader_path.is_distinct_from(loader_path), AssetReference.is_missing == True, # noqa: E712 AssetReference.deleted_at.isnot(None), ) ) .values( - asset_id=asset_id, mtime_ns=int(mtime_ns), is_missing=False, - deleted_at=None, updated_at=now, + asset_id=asset_id, mtime_ns=int(mtime_ns), loader_path=loader_path, + is_missing=False, deleted_at=None, updated_at=now, ) ) res2 = session.execute(upd) diff --git a/app/assets/scanner.py b/app/assets/scanner.py index 2c1e97840..42c4c1e9d 100644 --- a/app/assets/scanner.py +++ b/app/assets/scanner.py @@ -36,7 +36,7 @@ from app.assets.services.hashing import HashCheckpoint, compute_blake3_hash from app.assets.services.image_dimensions import extract_image_dimensions from app.assets.services.metadata_extract import extract_file_metadata from app.assets.services.path_utils import ( - compute_relative_filename, + compute_loader_path, get_comfy_models_folders, get_name_and_tags_from_asset_path, ) @@ -63,7 +63,7 @@ RootType = Literal["models", "input", "output"] def get_prefixes_for_root(root: RootType) -> list[str]: if root == "models": bases: list[str] = [] - for _bucket, paths in get_comfy_models_folders(): + for _bucket, paths, _exts in get_comfy_models_folders(): bases.extend(paths) return [os.path.abspath(p) for p in bases] if root == "input": @@ -81,7 +81,7 @@ def get_all_known_prefixes() -> list[str]: def collect_models_files() -> list[str]: out: list[str] = [] - for folder_name, bases in get_comfy_models_folders(): + for folder_name, bases, _exts in get_comfy_models_folders(): rel_files = folder_paths.get_filename_list(folder_name) or [] for rel_path in rel_files: if not all(is_visible(part) for part in Path(rel_path).parts): @@ -308,7 +308,7 @@ def build_asset_specs( if not stat_p.st_size: continue name, tags = get_name_and_tags_from_asset_path(abs_p) - rel_fname = compute_relative_filename(abs_p) + rel_fname = compute_loader_path(abs_p) # Extract metadata (tier 1: filesystem, tier 2: safetensors header) metadata = None @@ -430,7 +430,7 @@ def enrich_asset( return new_level initial_mtime_ns = get_mtime_ns(stat_p) - rel_fname = compute_relative_filename(file_path) + rel_fname = compute_loader_path(file_path) mime_type: str | None = None metadata = None diff --git a/app/assets/services/asset_management.py b/app/assets/services/asset_management.py index d4e4fc61c..a4c8b5a75 100644 --- a/app/assets/services/asset_management.py +++ b/app/assets/services/asset_management.py @@ -38,7 +38,7 @@ from app.assets.database.queries import ( update_reference_updated_at, ) from app.assets.helpers import select_best_live_path -from app.assets.services.path_utils import compute_relative_filename +from app.assets.services.path_utils import compute_loader_path from app.assets.services.schemas import ( AssetData, AssetDetailResult, @@ -91,7 +91,7 @@ def update_asset_metadata( update_reference_name(session, reference_id=reference_id, name=name) touched = True - computed_filename = compute_relative_filename(ref.file_path) if ref.file_path else None + computed_filename = compute_loader_path(ref.file_path) if ref.file_path else None new_meta: dict | None = None if user_metadata is not None: diff --git a/app/assets/services/bulk_ingest.py b/app/assets/services/bulk_ingest.py index 4036530e6..c98658bf1 100644 --- a/app/assets/services/bulk_ingest.py +++ b/app/assets/services/bulk_ingest.py @@ -56,6 +56,7 @@ class ReferenceRow(TypedDict): id: str asset_id: str file_path: str + loader_path: str | None mtime_ns: int owner_id: str name: str @@ -172,6 +173,8 @@ def batch_insert_seed_assets( "id": reference_id, "asset_id": asset_id, "file_path": absolute_path, + # spec["fname"] is compute_loader_path(abs_path) from build_asset_specs. + "loader_path": spec["fname"], "mtime_ns": spec["mtime_ns"], "owner_id": owner_id, "name": spec["info_name"], diff --git a/app/assets/services/ingest.py b/app/assets/services/ingest.py index 61359464c..1ffb3d634 100644 --- a/app/assets/services/ingest.py +++ b/app/assets/services/ingest.py @@ -33,7 +33,7 @@ from app.assets.services.bulk_ingest import batch_insert_seed_assets from app.assets.services.file_utils import get_size_and_mtime_ns from app.assets.services.image_dimensions import extract_image_dimensions from app.assets.services.path_utils import ( - compute_relative_filename, + compute_loader_path, get_name_and_tags_from_asset_path, get_path_derived_tags_from_path, resolve_destination_from_tags, @@ -92,6 +92,7 @@ def _ingest_file_from_path( name=info_name or os.path.basename(locator), mtime_ns=mtime_ns, owner_id=owner_id, + loader_path=compute_loader_path(locator), ) # Get the reference we just created/updated @@ -244,7 +245,7 @@ def ingest_existing_file( "mtime_ns": mtime_ns, "info_name": name, "tags": tags, - "fname": os.path.basename(abs_path), + "fname": compute_loader_path(abs_path), "metadata": None, "hash": None, "mime_type": mime_type, @@ -304,7 +305,7 @@ def _register_existing_asset( return result new_meta = dict(user_metadata) - computed_filename = compute_relative_filename(ref.file_path) if ref.file_path else None + computed_filename = compute_loader_path(ref.file_path) if ref.file_path else None if computed_filename: new_meta["filename"] = computed_filename @@ -351,7 +352,7 @@ def _update_metadata_with_filename( current_metadata: dict | None, user_metadata: dict[str, Any], ) -> None: - computed_filename = compute_relative_filename(file_path) if file_path else None + computed_filename = compute_loader_path(file_path) if file_path else None current_meta = current_metadata or {} new_meta = dict(current_meta) diff --git a/app/assets/services/path_utils.py b/app/assets/services/path_utils.py index 3430f5561..7c27c8878 100644 --- a/app/assets/services/path_utils.py +++ b/app/assets/services/path_utils.py @@ -9,20 +9,23 @@ _NON_MODEL_FOLDER_NAMES = frozenset({"configs", "custom_nodes"}) _KNOWN_SUBFOLDER_TAGS = frozenset({"3d", "pasted", "painter", "threed", "webcam"}) -def get_comfy_models_folders() -> list[tuple[str, list[str]]]: - """Build list of (folder_name, base_paths[]) for all model locations. +def get_comfy_models_folders() -> list[tuple[str, list[str], set[str]]]: + """Build list of (folder_name, base_paths[], extensions) for all model locations. Includes every category registered in folder_names_and_paths, regardless of whether its paths are under the main models_dir, but excludes non-model entries like configs and custom_nodes. + + An empty extensions set means the category accepts any extension, + matching folder_paths.filter_files_extensions semantics. """ - targets: list[tuple[str, list[str]]] = [] + targets: list[tuple[str, list[str], set[str]]] = [] for name, values in folder_paths.folder_names_and_paths.items(): if name in _NON_MODEL_FOLDER_NAMES: continue - paths, _exts = values[0], values[1] + paths, exts = values[0], values[1] if paths: - targets.append((name, paths)) + targets.append((name, paths, set(exts))) return targets @@ -44,7 +47,9 @@ def resolve_destination_from_tags(tags: list[str]) -> tuple[str, list[str]]: folder_name = model_type_tags[0].split(":", 1)[1] if not folder_name: raise ValueError("models uploads require exactly one model_type: tag") - model_folder_paths = dict(get_comfy_models_folders()) + model_folder_paths = { + name: paths for name, paths, _exts in get_comfy_models_folders() + } try: bases = model_folder_paths[folder_name] except KeyError: @@ -79,11 +84,14 @@ def _is_relative_to(child: str, parent: str) -> bool: def compute_asset_response_paths(file_path: str) -> tuple[str, str | None] | None: - """Return public (file_path, display_name) response fields for a file path. + """Return (logical_path, display_name) for a file path. - These fields are storage locators, not model-loader namespaces. Registered - model-folder membership is represented by backend tags such as - ``model_type:``; response paths only use known storage roots. + ``logical_path`` is the internal namespaced storage locator (e.g. + ``models/checkpoints/foo/bar.safetensors``); ``display_name`` is the + human-facing label below that namespace, served on Asset responses. These + are storage locators, not model-loader namespaces. Registered model-folder + membership is represented by backend tags such as + ``model_type:``; these paths only use known storage roots. """ fp_abs = os.path.abspath(file_path) candidates: list[tuple[int, int, str, str]] = [] @@ -117,23 +125,26 @@ def compute_display_name(file_path: str) -> str | None: return result[1] if result else None -def compute_file_path(file_path: str) -> str | None: - """Return the asset's logical storage `file_path`, or None for unknown paths.""" +def compute_logical_path(file_path: str) -> str | None: + """Return the internal namespaced storage locator, or None for unknown paths.""" result = compute_asset_response_paths(file_path) return result[0] if result else None -def compute_relative_filename(file_path: str) -> str | None: +def compute_loader_path(file_path: str) -> str | None: """ - Return the model's path relative to the last well-known folder (the model category), - using forward slashes, eg: + Return the asset's in-root loader path: the path relative to the last + well-known folder (the model category), using forward slashes, eg: /.../models/checkpoints/flux/123/flux.safetensors -> "flux/123/flux.safetensors" /.../models/text_encoders/clip_g.safetensors -> "clip_g.safetensors" - This is legacy metadata/view filename logic, not the public Asset response - `display_name`. Response fields should use compute_asset_response_paths(). + This is the value model loaders consume (the model category is dropped). It + is persisted as ``AssetReference.loader_path`` and served as the public + Asset response `loader_path` field. The human-facing `display_name` comes + from compute_asset_response_paths(). - For non-model paths, returns None. + For input/output/temp paths the full path relative to that root is returned. + For paths outside any known root, returns None. """ try: root_category, rel_path = get_asset_category_and_relative_path(file_path) @@ -198,8 +209,14 @@ def get_asset_category_and_relative_path( return "temp", _compute_relative(fp_abs, temp_base) # 4) models (check deepest matching base to avoid ambiguity) + ext = os.path.splitext(fp_abs)[1].lower() best: tuple[int, str, str] | None = None # (base_len, bucket, rel_inside_bucket) - for bucket, bases in get_comfy_models_folders(): + for bucket, bases, extensions in get_comfy_models_folders(): + # A bucket only lists files within its extension set (empty set + # accepts any extension), so a bucket that cannot load the file + # must not contribute a loader path. + if extensions and ext not in extensions: + continue for b in bases: base_abs = os.path.abspath(b) if not _check_is_within(fp_abs, base_abs): @@ -225,6 +242,12 @@ def get_backend_system_tags_from_path(path: str) -> list[str]: The returned tags are only the backend-generated system tags: ``models``, ``model_type:``, ``input``, ``output``, and ``temp``. Model type tags are based on registered folder names, not path components. + + A ``model_type:`` tag is only emitted when the file's + extension is accepted by that folder's registered extension set, so + categories sharing a base directory tag only the files they can + actually load. Files under a model base whose extension matches no + category still get the ``models`` tag. """ fp_abs = os.path.abspath(path) fp_path = Path(fp_abs) @@ -242,17 +265,23 @@ def get_backend_system_tags_from_path(path: str) -> list[str]: if fp_path.is_relative_to(os.path.abspath(base)): _add(role) + ext = os.path.splitext(fp_abs)[1].lower() model_types: list[str] = [] - for folder_name, bases in get_comfy_models_folders(): + under_models_base = False + for folder_name, bases, extensions in get_comfy_models_folders(): for base in bases: if fp_path.is_relative_to(os.path.abspath(base)): - model_types.append(folder_name) + under_models_base = True + # Empty set accepts any extension, matching + # folder_paths.filter_files_extensions semantics. + if not extensions or ext in extensions: + model_types.append(folder_name) break - if model_types: + if under_models_base: _add("models") - for folder_name in model_types: - _add(f"model_type:{folder_name}") + for folder_name in model_types: + _add(f"model_type:{folder_name}") if not tags: raise ValueError( diff --git a/app/assets/services/schemas.py b/app/assets/services/schemas.py index 4d2af8a02..0fda6871d 100644 --- a/app/assets/services/schemas.py +++ b/app/assets/services/schemas.py @@ -25,6 +25,7 @@ class ReferenceData: preview_id: str | None created_at: datetime updated_at: datetime + loader_path: str | None = None system_metadata: dict[str, Any] | None = None job_id: str | None = None last_access_time: datetime | None = None @@ -93,6 +94,7 @@ def extract_reference_data(ref: AssetReference) -> ReferenceData: id=ref.id, name=ref.name, file_path=ref.file_path, + loader_path=ref.loader_path, user_metadata=ref.user_metadata, preview_id=ref.preview_id, system_metadata=ref.system_metadata, diff --git a/openapi.yaml b/openapi.yaml index 1f1378c19..0cf177815 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -11,12 +11,12 @@ components: description: Blake3 hash of the asset content. pattern: ^blake3:[a-f0-9]{64}$ type: string - file_path: - description: Runtime storage locator for filesystem-backed assets, using Comfy storage namespaces such as `input/`, `output/`, `temp/`, or `models/`. Not an absolute filesystem path, unique identity, or model loader path. + loader_path: + description: The value a loader consumes to load this asset. Null when no loader can resolve the file. nullable: true type: string display_name: - description: Human-facing label derived from `file_path`, usually the path below the top-level storage namespace. Not unique. + description: Human-facing label for the asset. Not unique. nullable: true type: string id: diff --git a/tests-unit/assets_test/queries/test_cache_state.py b/tests-unit/assets_test/queries/test_cache_state.py index ead60e570..49d0e8d4c 100644 --- a/tests-unit/assets_test/queries/test_cache_state.py +++ b/tests-unit/assets_test/queries/test_cache_state.py @@ -176,6 +176,39 @@ class TestUpsertReference: ref = session.query(AssetReference).filter_by(file_path=file_path).one() assert ref.mtime_ns == final_mtime + def test_upsert_refreshes_loader_path_on_existing_reference(self, session: Session): + """Re-ingesting an existing reference writes the loader_path computed + by that ingest, healing NULL or stale values even when nothing else + about the row changed.""" + asset = _make_asset(session, "hash1") + file_path = "/models/checkpoints/sub/model.safetensors" + + upsert_reference( + session, asset_id=asset.id, file_path=file_path, name="model", + mtime_ns=100, loader_path=None, + ) + session.commit() + + created, updated = upsert_reference( + session, asset_id=asset.id, file_path=file_path, name="model", + mtime_ns=100, loader_path="sub/model.safetensors", + ) + session.commit() + + assert created is False + assert updated is True + ref = session.query(AssetReference).filter_by(file_path=file_path).one() + assert ref.loader_path == "sub/model.safetensors" + + # Identical loader_path is a no-op, not a spurious update. + created, updated = upsert_reference( + session, asset_id=asset.id, file_path=file_path, name="model", + mtime_ns=100, loader_path="sub/model.safetensors", + ) + session.commit() + assert created is False + assert updated is False + def test_upsert_restores_missing_reference(self, session: Session): """Upserting a reference that was marked missing should restore it.""" asset = _make_asset(session, "hash1") diff --git a/tests-unit/assets_test/services/test_asset_response_loader_path.py b/tests-unit/assets_test/services/test_asset_response_loader_path.py new file mode 100644 index 000000000..f128a25e1 --- /dev/null +++ b/tests-unit/assets_test/services/test_asset_response_loader_path.py @@ -0,0 +1,83 @@ +"""Tests for how _build_asset_response derives the response `loader_path`. + +Guards the persist-and-read contract: the response reads the stored +`loader_path` verbatim, with no read-time recomputation. Like tags, the +value is a seed-time derivative healed by the scan lifecycle. +""" + +from datetime import datetime +from pathlib import Path +from unittest.mock import patch + +from app.assets.api.routes import _build_asset_response +from app.assets.services.schemas import AssetDetailResult, ReferenceData + +_TS = datetime(2024, 1, 1, 0, 0, 0) + + +def _make_result( + *, file_path: str | None, loader_path: str | None +) -> AssetDetailResult: + ref = ReferenceData( + id="ref-1", + name="model.safetensors", + file_path=file_path, + loader_path=loader_path, + user_metadata=None, + preview_id=None, + created_at=_TS, + updated_at=_TS, + last_access_time=_TS, + ) + return AssetDetailResult(ref=ref, asset=None, tags=[]) + + +def test_uses_persisted_loader_path_without_recomputing(): + """A stored loader_path is returned verbatim, not re-derived from file_path. + + The sentinel value could never be produced by compute_loader_path for this + file_path, so seeing it in the response proves the stored column is read. + """ + result = _make_result( + file_path="/unmatched/root/model.safetensors", + loader_path="SENTINEL/stored.safetensors", + ) + + resp = _build_asset_response(result) + + assert resp.loader_path == "SENTINEL/stored.safetensors" + + +def test_null_stored_loader_path_is_served_as_null(tmp_path: Path): + """No read-time recomputation: a NULL column is served as null even when + the path would resolve.""" + models = tmp_path / "models" + ckpt = models / "checkpoints" + ckpt.mkdir(parents=True) + f = ckpt / "bar.safetensors" + f.touch() + + with patch("app.assets.services.path_utils.folder_paths") as mock_fp, patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[("checkpoints", [str(ckpt)], {".safetensors"})], + ): + mock_fp.get_input_directory.return_value = str(tmp_path / "in") + mock_fp.get_output_directory.return_value = str(tmp_path / "out") + mock_fp.get_temp_directory.return_value = str(tmp_path / "tmp") + mock_fp.models_dir = str(models) + + result = _make_result(file_path=str(f), loader_path=None) + resp = _build_asset_response(result) + + assert resp.loader_path is None + assert resp.display_name == "checkpoints/bar.safetensors" + + +def test_all_path_fields_null_without_file_path(): + """API-created / hash-only references (no file_path) expose no paths.""" + result = _make_result(file_path=None, loader_path=None) + + resp = _build_asset_response(result) + + assert resp.loader_path is None + assert resp.display_name is None diff --git a/tests-unit/assets_test/services/test_bulk_ingest.py b/tests-unit/assets_test/services/test_bulk_ingest.py index 3754ece32..a3889f235 100644 --- a/tests-unit/assets_test/services/test_bulk_ingest.py +++ b/tests-unit/assets_test/services/test_bulk_ingest.py @@ -191,6 +191,8 @@ class TestBatchInsertSeedAssets: refs = session.query(AssetReference).all() assert len(refs) == 1 assert refs[0].file_path == absolute_path + # loader_path is persisted from the spec's fname (compute_loader_path). + assert refs[0].loader_path == "same-file.safetensors" assert set(get_reference_tags(session, reference_id=refs[0].id)) == { "models", "model_type:checkpoints", @@ -215,8 +217,8 @@ class TestBatchInsertSeedAssets: patch( "app.assets.services.path_utils.get_comfy_models_folders", return_value=[ - ("checkpoints", [str(shared_root)]), - ("diffusion_models", [str(shared_root)]), + ("checkpoints", [str(shared_root)], {".safetensors"}), + ("diffusion_models", [str(shared_root)], {".safetensors"}), ], ), ): @@ -251,6 +253,36 @@ class TestBatchInsertSeedAssets: "model_type:diffusion_models", } + def test_loader_path_persisted_as_null_when_fname_is_none( + self, session: Session, temp_dir: Path + ): + """A file with no in-root loader path (fname=None, e.g. an orphan under + models_root) persists loader_path as NULL rather than a synthesized value.""" + file_path = temp_dir / "orphan.bin" + file_path.write_bytes(b"x") + + specs: list[SeedAssetSpec] = [ + { + "abs_path": str(file_path), + "size_bytes": 1, + "mtime_ns": 1234567890000000000, + "info_name": "orphan.bin", + "tags": [], + "fname": None, + "metadata": None, + "hash": None, + "mime_type": None, + } + ] + + result = batch_insert_seed_assets(session, specs=specs, owner_id="") + + assert result.inserted_refs == 1 + refs = session.query(AssetReference).all() + assert len(refs) == 1 + assert refs[0].file_path == str(file_path) + assert refs[0].loader_path is None + class TestMetadataExtraction: def test_extracts_mime_type_for_model_files(self, temp_dir: Path): diff --git a/tests-unit/assets_test/services/test_ingest.py b/tests-unit/assets_test/services/test_ingest.py index 93e2469fe..7fa882df0 100644 --- a/tests-unit/assets_test/services/test_ingest.py +++ b/tests-unit/assets_test/services/test_ingest.py @@ -329,6 +329,45 @@ class TestIngestExistingFileTagFK: assert "output" in ref_tag_names +class TestIngestExistingFileLoaderPath: + """Outputs saved into a subfolder must persist the subfolder-qualified + loader path, not the bare basename (regression: spec["fname"] was + os.path.basename).""" + + def test_subfoldered_output_persists_relative_loader_path( + self, mock_create_session, temp_dir: Path, session: Session + ): + input_dir = temp_dir / "input" + output_dir = temp_dir / "output" + temp_root = temp_dir / "temp" + for directory in (input_dir, output_dir, temp_root): + directory.mkdir() + file_path = output_dir / "sub" / "img_00001_.png" + file_path.parent.mkdir() + file_path.write_bytes(b"image data") + + with ( + patch("app.assets.services.path_utils.folder_paths") as mock_fp, + patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[], + ), + ): + mock_fp.get_input_directory.return_value = str(input_dir) + mock_fp.get_output_directory.return_value = str(output_dir) + mock_fp.get_temp_directory.return_value = str(temp_root) + + assert ingest_existing_file(abs_path=str(file_path)) is True + + ref = ( + session.query(AssetReference) + .filter_by(file_path=str(file_path)) + .one() + ) + assert ref.loader_path == "sub/img_00001_.png" + assert (ref.user_metadata or {}).get("filename") == "sub/img_00001_.png" + + class TestIngestImageDimensions: """system_metadata should carry {kind, width, height} for image assets.""" diff --git a/tests-unit/assets_test/services/test_path_utils.py b/tests-unit/assets_test/services/test_path_utils.py index af08afa30..ddf23c676 100644 --- a/tests-unit/assets_test/services/test_path_utils.py +++ b/tests-unit/assets_test/services/test_path_utils.py @@ -8,7 +8,8 @@ import pytest from app.assets.services.path_utils import ( compute_display_name, - compute_file_path, + compute_loader_path, + compute_logical_path, get_asset_category_and_relative_path, get_known_input_subfolder_tags_from_path, get_known_subfolder_tags, @@ -38,7 +39,7 @@ def fake_dirs(): with patch( "app.assets.services.path_utils.get_comfy_models_folders", - return_value=[("checkpoints", [str(models_dir)])], + return_value=[("checkpoints", [str(models_dir)], {".safetensors"})], ): yield { "input": input_dir, @@ -107,7 +108,7 @@ class TestGetAssetCategoryAndRelativePath: with patch( "app.assets.services.path_utils.get_comfy_models_folders", - return_value=[("LLM", [str(llm_dir)])], + return_value=[("LLM", [str(llm_dir)], {".safetensors"})], ): _name, tags = get_name_and_tags_from_asset_path(str(f)) @@ -136,8 +137,8 @@ class TestGetAssetCategoryAndRelativePath: with patch( "app.assets.services.path_utils.get_comfy_models_folders", return_value=[ - ("checkpoints", [str(shared_root)]), - ("loras", [str(shared_root)]), + ("checkpoints", [str(shared_root)], {".safetensors"}), + ("loras", [str(shared_root)], {".safetensors"}), ], ): _name, tags = get_name_and_tags_from_asset_path(str(f)) @@ -146,6 +147,55 @@ class TestGetAssetCategoryAndRelativePath: assert "model_type:checkpoints" in tags assert "model_type:loras" in tags + def test_shared_root_model_type_tags_respect_bucket_extensions(self, fake_dirs): + """Buckets sharing a base dir only tag files matching their extensions.""" + shared_root = fake_dirs["models"].parent / "unet" + shared_root.mkdir() + safetensors_file = shared_root / "wan.safetensors" + gguf_file = shared_root / "wan.gguf" + safetensors_file.touch() + gguf_file.touch() + + with patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[ + ("diffusion_models", [str(shared_root)], {".safetensors"}), + ("unet_gguf", [str(shared_root)], {".gguf"}), + ], + ): + _name, safetensors_tags = get_name_and_tags_from_asset_path(str(safetensors_file)) + _name, gguf_tags = get_name_and_tags_from_asset_path(str(gguf_file)) + + assert "model_type:diffusion_models" in safetensors_tags + assert "model_type:unet_gguf" not in safetensors_tags + assert "model_type:unet_gguf" in gguf_tags + assert "model_type:diffusion_models" not in gguf_tags + + def test_empty_extension_set_tags_any_extension(self, fake_dirs): + """Custom buckets registered without extensions accept every file.""" + custom_root = fake_dirs["models"].parent / "custom_bucket" + custom_root.mkdir() + f = custom_root / "weights.bin" + f.touch() + + with patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[("custom_bucket", [str(custom_root)], set())], + ): + _name, tags = get_name_and_tags_from_asset_path(str(f)) + + assert "models" in tags + assert "model_type:custom_bucket" in tags + + def test_no_extension_match_keeps_models_tag_without_model_type(self, fake_dirs): + f = fake_dirs["models"] / "notes.txt" + f.touch() + + _name, tags = get_name_and_tags_from_asset_path(str(f)) + + assert "models" in tags + assert not any(tag.startswith("model_type:") for tag in tags) + def test_output_backed_registered_folder_gets_model_and_output_tags(self, fake_dirs): output_checkpoints_dir = fake_dirs["output"] / "checkpoints" output_checkpoints_dir.mkdir() @@ -154,7 +204,7 @@ class TestGetAssetCategoryAndRelativePath: with patch( "app.assets.services.path_utils.get_comfy_models_folders", - return_value=[("checkpoints", [str(output_checkpoints_dir)])], + return_value=[("checkpoints", [str(output_checkpoints_dir)], {".safetensors"})], ): _name, tags = get_name_and_tags_from_asset_path(str(f)) @@ -209,7 +259,7 @@ class TestResponseStoragePaths: f = sub / "image.png" f.touch() - assert compute_file_path(str(f)) == "input/some/folder/image.png" + assert compute_logical_path(str(f)) == "input/some/folder/image.png" assert compute_display_name(str(f)) == "some/folder/image.png" def test_output_file_path_and_display_name_include_subfolder(self, fake_dirs): @@ -218,18 +268,18 @@ class TestResponseStoragePaths: f = sub / "ComfyUI_00001_.png" f.touch() - assert compute_file_path(str(f)) == "output/renders/ComfyUI_00001_.png" + assert compute_logical_path(str(f)) == "output/renders/ComfyUI_00001_.png" assert compute_display_name(str(f)) == "renders/ComfyUI_00001_.png" def test_temp_file_path_and_display_name(self, fake_dirs): f = fake_dirs["temp"] / "preview.png" f.touch() - assert compute_file_path(str(f)) == "temp/preview.png" + assert compute_logical_path(str(f)) == "temp/preview.png" assert compute_display_name(str(f)) == "preview.png" def test_exact_storage_root_has_no_display_name(self, fake_dirs): - assert compute_file_path(str(fake_dirs["input"])) == "input" + assert compute_logical_path(str(fake_dirs["input"])) == "input" assert compute_display_name(str(fake_dirs["input"])) is None def test_longest_matching_builtin_root_wins(self, fake_dirs, tmp_path: Path): @@ -244,7 +294,7 @@ class TestResponseStoragePaths: mock_fp.get_temp_directory.return_value = str(tmp_path / "temp") mock_fp.models_dir = str(fake_dirs["models_root"]) - assert compute_file_path(str(f)) == "output/image.png" + assert compute_logical_path(str(f)) == "output/image.png" assert compute_display_name(str(f)) == "image.png" def test_model_file_path_is_relative_to_physical_models_root(self, fake_dirs): @@ -253,7 +303,7 @@ class TestResponseStoragePaths: f = sub / "model.safetensors" f.touch() - assert compute_file_path(str(f)) == "models/checkpoints/flux/model.safetensors" + assert compute_logical_path(str(f)) == "models/checkpoints/flux/model.safetensors" assert compute_display_name(str(f)) == "checkpoints/flux/model.safetensors" name, tags = get_name_and_tags_from_asset_path(str(f)) @@ -277,9 +327,11 @@ class TestResponseStoragePaths: with patch( "app.assets.services.path_utils.get_comfy_models_folders", - return_value=[(folder_name, [str(default_model_dir), str(output_model_dir)])], + return_value=[ + (folder_name, [str(default_model_dir), str(output_model_dir)], {".safetensors"}) + ], ): - assert compute_file_path(str(f)) == f"output/{folder_name}/saved.safetensors" + assert compute_logical_path(str(f)) == f"output/{folder_name}/saved.safetensors" assert compute_display_name(str(f)) == f"{folder_name}/saved.safetensors" name, tags = get_name_and_tags_from_asset_path(str(f)) @@ -299,10 +351,10 @@ class TestResponseStoragePaths: with patch( "app.assets.services.path_utils.get_comfy_models_folders", - return_value=[(folder_name, [str(output_model_dir)])], + return_value=[(folder_name, [str(output_model_dir)], {".safetensors"})], ): assert ( - compute_file_path(str(f)) + compute_logical_path(str(f)) == "output/loras/experiments/my_lora.safetensors" ) assert compute_display_name(str(f)) == "loras/experiments/my_lora.safetensors" @@ -323,9 +375,9 @@ class TestResponseStoragePaths: with patch( "app.assets.services.path_utils.get_comfy_models_folders", - return_value=[("checkpoints", [str(external_checkpoints_dir)])], + return_value=[("checkpoints", [str(external_checkpoints_dir)], {".safetensors"})], ): - assert compute_file_path(str(f)) is None + assert compute_logical_path(str(f)) is None assert compute_display_name(str(f)) is None name, tags = get_name_and_tags_from_asset_path(str(f)) @@ -347,10 +399,10 @@ class TestResponseStoragePaths: with patch( "app.assets.services.path_utils.get_comfy_models_folders", - return_value=[("checkpoints", [str(foo_dir), str(bar_dir)])], + return_value=[("checkpoints", [str(foo_dir), str(bar_dir)], {".safetensors"})], ): - assert compute_file_path(str(foo_file)) is None - assert compute_file_path(str(bar_file)) is None + assert compute_logical_path(str(foo_file)) is None + assert compute_logical_path(str(bar_file)) is None assert compute_display_name(str(foo_file)) is None assert compute_display_name(str(bar_file)) is None @@ -362,9 +414,9 @@ class TestResponseStoragePaths: with patch( "app.assets.services.path_utils.get_comfy_models_folders", - return_value=[("text_encoders", [str(output_clip_dir)])], + return_value=[("text_encoders", [str(output_clip_dir)], {".safetensors"})], ): - assert compute_file_path(str(f)) == "output/clip/clip_l.safetensors" + assert compute_logical_path(str(f)) == "output/clip/clip_l.safetensors" assert compute_display_name(str(f)) == "clip/clip_l.safetensors" name, tags = get_name_and_tags_from_asset_path(str(f)) @@ -384,9 +436,11 @@ class TestResponseStoragePaths: with patch( "app.assets.services.path_utils.get_comfy_models_folders", - return_value=[("diffusion_models", [str(unet_dir), str(diffusion_models_dir)])], + return_value=[ + ("diffusion_models", [str(unet_dir), str(diffusion_models_dir)], {".safetensors"}) + ], ): - assert compute_file_path(str(f)) == "models/unet/wan.safetensors" + assert compute_logical_path(str(f)) == "models/unet/wan.safetensors" assert compute_display_name(str(f)) == "unet/wan.safetensors" name, tags = get_name_and_tags_from_asset_path(str(f)) @@ -400,7 +454,7 @@ class TestResponseStoragePaths: f.parent.mkdir() f.touch() - assert compute_file_path(str(f)) == "models/not_registered/orphan.bin" + assert compute_logical_path(str(f)) == "models/not_registered/orphan.bin" assert compute_display_name(str(f)) == "not_registered/orphan.bin" def test_output_checkpoint_folder_without_registration_has_only_output_tag(self, fake_dirs): @@ -412,7 +466,7 @@ class TestResponseStoragePaths: "app.assets.services.path_utils.get_comfy_models_folders", return_value=[], ): - assert compute_file_path(str(f)) == "output/checkpoints/saved.safetensors" + assert compute_logical_path(str(f)) == "output/checkpoints/saved.safetensors" assert compute_display_name(str(f)) == "checkpoints/saved.safetensors" name, tags = get_name_and_tags_from_asset_path(str(f)) @@ -422,10 +476,119 @@ class TestResponseStoragePaths: assert not any(tag.startswith("model_type:") for tag in tags) def test_unknown_path_returns_none(self): - assert compute_file_path("/some/random/path.png") is None + assert compute_logical_path("/some/random/path.png") is None assert compute_display_name("/some/random/path.png") is None +class TestLoaderPath: + """In-root loader path: relative to the storage root, model category dropped.""" + + def test_model_loader_path_drops_category(self, fake_dirs): + sub = fake_dirs["models"] / "flux" + sub.mkdir() + f = sub / "model.safetensors" + f.touch() + + # logical_path keeps the category, file_path (loader) drops it + assert compute_logical_path(str(f)) == "models/checkpoints/flux/model.safetensors" + assert compute_loader_path(str(f)) == "flux/model.safetensors" + + def test_model_loader_path_flat_file(self, fake_dirs): + f = fake_dirs["models"] / "model.safetensors" + f.touch() + + assert compute_loader_path(str(f)) == "model.safetensors" + + def test_input_loader_path_keeps_subfolders(self, fake_dirs): + sub = fake_dirs["input"] / "some" / "folder" + sub.mkdir(parents=True) + f = sub / "image.png" + f.touch() + + assert compute_loader_path(str(f)) == "some/folder/image.png" + + def test_temp_loader_path(self, fake_dirs): + f = fake_dirs["temp"] / "preview.png" + f.touch() + + assert compute_loader_path(str(f)) == "preview.png" + + def test_unregistered_file_under_models_root_has_no_loader_path(self, fake_dirs): + # Under models_root but not within any registered category base. + f = fake_dirs["models_root"] / "not_registered" / "orphan.bin" + f.parent.mkdir() + f.touch() + + # It still has a namespaced logical_path, but no loader path. + assert compute_logical_path(str(f)) == "models/not_registered/orphan.bin" + assert compute_loader_path(str(f)) is None + + def test_extension_mismatch_in_registered_bucket_has_no_loader_path(self, fake_dirs): + # Inside a registered bucket, but the bucket's extension set cannot + # load it: no model_type tag, and no loader path either. + f = fake_dirs["models"] / "notes.txt" + f.touch() + + assert compute_logical_path(str(f)) == "models/checkpoints/notes.txt" + assert compute_loader_path(str(f)) is None + + def test_shared_base_loader_path_uses_extension_matching_bucket(self, fake_dirs): + shared_root = fake_dirs["models"].parent / "unet" + shared_root.mkdir() + f = shared_root / "wan.gguf" + f.touch() + + with patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[ + ("diffusion_models", [str(shared_root)], {".safetensors"}), + ("unet_gguf", [str(shared_root)], {".gguf"}), + ], + ): + assert compute_loader_path(str(f)) == "wan.gguf" + + def test_match_all_bucket_provides_loader_path_for_any_extension(self, fake_dirs): + custom_root = fake_dirs["models"].parent / "custom_bucket" + custom_root.mkdir() + f = custom_root / "weights.bin" + f.touch() + + with patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[("custom_bucket", [str(custom_root)], set())], + ): + assert compute_loader_path(str(f)) == "weights.bin" + + def test_extra_path_model_has_loader_path_but_no_logical_path(self, tmp_path: Path): + """Registered category base outside models_dir (extra_model_paths style). + + Loadable, so loader_path resolves; but it is not under any canonical + storage root, so logical_path/display_name are None. This asymmetry is + intentional: loader_path resolves every registered model-folder base, + logical_path only resolves the canonical storage roots. + """ + extra = tmp_path / "extra_ckpts" + extra.mkdir() + f = extra / "foo.safetensors" + f.touch() + + with patch("app.assets.services.path_utils.folder_paths") as mock_fp, patch( + "app.assets.services.path_utils.get_comfy_models_folders", + return_value=[("checkpoints", [str(extra)], {".safetensors"})], + ): + mock_fp.get_input_directory.return_value = str(tmp_path / "in") + mock_fp.get_output_directory.return_value = str(tmp_path / "out") + mock_fp.get_temp_directory.return_value = str(tmp_path / "tmp") + mock_fp.models_dir = str(tmp_path / "models") # extra is NOT under this + + assert compute_loader_path(str(f)) == "foo.safetensors" + assert compute_logical_path(str(f)) is None + assert compute_display_name(str(f)) is None + + def test_unknown_path_returns_none(self): + assert compute_loader_path("/some/random/path.png") is None + + class TestResolveDestinationFromTags: def test_extra_tags_are_not_path_components(self, fake_dirs): base_dir, subdirs = resolve_destination_from_tags(["input", "unit-tests", "foo"]) diff --git a/tests-unit/assets_test/test_uploads.py b/tests-unit/assets_test/test_uploads.py index 09262b25e..7be7b0935 100644 --- a/tests-unit/assets_test/test_uploads.py +++ b/tests-unit/assets_test/test_uploads.py @@ -55,7 +55,7 @@ def test_upload_ok_duplicate_reference(http: requests.Session, api_base: str, ma assert a2["asset_hash"] == a1["asset_hash"] assert a2["hash"] == a1["hash"] assert a2["id"] != a1["id"] # new reference with same content - assert a2.get("file_path") is None + assert a2.get("loader_path") is None assert a2.get("display_name") is None # Third upload with the same data but different name also creates new AssetReference @@ -67,7 +67,7 @@ def test_upload_ok_duplicate_reference(http: requests.Session, api_base: str, ma assert a3["asset_hash"] == a1["asset_hash"] assert a3["id"] != a1["id"] assert a3["id"] != a2["id"] - assert a3.get("file_path") is None + assert a3.get("loader_path") is None assert a3.get("display_name") is None @@ -102,17 +102,17 @@ def test_upload_fastpath_from_existing_hash_no_file(http: requests.Session, api_ assert "checkpoints" in b2["tags"] assert "uploaded" not in b2["tags"] assert not any(tag.startswith("model_type:") for tag in b2["tags"]) - assert b2.get("file_path") is None + assert b2.get("loader_path") is None assert b2.get("display_name") is None rg = http.get(f"{api_base}/api/assets/{b2['id']}", timeout=120) detail = rg.json() assert rg.status_code == 200, detail - assert detail.get("file_path") is None + assert detail.get("loader_path") is None assert detail.get("display_name") is None -def test_create_from_hash_with_model_tags_does_not_synthesize_file_path( +def test_create_from_hash_with_model_tags_does_not_synthesize_loader_path( http: requests.Session, api_base: str ): seed_name = "from_hash_seed.safetensors" @@ -137,13 +137,13 @@ def test_create_from_hash_with_model_tags_does_not_synthesize_file_path( assert created_r.status_code == 201, created assert created["created_new"] is False assert created["asset_hash"] == seed["asset_hash"] - assert created.get("file_path") is None + assert created.get("loader_path") is None assert created.get("display_name") is None detail_r = http.get(f"{api_base}/api/assets/{created['id']}", timeout=120) detail = detail_r.json() assert detail_r.status_code == 200, detail - assert detail.get("file_path") is None + assert detail.get("loader_path") is None assert detail.get("display_name") is None @@ -204,7 +204,7 @@ def test_duplicate_byte_upload_is_reference_only_and_does_not_need_destination( assert "not-a-destination" in duplicate["tags"] assert "uploaded" not in duplicate["tags"] assert "input" not in duplicate["tags"] - assert duplicate.get("file_path") is None + assert duplicate.get("loader_path") is None assert duplicate.get("display_name") is None @@ -233,23 +233,20 @@ def test_upload_multiple_tags_fields_are_merged(http: requests.Session, api_base ( "tags", "extension", - "expected_prefix", "expected_display_prefix", ), [ - (["input", "unit-tests"], ".png", "input", ""), + (["input", "unit-tests"], ".png", ""), ( ["models", "model_type:checkpoints", "unit-tests"], ".safetensors", - "models/checkpoints", "checkpoints/", ), ], ) -def test_upload_response_includes_file_path_and_display_name( +def test_upload_response_includes_loader_path_and_display_name( tags: list[str], extension: str, - expected_prefix: str, expected_display_prefix: str, http: requests.Session, api_base: str, @@ -270,16 +267,18 @@ def test_upload_response_includes_file_path_and_display_name( assert created_r.status_code in (200, 201), created stored_filename = get_asset_filename(created["asset_hash"], extension) expected_suffix = stored_filename - expected_file_path = f"{expected_prefix}/{expected_suffix}" expected_display_name = f"{expected_display_prefix}{expected_suffix}" + # In-root loader path: model category dropped, no subfolders here -> just the filename. + expected_loader_path = expected_suffix - assert created["file_path"] == expected_file_path + assert created["loader_path"] == expected_loader_path assert created["display_name"] == expected_display_name + assert "logical_path" not in created detail_r = http.get(f"{api_base}/api/assets/{created['id']}", timeout=120) detail = detail_r.json() assert detail_r.status_code == 200, detail - assert detail["file_path"] == expected_file_path + assert detail["loader_path"] == expected_loader_path assert detail["display_name"] == expected_display_name list_r = http.get( @@ -290,7 +289,7 @@ def test_upload_response_includes_file_path_and_display_name( listed = list_r.json() assert list_r.status_code == 200, listed match = next(a for a in listed["assets"] if a["id"] == created["id"]) - assert match["file_path"] == expected_file_path + assert match["loader_path"] == expected_loader_path assert match["display_name"] == expected_display_name