Add tags_all / tags_any / tags_none tag filters to the assets list API (#15332)

* Implement tags_all/tags_any/tags_none on the assets list API (BE-6600) Adds the three canonically-named tag filter params to GET /api/assets and GET /api/assets/tags/refine: - tags_all: asset carries every tag (replaces include_tags) - tags_any: asset carries at least one tag (new) - tags_none: asset carries no tag (replaces exclude_tags) Clauses intersect; tags_none always wins. include_tags/exclude_tags remain as permanent deprecated aliases and behave exactly as before when used on their own. Invalid combinations return 400 INVALID_TAG_FILTER, but only when the request uses at least one new-name parameter (non-empty after normalisation): - mixed spellings of one slot (include_tags with tags_all, exclude_tags with tags_none) - the same tag in the effective all-list and none-list (query can never match) Old-names-only requests gain no new error paths: include_tags=a&exclude_tags=a still returns an empty 200. tags_any/tags_none overlap stays valid (dead term, not a dead query). * Address review findings: positional-compat, deprecation metadata, test matrix - Move any_tags to the end of the four touched signatures: inserting it mid-signature silently misbound pre-existing positional callers (e.g. a caller passing name_contains positionally would have it consumed as any_tags). - Mark include_tags/exclude_tags Field(deprecated=True) on both list schemas so generated schema metadata matches the contract, not just a comment (schemas_out.py already uses this form for Asset.name). - Add tests: legal cross-slot old/new combinations, repeated query-key concatenation (pins Core behavior; outside the cross-platform contract), tags_any two-page cursor consistency (total/has_more/ no-overlap), refine-route mixed-spelling rejection + legacy-conflict preservation, and schema deprecation metadata. * Pin tag-value opacity: case-sensitive matching, byte-exact conflict check The prod tag survey (~/comfy/prod-model-tag-shape.md) found live case-distinct tag pairs (SEEDVR2/seedvr2) that resolve differently, so the contract now states tag values are opaque byte-strings. Pin that: case-distinct tags filter separately, and a case-distinct all/none pair is not an INVALID_TAG_FILTER conflict. * Document tags_all/tags_any/tags_none in openapi.yaml, deprecate aliases Add the three tag-filter parameters to both listAssets and getAssetTagHistogram parameter blocks and mark include_tags/exclude_tags deprecated: true, keeping the spec in step with the runtime schemas so generated clients can discover the new filters while the aliases stay present for existing consumers. * Move schemas_in import to module scope in test_list_filter Review feedback: no import cycle requires the local import. * Silence per-request DeprecationWarning in the tag-filter remap shim Reading the deprecated include_tags/exclude_tags fields by attribute fires pydantic's DeprecationWarning on every list/refine request even for callers using only the new names. The warning is aimed at API clients, not the server's own remap; read via model_dump instead. * Cap tag-filter lists at 100 entries, all spellings Review finding: unbounded tag lists fan out into one correlated EXISTS per tag on both page and count statements. Cap each list at 100 normalized entries with 400 INVALID_TAG_FILTER naming the parameter. Applies to the legacy spellings as well — a deliberate, decided exception to the old-names-behave-identically rule, since a cap only on new names would leave the same fan-out reachable through the aliases. * Strip process narration from comments Comments carried decision dates, contract cross-references, and review context. Keep only the constraints the code cannot show, one line each.
2026-08-24 18:10:27 +08:00 · 2026-08-10 14:05:21 -07:00
parent 7d11ec31cb
commit 34744cd29e
9 changed files with 624 additions and 19 deletions
--- a/app/assets/api/routes.py
+++ b/app/assets/api/routes.py
@@ -18,7 +18,7 @@ from app.assets.api.schemas_in import (
    AssetValidationError,
    UploadError,
 )
-from app.assets.helpers import validate_blake3_hash
+from app.assets.helpers import normalize_tags, validate_blake3_hash
 from app.assets.api.upload import (
    delete_temp_file_if_exists,
    parse_multipart_upload,
@@ -117,6 +117,87 @@ def _build_validation_error_response(code: str, ve: ValidationError) -> web.Resp
    return _build_error_response(400, code, "Validation failed.", {"errors": errors})


+class InvalidTagFilterError(Exception):
+    """Invalid combination of tag-filter query parameters."""
+
+    def __init__(self, message: str, details: dict):
+        super().__init__(message)
+        self.details = details
+
+
+# Caps the per-tag EXISTS fan-out; deliberately covers the legacy spellings too.
+MAX_TAG_FILTER_TAGS = 100
+
+
+def _resolve_tag_filters(
+    q: schemas_in.ListAssetsQuery | schemas_in.TagsRefineQuery,
+) -> tuple[list[str], list[str], list[str]]:
+    """Resolve legacy (include/exclude) and new (all/any/none) tag-filter
+    spellings into effective (all, any, none) lists.
+
+    Combination validation applies only when the request uses at least one
+    new-name parameter (non-empty after normalisation); requests using only
+    the legacy names keep their historical behaviour, including degenerate
+    combinations like include_tags=a&exclude_tags=a.
+    """
+    # model_dump, not attribute access: deprecated fields warn on every attribute read.
+    legacy = q.model_dump(include={"include_tags", "exclude_tags"})
+    include_tags = normalize_tags(legacy["include_tags"])
+    exclude_tags = normalize_tags(legacy["exclude_tags"])
+    tags_all = normalize_tags(q.tags_all)
+    tags_any = normalize_tags(q.tags_any)
+    tags_none = normalize_tags(q.tags_none)
+
+    for param_name, values in (
+        ("include_tags", include_tags),
+        ("exclude_tags", exclude_tags),
+        ("tags_all", tags_all),
+        ("tags_any", tags_any),
+        ("tags_none", tags_none),
+    ):
+        if len(values) > MAX_TAG_FILTER_TAGS:
+            raise InvalidTagFilterError(
+                f"'{param_name}' lists {len(values)} tags; the maximum is "
+                f"{MAX_TAG_FILTER_TAGS}.",
+                {
+                    "parameter": param_name,
+                    "count": len(values),
+                    "max": MAX_TAG_FILTER_TAGS,
+                },
+            )
+
+    if not (tags_all or tags_any or tags_none):
+        return include_tags, [], exclude_tags
+
+    if include_tags and tags_all:
+        raise InvalidTagFilterError(
+            "Cannot combine 'include_tags' with 'tags_all'; use 'tags_all'.",
+            {"parameters": ["include_tags", "tags_all"]},
+        )
+    if exclude_tags and tags_none:
+        raise InvalidTagFilterError(
+            "Cannot combine 'exclude_tags' with 'tags_none'; use 'tags_none'.",
+            {"parameters": ["exclude_tags", "tags_none"]},
+        )
+
+    all_param, all_list = (
+        ("tags_all", tags_all) if tags_all else ("include_tags", include_tags)
+    )
+    none_param, none_list = (
+        ("tags_none", tags_none) if tags_none else ("exclude_tags", exclude_tags)
+    )
+
+    conflicting = sorted(set(all_list) & set(none_list))
+    if conflicting:
+        raise InvalidTagFilterError(
+            f"Query can never match: {', '.join(repr(t) for t in conflicting)} "
+            f"required by '{all_param}' but rejected by '{none_param}'.",
+            {"conflicting_tags": conflicting, "parameters": [all_param, none_param]},
+        )
+
+    return all_list, tags_any, none_list
+
+
 def _validate_sort_field(requested: str | None) -> str:
    if not requested:
        return "created_at"
@@ -217,6 +298,11 @@ async def list_assets_route(request: web.Request) -> web.Response:
    except ValidationError as ve:
        return _build_validation_error_response("INVALID_QUERY", ve)

+    try:
+        tags_all, tags_any, tags_none = _resolve_tag_filters(q)
+    except InvalidTagFilterError as e:
+        return _build_error_response(400, "INVALID_TAG_FILTER", str(e), e.details)
+
    sort = _validate_sort_field(q.sort)
    order_candidate = (q.order or "desc").lower()
    order = order_candidate if order_candidate in {"asc", "desc"} else "desc"
@@ -224,8 +310,9 @@ async def list_assets_route(request: web.Request) -> web.Response:
    try:
        result = list_assets_page(
            owner_id=USER_MANAGER.get_request_user_id(request),
-            include_tags=q.include_tags,
-            exclude_tags=q.exclude_tags,
+            include_tags=tags_all,
+            exclude_tags=tags_none,
+            any_tags=tags_any,
            name_contains=q.name_contains,
            metadata_filter=q.metadata_filter,
            limit=q.limit,
@@ -715,10 +802,16 @@ async def get_tags_refine(request: web.Request) -> web.Response:
    except ValidationError as ve:
        return _build_validation_error_response("INVALID_QUERY", ve)

+    try:
+        tags_all, tags_any, tags_none = _resolve_tag_filters(q)
+    except InvalidTagFilterError as e:
+        return _build_error_response(400, "INVALID_TAG_FILTER", str(e), e.details)
+
    tag_counts = list_tag_histogram(
        owner_id=USER_MANAGER.get_request_user_id(request),
-        include_tags=q.include_tags,
-        exclude_tags=q.exclude_tags,
+        include_tags=tags_all,
+        exclude_tags=tags_none,
+        any_tags=tags_any,
        name_contains=q.name_contains,
        metadata_filter=q.metadata_filter,
        limit=q.limit,
--- a/app/assets/api/schemas_in.py
+++ b/app/assets/api/schemas_in.py
@@ -50,8 +50,12 @@ class ParsedUpload:


 class ListAssetsQuery(BaseModel):
-    include_tags: list[str] = Field(default_factory=list)
-    exclude_tags: list[str] = Field(default_factory=list)
+    # Deprecated spellings: include_tags ≡ tags_all, exclude_tags ≡ tags_none.
+    include_tags: list[str] = Field(default_factory=list, deprecated=True)
+    exclude_tags: list[str] = Field(default_factory=list, deprecated=True)
+    tags_all: list[str] = Field(default_factory=list)
+    tags_any: list[str] = Field(default_factory=list)
+    tags_none: list[str] = Field(default_factory=list)
    name_contains: str | None = None

    # Accept either a JSON string (query param) or a dict
@@ -70,7 +74,10 @@ class ListAssetsQuery(BaseModel):
    )
    order: Literal["asc", "desc"] = "desc"

-    @field_validator("include_tags", "exclude_tags", mode="before")
+    @field_validator(
+        "include_tags", "exclude_tags", "tags_all", "tags_any", "tags_none",
+        mode="before",
+    )
    @classmethod
    def _split_csv_tags(cls, v):
        # Accept "a,b,c" or ["a","b"] (we are liberal in what we accept)
@@ -154,13 +161,20 @@ class CreateFromHashBody(BaseModel):


 class TagsRefineQuery(BaseModel):
-    include_tags: list[str] = Field(default_factory=list)
-    exclude_tags: list[str] = Field(default_factory=list)
+    # Deprecated spellings: include_tags ≡ tags_all, exclude_tags ≡ tags_none.
+    include_tags: list[str] = Field(default_factory=list, deprecated=True)
+    exclude_tags: list[str] = Field(default_factory=list, deprecated=True)
+    tags_all: list[str] = Field(default_factory=list)
+    tags_any: list[str] = Field(default_factory=list)
+    tags_none: list[str] = Field(default_factory=list)
    name_contains: str | None = None
    metadata_filter: dict[str, Any] | None = None
    limit: conint(ge=1, le=1000) = 100

-    @field_validator("include_tags", "exclude_tags", mode="before")
+    @field_validator(
+        "include_tags", "exclude_tags", "tags_all", "tags_any", "tags_none",
+        mode="before",
+    )
    @classmethod
    def _split_csv_tags(cls, v):
        if v is None:
--- a/app/assets/database/queries/asset_reference.py
+++ b/app/assets/database/queries/asset_reference.py
@@ -268,6 +268,8 @@ def list_references_page(
    order: str | None = None,
    after_cursor_value: object | None = None,
    after_cursor_id: str | None = None,
+    # Appended last so pre-existing positional callers keep binding correctly.
+    any_tags: Sequence[str] | None = None,
 ) -> tuple[list[AssetReference], dict[str, list[str]], int]:
    """List references with pagination, filtering, and sorting.

@@ -293,7 +295,7 @@ def list_references_page(
        escaped, esc = escape_sql_like_string(name_contains)
        base = base.where(AssetReference.name.ilike(f"%{escaped}%", escape=esc))

-    base = apply_tag_filters(base, include_tags, exclude_tags)
+    base = apply_tag_filters(base, include_tags, exclude_tags, any_tags)
    base = apply_metadata_filter(base, metadata_filter)

    sort = (sort or "created_at").lower()
@@ -345,7 +347,7 @@ def list_references_page(
        count_stmt = count_stmt.where(
            AssetReference.name.ilike(f"%{escaped}%", escape=esc)
        )
-    count_stmt = apply_tag_filters(count_stmt, include_tags, exclude_tags)
+    count_stmt = apply_tag_filters(count_stmt, include_tags, exclude_tags, any_tags)
    count_stmt = apply_metadata_filter(count_stmt, metadata_filter)

    total = int(session.execute(count_stmt).scalar_one() or 0)
--- a/app/assets/database/queries/common.py
+++ b/app/assets/database/queries/common.py
@@ -60,10 +60,13 @@ def apply_tag_filters(
    stmt: sa.sql.Select,
    include_tags: Sequence[str] | None = None,
    exclude_tags: Sequence[str] | None = None,
+    any_tags: Sequence[str] | None = None,
 ) -> sa.sql.Select:
-    """include_tags: every tag must be present; exclude_tags: none may be present."""
+    """include_tags: every tag must be present; any_tags: at least one must be
+    present; exclude_tags: none may be present."""
    include_tags = normalize_tags(include_tags)
    exclude_tags = normalize_tags(exclude_tags)
+    any_tags = normalize_tags(any_tags)

    if include_tags:
        for tag_name in include_tags:
@@ -74,6 +77,14 @@ def apply_tag_filters(
                )
            )

+    if any_tags:
+        stmt = stmt.where(
+            exists().where(
+                (AssetReferenceTag.asset_reference_id == AssetReference.id)
+                & (AssetReferenceTag.tag_name.in_(any_tags))
+            )
+        )
+
    if exclude_tags:
        stmt = stmt.where(
            ~exists().where(
--- a/app/assets/database/queries/tags.py
+++ b/app/assets/database/queries/tags.py
@@ -340,6 +340,8 @@ def list_tag_counts_for_filtered_assets(
    name_contains: str | None = None,
    metadata_filter: dict | None = None,
    limit: int = 100,
+    # Appended last so pre-existing positional callers keep binding correctly.
+    any_tags: Sequence[str] | None = None,
 ) -> dict[str, int]:
    """Return tag counts for assets matching the given filters.

@@ -359,7 +361,7 @@ def list_tag_counts_for_filtered_assets(
        escaped, esc = escape_sql_like_string(name_contains)
        ref_sq = ref_sq.where(AssetReference.name.ilike(f"%{escaped}%", escape=esc))

-    ref_sq = apply_tag_filters(ref_sq, include_tags, exclude_tags)
+    ref_sq = apply_tag_filters(ref_sq, include_tags, exclude_tags, any_tags)
    ref_sq = apply_metadata_filter(ref_sq, metadata_filter)
    ref_sq = ref_sq.subquery()

--- a/app/assets/services/asset_management.py
+++ b/app/assets/services/asset_management.py
@@ -279,6 +279,8 @@ def list_assets_page(
    sort: str = "created_at",
    order: str = "desc",
    after: str | None = None,
+    # Appended last so pre-existing positional callers keep binding correctly.
+    any_tags: Sequence[str] | None = None,
 ) -> ListAssetsResult:
    """List assets with optional cursor pagination.

@@ -317,6 +319,7 @@ def list_assets_page(
            owner_id=owner_id,
            include_tags=include_tags,
            exclude_tags=exclude_tags,
+            any_tags=any_tags,
            name_contains=name_contains,
            metadata_filter=metadata_filter,
            limit=fetch_limit,
--- a/app/assets/services/tagging.py
+++ b/app/assets/services/tagging.py
@@ -85,6 +85,8 @@ def list_tag_histogram(
    name_contains: str | None = None,
    metadata_filter: dict | None = None,
    limit: int = 100,
+    # Appended last so pre-existing positional callers keep binding correctly.
+    any_tags: Sequence[str] | None = None,
 ) -> dict[str, int]:
    with create_session() as session:
        return list_tag_counts_for_filtered_assets(
@@ -92,6 +94,7 @@ def list_tag_histogram(
            owner_id=owner_id,
            include_tags=include_tags,
            exclude_tags=exclude_tags,
+            any_tags=any_tags,
            name_contains=name_contains,
            metadata_filter=metadata_filter,
            limit=limit,