From e807d60e45aee8d370b756d6ba523501c6bb0495 Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Thu, 2 Jul 2026 07:38:22 +1200 Subject: [PATCH] feat(assets): add in-root loader file_path, rename storage locator to logical_path Split the Asset response path fields so model-loader consumers get a category-relative path. The namespaced storage locator moves to `logical_path`; the new `file_path` is the in-root loader path (model category dropped), e.g. models/checkpoints/foo/bar.safetensors -> foo/bar.safetensors. --- app/assets/api/routes.py | 14 +++- app/assets/api/schemas_out.py | 10 ++- app/assets/scanner.py | 6 +- app/assets/services/asset_management.py | 4 +- app/assets/services/ingest.py | 6 +- app/assets/services/path_utils.py | 19 +++-- openapi.yaml | 8 +- .../assets_test/services/test_path_utils.py | 82 +++++++++++++++---- tests-unit/assets_test/test_uploads.py | 7 +- 9 files changed, 113 insertions(+), 43 deletions(-) diff --git a/app/assets/api/routes.py b/app/assets/api/routes.py index eca2f887f..c0a5228d8 100644 --- a/app/assets/api/routes.py +++ b/app/assets/api/routes.py @@ -39,7 +39,10 @@ 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_asset_response_paths, + compute_loader_path, +) from app.assets.services.tagging import list_tag_histogram ROUTES = web.RouteTableDef() @@ -163,15 +166,18 @@ def _build_asset_response(result: schemas.AssetDetailResult | schemas.UploadResu 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) + logical_path, display_name = paths if paths else (None, None) + # In-root loader path (model category dropped): what model loaders consume. + loader_path = compute_loader_path(result.ref.file_path) else: - file_path, display_name = None, None + logical_path, display_name, loader_path = None, 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, + file_path=loader_path, + logical_path=logical_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..7143ce77a 100644 --- a/app/assets/api/schemas_out.py +++ b/app/assets/api/schemas_out.py @@ -12,16 +12,20 @@ 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 `file_path`, `logical_path`, and `display_name` when present.", ) hash: str | None = None file_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="In-root loader path for filesystem-backed assets: the path relative to its storage root with the top-level model category dropped (e.g. `models/checkpoints/foo/bar.safetensors` -> `foo/bar.safetensors`). This is the value model loaders consume. `None` when the file is not within a recognized root or model category.", + ) + logical_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/` (e.g. `models/checkpoints/foo/bar.safetensors`). Not an absolute filesystem path, unique identity, or model loader path.", ) 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 derived from `logical_path`, usually the path below the top-level storage namespace. Not unique.", ) asset_hash: str | None = None size: int | None = None diff --git a/app/assets/scanner.py b/app/assets/scanner.py index b13276b49..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, ) @@ -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/ingest.py b/app/assets/services/ingest.py index 61359464c..e6cc42fe9 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, @@ -304,7 +304,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 +351,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 16277d90f..bb497c454 100644 --- a/app/assets/services/path_utils.py +++ b/app/assets/services/path_utils.py @@ -122,23 +122,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 asset's namespaced storage `logical_path`, 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 + backs the public Asset response `file_path` field and the internal + ``computed_filename`` metadata. The namespaced storage locator (`logical_path`) + and human-facing `display_name` come 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) diff --git a/openapi.yaml b/openapi.yaml index 1f1378c19..094b2277d 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -12,11 +12,15 @@ components: 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. + description: 'In-root loader path for filesystem-backed assets: the path relative to its storage root with the top-level model category dropped (e.g. `models/checkpoints/foo/bar.safetensors` -> `foo/bar.safetensors`). This is the value model loaders consume. `None` when the file is not within a recognized root or model category.' + nullable: true + type: string + logical_path: + description: Runtime storage locator for filesystem-backed assets, using Comfy storage namespaces such as `input/`, `output/`, `temp/`, or `models/` (e.g. `models/checkpoints/foo/bar.safetensors`). Not an absolute filesystem path, unique identity, or model loader path. 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 derived from `logical_path`, usually the path below the top-level storage namespace. Not unique. nullable: true type: string id: diff --git a/tests-unit/assets_test/services/test_path_utils.py b/tests-unit/assets_test/services/test_path_utils.py index 3c4db067c..a14487193 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, @@ -258,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): @@ -267,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): @@ -293,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): @@ -302,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)) @@ -330,7 +331,7 @@ class TestResponseStoragePaths: (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)) @@ -353,7 +354,7 @@ class TestResponseStoragePaths: 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" @@ -376,7 +377,7 @@ class TestResponseStoragePaths: "app.assets.services.path_utils.get_comfy_models_folders", 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)) @@ -400,8 +401,8 @@ class TestResponseStoragePaths: "app.assets.services.path_utils.get_comfy_models_folders", 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 @@ -415,7 +416,7 @@ class TestResponseStoragePaths: "app.assets.services.path_utils.get_comfy_models_folders", 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)) @@ -439,7 +440,7 @@ class TestResponseStoragePaths: ("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)) @@ -453,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): @@ -465,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)) @@ -475,10 +476,57 @@ 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_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..d428e74ca 100644 --- a/tests-unit/assets_test/test_uploads.py +++ b/tests-unit/assets_test/test_uploads.py @@ -270,16 +270,20 @@ 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_logical_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_file_path = expected_suffix assert created["file_path"] == expected_file_path + assert created["logical_path"] == expected_logical_path assert created["display_name"] == expected_display_name 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["logical_path"] == expected_logical_path assert detail["display_name"] == expected_display_name list_r = http.get( @@ -291,6 +295,7 @@ def test_upload_response_includes_file_path_and_display_name( 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["logical_path"] == expected_logical_path assert match["display_name"] == expected_display_name