fix(assets): move model asset on model_type: edit to stay loader-coherent#14561
Draft
synap5e wants to merge 2 commits into
Conversation
4a757fc to
4340337
Compare
…rent Under supports_model_type_tags, a model_type: edit on a filesystem-backed model asset now relocates and re-registers the file to the folder matching the new model_type:<folder_name>, so the file location stays coherent with the label instead of a label-only relabel that left the file loader-orphaned. Triggered off the POST /tags add (apply_tags): on a registered folder change the file moves (preserving subfolder + filename), path-derived system tags are re-derived, and filename metadata refreshed, all in one transaction with file rollback on failure. Collisions are rejected (never clobber); an unregistered model_type: stays a permissive label; non-model and hash-only references are untouched. Shared on-disk folders keep their plural twins.
Make the edit path symmetric with upload: model_type: is an operational placement tag on a model asset, so an unregistered folder_name is an invalid placement target and is rejected (400 UNKNOWN_MODEL_TYPE), not stored as a label. A real edit-type action always targets a registered folder from discovery, so only junk manual adds are affected. Narrowly update the system-looking-labels test to drop the now-operational bare model_type: case (other reserved-ish prefixes still store as labels) and add a focused unknown-model_type:-on-model-asset rejection test.
57d0efd to
5e267fa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes BE-1641. Stacks on #14511 (
synap5e/assets-namespaced-tags); auto-retargets tomasterwhen that lands.Core advertises
supports_model_type_tags: true, and the FE edit-type flow writesmodel_type:<folder_name>under that flag. But Core tag mutation was label-only: relabeling a checkpoint asmodel_type:lorasleft the file incheckpoints/while the FE grouped/node-created it as a LoRA — a load-incoherent asset. Core's upload write was already semantic (BE-1463); only the edit-type relabel was not.This makes the edit coherent by moving/re-registering the file to match the new
model_type:, matching the upload contract.How it works
The FE edit-type flow diff-dispatches
DELETE /tags(oldmodel_type:) thenPOST /tags(newmodel_type:). The move is driven off thePOST /tagsadd (apply_tags), so there is no FE change. When amodel_type:<folder>is applied to a filesystem-backed model asset whose file isn't already under that folder:<folder>→ its base path (same machinery the upload path uses).new_base / <relative path>— preserving subfolder structure and filename (no rehash),os.replacewith ashutil.movecross-device fallback.models+model_type:*) from the new location, dropping stale ones and refreshingfilenamemetadata — all in one transaction, with file rollback if the DB step fails.Edge behavior
model_type:on a non-2xx, so the asset stays coherent.model_type:→ kept as a permissive label (no move, no reject) — preserves Core's existing local/trusted labeling contract (spec-drift §3). A real edit-type action always targets a registered folder.diffusion_models/unet_gguf) → no physical move; pluralmodel_type:twins are preserved (spec-drift §1).Tests
tests-unit/assets_test/services/test_model_type_move.py— 13 service-level tests (move happy-path, subfolder/label/filename preservation, same-type & shared-dir no-ops, hash-only/non-model label-only, unknown-folder→label, collision×2→409, DB-failure rollback).tests-unit/assets_test/test_tags_api.py— 2 HTTP end-to-end tests (real DELETE-then-POST edit-type move verified on disk; unknown folder stays a label).tests-unit/assets_test: 435 passed, 10 skipped.Acceptance criteria
supports_model_type_tags, amodel_type:edit moves/re-registers the file so location stays coherent with the label.model_type:writes" guarantee now holds coherently for edit-type (move), matching the semantic upload path (BE-1463).