mirror of
https://github.com/comfyanonymous/ComfyUI.git
synced 2026-07-03 13:19:23 +08:00
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.
This commit is contained in:
Simon Pinfold
parent
06b2d4c1d0
commit
e807d60e45
@ -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,
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -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"])
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user