fix(assets): move model asset on model_type: edit to stay loader-coherent

app/assets/api/routes.py

@@ -26,6 +26,7 @@
 from app.assets.services import (
     DependencyMissingError,
     HashMismatchError,
+    ModelMoveError,
     apply_tags,
     asset_exists,
     create_from_hash,
@@ -623,6 +624,11 @@ async def add_asset_tags(request: web.Request) -> web.Response:
             already_present=result.already_present,
             total_tags=result.total_tags,
         )
+    except ModelMoveError as me:
+        # A model_type: edit that couldn't be applied coherently (unknown
+        # folder, or destination collision). The FE re-adds the prior
+        # model_type: tag on a non-2xx, so the asset stays coherent.
+        return _build_error_response(me.status, me.code, me.message, {"id": reference_id})
     except PermissionError as pe:
         return _build_error_response(403, "FORBIDDEN", str(pe), {"id": reference_id})
     except ValueError as ve:

app/assets/services/__init__.py

@@ -22,9 +22,11 @@
 from app.assets.services.ingest import (
     DependencyMissingError,
     HashMismatchError,
+    ModelMoveError,
     create_from_hash,
     ingest_existing_file,
     register_output_files,
+    relocate_model_asset_for_model_type_tags,
     upload_from_temp_path,
 )
 from app.assets.database.queries import (
@@ -62,8 +64,10 @@
     "HashMismatchError",
     "IngestResult",
     "ListAssetsResult",
+    "ModelMoveError",
     "RegisterAssetResult",
     "RemoveTagsResult",
+    "relocate_model_asset_for_model_type_tags",
     "TagUsage",
     "UploadResult",
     "UserMetadata",

app/assets/services/ingest.py

@@ -2,11 +2,13 @@
 import logging
 import mimetypes
 import os
+import shutil
 from typing import Any, Sequence
 
 from sqlalchemy.orm import Session
 
 import app.assets.services.hashing as hashing
+from app.assets.database.models import AssetReference
 from app.assets.database.queries import (
     add_tags_to_reference,
     count_active_siblings,
@@ -20,6 +22,7 @@
     list_references_by_asset_id,
     reference_exists,
     remove_missing_tag_for_asset_id,
+    remove_tags_from_reference,
     set_reference_metadata,
     set_reference_system_metadata,
     set_reference_tags,
@@ -35,7 +38,9 @@
 from app.assets.services.path_utils import (
     compute_relative_filename,
     get_backend_system_tags_from_path,
+    get_model_base_for_folder,
     get_name_and_tags_from_asset_path,
+    model_folders_for_path,
     resolve_destination_from_tags,
     validate_path_within_base,
 )
@@ -453,6 +458,187 @@ def __init__(self, message: str):
         super().__init__(message)
 
 
+class ModelMoveError(Exception):
+    """A model_type: edit could not be applied coherently (BE-1641).
+
+    Carries an HTTP-ish ``status``/``code`` so the route can surface a precise
+    4xx (rather than the generic 404 the bare ValueError path produces). The FE
+    edit-type flow compensates on any non-2xx by re-adding the prior
+    ``model_type:`` tag, so a reject here leaves the asset coherent.
+    """
+
+    def __init__(self, code: str, message: str, status: int = 409):
+        self.code = code
+        self.message = message
+        self.status = status
+        super().__init__(message)
+
+
+def _move_file(src: str, dst: str) -> None:
+    """Relocate a file, falling back to a cross-device copy+unlink.
+
+    ``os.replace`` is atomic but fails with ``EXDEV`` when src and dst live on
+    different filesystems (``extra_model_paths`` may point at another mount), so
+    fall back to ``shutil.move`` there.
+    """
+    try:
+        os.replace(src, dst)
+    except OSError:
+        shutil.move(src, dst)
+
+
+def relocate_model_asset_for_model_type_tags(
+    session: Session,
+    ref: AssetReference,
+    requested_tags: Sequence[str],
+    origin: str = "automatic",
+) -> bool:
+    """Move a filesystem-backed model asset to match an added ``model_type:`` tag.
+
+    BE-1641 / spec-drift §2: under the ``supports_model_type_tags`` contract a
+    ``model_type:`` edit must stay coherent on *edit*, not just upload. When a
+    ``model_type:<folder>`` tag is applied to a filesystem-backed model asset
+    whose file is not already under that folder, move the file to the folder's
+    base and re-derive the path-based system tags so location and label agree.
+
+    Mutates ``ref`` and its tags in-place within ``session`` (the caller owns
+    the commit). Returns True if a physical move happened, False otherwise
+    (non-filesystem / hash-only / non-model asset, no ``model_type:`` added, or
+    the target folder already covers the current path — the shared-dir case in
+    spec-drift §1).
+
+    Raises:
+        ModelMoveError: the target folder is unknown, or the destination is
+            already occupied (collision) — never clobbers (Q2).
+    """
+    if not ref.file_path:
+        # API-created / hash-only reference: nothing on disk to move. Labels
+        # stay labels (matches AC scope: "filesystem-backed model asset").
+        return False
+
+    requested_folders = [
+        t.split(":", 1)[1]
+        for t in normalize_tags(list(requested_tags))
+        if t.startswith("model_type:") and t.split(":", 1)[1]
+    ]
+    if not requested_folders:
+        return False
+
+    old_path = os.path.abspath(ref.file_path)
+    current_folders = set(model_folders_for_path(old_path))
+    if not current_folders:
+        # Not under any model base (e.g. an input/output asset). A model_type:
+        # label here is meaningless for placement; leave it as a plain label.
+        return False
+
+    # The FE emits exactly one model_type: per edit; if several are requested,
+    # the last one wins deterministically.
+    target_folder = requested_folders[-1]
+
+    # Shared on-disk dir (spec-drift §1): the path already covers the target
+    # folder, so re-deriving would keep both twins — no physical move needed.
+    if target_folder in current_folders:
+        return False
+
+    # On a model asset, model_type: is an operational placement tag (it decides
+    # where the file lives), not a free-form label — exactly as it is for a
+    # new-byte upload (resolve_destination_from_tags). An edit IS a placement,
+    # so an unregistered folder_name is an invalid placement target and is
+    # rejected on both paths. A genuine edit-type action always targets a
+    # registered folder_name from the discovery vocabulary, so this only fires
+    # on junk manual model_type: adds.
+    try:
+        new_base = get_model_base_for_folder(target_folder)
+    except ValueError as e:
+        raise ModelMoveError("UNKNOWN_MODEL_TYPE", str(e), status=400)
+
+    rel = compute_relative_filename(old_path)
+    if not rel:
+        raise ModelMoveError(
+            "INVALID_MODEL_PATH",
+            f"cannot determine relative path for model asset: {old_path}",
+            status=400,
+        )
+    new_path = os.path.abspath(os.path.join(new_base, rel))
+    try:
+        validate_path_within_base(new_path, new_base)
+    except ValueError as e:
+        raise ModelMoveError("INVALID_MODEL_PATH", str(e), status=400)
+
+    if new_path == old_path:
+        return False
+
+    # Q2: collision -> reject, never clobber. Cover both an on-disk file and a
+    # reference that already owns the destination path.
+    if os.path.exists(new_path):
+        raise ModelMoveError(
+            "DESTINATION_EXISTS", f"destination already exists: {new_path}"
+        )
+    if get_reference_by_file_path(session, new_path) is not None:
+        raise ModelMoveError(
+            "DESTINATION_EXISTS", f"destination already registered: {new_path}"
+        )
+
+    os.makedirs(os.path.dirname(new_path), exist_ok=True)
+    _move_file(old_path, new_path)
+    try:
+        _reregister_moved_reference(session, ref, new_path, origin=origin)
+    except Exception:
+        # Never half-move: roll the file back before surfacing the failure.
+        with contextlib.suppress(Exception):
+            _move_file(new_path, old_path)
+        raise
+    return True
+
+
+def _reregister_moved_reference(
+    session: Session,
+    ref: AssetReference,
+    new_path: str,
+    origin: str = "automatic",
+) -> None:
+    """Point ``ref`` at ``new_path`` and reconcile path-derived system tags.
+
+    Re-derives ``models`` + ``model_type:*`` from the new location, drops any
+    stale ``model_type:`` no longer justified by the path, and refreshes the
+    relative ``filename`` metadata. User labels are left untouched.
+    """
+    # Bytes are unchanged by a move; only the location and mtime can shift
+    # (shutil.move's cross-device fallback re-stats).
+    _size_bytes, mtime_ns = get_size_and_mtime_ns(new_path)
+    ref.file_path = new_path
+    ref.mtime_ns = mtime_ns
+    ref.updated_at = get_utc_now()
+    session.flush()
+
+    derived = get_backend_system_tags_from_path(new_path)
+    derived_model_types = {t for t in derived if t.startswith("model_type:")}
+
+    current = set(get_reference_tags(session, reference_id=ref.id))
+    stale = {
+        t for t in current if t.startswith("model_type:")
+    } - derived_model_types
+    if stale:
+        remove_tags_from_reference(
+            session, reference_id=ref.id, tags=sorted(stale)
+        )
+    add_tags_to_reference(
+        session,
+        reference_id=ref.id,
+        tags=derived,
+        origin=origin,
+        create_if_missing=True,
+    )
+
+    _update_metadata_with_filename(
+        session,
+        reference_id=ref.id,
+        file_path=new_path,
+        current_metadata=ref.user_metadata,
+        user_metadata={},
+    )
+
+
 def upload_from_temp_path(
     temp_path: str,
     name: str | None = None,

app/assets/services/path_utils.py

@@ -25,6 +25,44 @@ def get_comfy_models_folders() -> list[tuple[str, list[str]]]:
     return targets
 
 
+def get_model_base_for_folder(folder_name: str) -> str:
+    """Resolve a registered model ``folder_name`` to its canonical base path.
+
+    This is the single-valued reverse lookup (model_type -> on-disk base) the
+    upload destination and the edit-type move (BE-1641) share: resolve the one
+    named folder to its first configured base. Never fans out.
+
+    Raises:
+        ValueError: the folder_name is not registered, or has no base path.
+    """
+    model_folder_paths = dict(get_comfy_models_folders())
+    try:
+        bases = model_folder_paths[folder_name]
+    except KeyError:
+        raise ValueError(f"unknown model category '{folder_name}'")
+    if not bases:
+        raise ValueError(f"no base path configured for category '{folder_name}'")
+    return os.path.abspath(bases[0])
+
+
+def model_folders_for_path(path: str) -> list[str]:
+    """Return every model folder_name whose registered base covers ``path``.
+
+    Set-valued (spec-drift §1): a path shared by >=2 registered folders (e.g.
+    ``diffusion_models`` and ``unet_gguf`` both registering ``models/unet``)
+    belongs to all of them. Purely lexical — does not require the file to exist.
+    Empty when ``path`` is not under any model base (e.g. an input/output file).
+    """
+    fp_path = Path(os.path.abspath(path))
+    out: list[str] = []
+    for folder_name, bases in get_comfy_models_folders():
+        for base in bases:
+            if fp_path.is_relative_to(os.path.abspath(base)):
+                out.append(folder_name)
+                break
+    return out
+
+
 def _validate_subfolder(subfolder: str | None) -> list[str]:
     if not subfolder:
         return []
@@ -65,14 +103,7 @@ def resolve_destination_from_tags(
         folder_name = model_type_tags[0].split(":", 1)[1]
         if not folder_name:
             raise ValueError("models uploads require exactly one model_type:<folder_name> tag")
-        model_folder_paths = dict(get_comfy_models_folders())
-        try:
-            bases = model_folder_paths[folder_name]
-        except KeyError:
-            raise ValueError(f"unknown model category '{folder_name}'")
-        if not bases:
-            raise ValueError(f"no base path configured for category '{folder_name}'")
-        base_dir = os.path.abspath(bases[0])
+        base_dir = get_model_base_for_folder(folder_name)
     elif root == "input":
         base_dir = os.path.abspath(folder_paths.get_input_directory())
     else:

app/assets/services/tagging.py

@@ -9,6 +9,7 @@
     remove_tags_from_reference,
 )
 from app.assets.database.queries.tags import list_tag_counts_for_filtered_assets
+from app.assets.services.ingest import relocate_model_asset_for_model_type_tags
 from app.assets.services.schemas import TagUsage
 from app.database.db import create_session
 
@@ -22,6 +23,12 @@ def apply_tags(
     with create_session() as session:
         ref_row = get_reference_with_owner_check(session, reference_id, owner_id)
 
+        # BE-1641: a flag-on model_type: edit on a filesystem-backed model asset
+        # must MOVE/re-register the file so its location stays coherent with the
+        # label (not a label-only relabel). Runs before the label add so the
+        # path-derived system tags are reconciled first; may raise ModelMoveError.
+        relocate_model_asset_for_model_type_tags(session, ref_row, tags)
+
         result = add_tags_to_reference(
             session,
             reference_id=reference_id,