Skip to content

Commit 34744cd

Browse files
authored
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.
1 parent 7d11ec3 commit 34744cd

9 files changed

Lines changed: 624 additions & 19 deletions

File tree

app/assets/api/routes.py

Lines changed: 98 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
AssetValidationError,
1919
UploadError,
2020
)
21-
from app.assets.helpers import validate_blake3_hash
21+
from app.assets.helpers import normalize_tags, validate_blake3_hash
2222
from app.assets.api.upload import (
2323
delete_temp_file_if_exists,
2424
parse_multipart_upload,
@@ -117,6 +117,87 @@ def _build_validation_error_response(code: str, ve: ValidationError) -> web.Resp
117117
return _build_error_response(400, code, "Validation failed.", {"errors": errors})
118118

119119

120+
class InvalidTagFilterError(Exception):
121+
"""Invalid combination of tag-filter query parameters."""
122+
123+
def __init__(self, message: str, details: dict):
124+
super().__init__(message)
125+
self.details = details
126+
127+
128+
# Caps the per-tag EXISTS fan-out; deliberately covers the legacy spellings too.
129+
MAX_TAG_FILTER_TAGS = 100
130+
131+
132+
def _resolve_tag_filters(
133+
q: schemas_in.ListAssetsQuery | schemas_in.TagsRefineQuery,
134+
) -> tuple[list[str], list[str], list[str]]:
135+
"""Resolve legacy (include/exclude) and new (all/any/none) tag-filter
136+
spellings into effective (all, any, none) lists.
137+
138+
Combination validation applies only when the request uses at least one
139+
new-name parameter (non-empty after normalisation); requests using only
140+
the legacy names keep their historical behaviour, including degenerate
141+
combinations like include_tags=a&exclude_tags=a.
142+
"""
143+
# model_dump, not attribute access: deprecated fields warn on every attribute read.
144+
legacy = q.model_dump(include={"include_tags", "exclude_tags"})
145+
include_tags = normalize_tags(legacy["include_tags"])
146+
exclude_tags = normalize_tags(legacy["exclude_tags"])
147+
tags_all = normalize_tags(q.tags_all)
148+
tags_any = normalize_tags(q.tags_any)
149+
tags_none = normalize_tags(q.tags_none)
150+
151+
for param_name, values in (
152+
("include_tags", include_tags),
153+
("exclude_tags", exclude_tags),
154+
("tags_all", tags_all),
155+
("tags_any", tags_any),
156+
("tags_none", tags_none),
157+
):
158+
if len(values) > MAX_TAG_FILTER_TAGS:
159+
raise InvalidTagFilterError(
160+
f"'{param_name}' lists {len(values)} tags; the maximum is "
161+
f"{MAX_TAG_FILTER_TAGS}.",
162+
{
163+
"parameter": param_name,
164+
"count": len(values),
165+
"max": MAX_TAG_FILTER_TAGS,
166+
},
167+
)
168+
169+
if not (tags_all or tags_any or tags_none):
170+
return include_tags, [], exclude_tags
171+
172+
if include_tags and tags_all:
173+
raise InvalidTagFilterError(
174+
"Cannot combine 'include_tags' with 'tags_all'; use 'tags_all'.",
175+
{"parameters": ["include_tags", "tags_all"]},
176+
)
177+
if exclude_tags and tags_none:
178+
raise InvalidTagFilterError(
179+
"Cannot combine 'exclude_tags' with 'tags_none'; use 'tags_none'.",
180+
{"parameters": ["exclude_tags", "tags_none"]},
181+
)
182+
183+
all_param, all_list = (
184+
("tags_all", tags_all) if tags_all else ("include_tags", include_tags)
185+
)
186+
none_param, none_list = (
187+
("tags_none", tags_none) if tags_none else ("exclude_tags", exclude_tags)
188+
)
189+
190+
conflicting = sorted(set(all_list) & set(none_list))
191+
if conflicting:
192+
raise InvalidTagFilterError(
193+
f"Query can never match: {', '.join(repr(t) for t in conflicting)} "
194+
f"required by '{all_param}' but rejected by '{none_param}'.",
195+
{"conflicting_tags": conflicting, "parameters": [all_param, none_param]},
196+
)
197+
198+
return all_list, tags_any, none_list
199+
200+
120201
def _validate_sort_field(requested: str | None) -> str:
121202
if not requested:
122203
return "created_at"
@@ -217,15 +298,21 @@ async def list_assets_route(request: web.Request) -> web.Response:
217298
except ValidationError as ve:
218299
return _build_validation_error_response("INVALID_QUERY", ve)
219300

301+
try:
302+
tags_all, tags_any, tags_none = _resolve_tag_filters(q)
303+
except InvalidTagFilterError as e:
304+
return _build_error_response(400, "INVALID_TAG_FILTER", str(e), e.details)
305+
220306
sort = _validate_sort_field(q.sort)
221307
order_candidate = (q.order or "desc").lower()
222308
order = order_candidate if order_candidate in {"asc", "desc"} else "desc"
223309

224310
try:
225311
result = list_assets_page(
226312
owner_id=USER_MANAGER.get_request_user_id(request),
227-
include_tags=q.include_tags,
228-
exclude_tags=q.exclude_tags,
313+
include_tags=tags_all,
314+
exclude_tags=tags_none,
315+
any_tags=tags_any,
229316
name_contains=q.name_contains,
230317
metadata_filter=q.metadata_filter,
231318
limit=q.limit,
@@ -715,10 +802,16 @@ async def get_tags_refine(request: web.Request) -> web.Response:
715802
except ValidationError as ve:
716803
return _build_validation_error_response("INVALID_QUERY", ve)
717804

805+
try:
806+
tags_all, tags_any, tags_none = _resolve_tag_filters(q)
807+
except InvalidTagFilterError as e:
808+
return _build_error_response(400, "INVALID_TAG_FILTER", str(e), e.details)
809+
718810
tag_counts = list_tag_histogram(
719811
owner_id=USER_MANAGER.get_request_user_id(request),
720-
include_tags=q.include_tags,
721-
exclude_tags=q.exclude_tags,
812+
include_tags=tags_all,
813+
exclude_tags=tags_none,
814+
any_tags=tags_any,
722815
name_contains=q.name_contains,
723816
metadata_filter=q.metadata_filter,
724817
limit=q.limit,

app/assets/api/schemas_in.py

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,12 @@ class ParsedUpload:
5050

5151

5252
class ListAssetsQuery(BaseModel):
53-
include_tags: list[str] = Field(default_factory=list)
54-
exclude_tags: list[str] = Field(default_factory=list)
53+
# Deprecated spellings: include_tags ≡ tags_all, exclude_tags ≡ tags_none.
54+
include_tags: list[str] = Field(default_factory=list, deprecated=True)
55+
exclude_tags: list[str] = Field(default_factory=list, deprecated=True)
56+
tags_all: list[str] = Field(default_factory=list)
57+
tags_any: list[str] = Field(default_factory=list)
58+
tags_none: list[str] = Field(default_factory=list)
5559
name_contains: str | None = None
5660

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

73-
@field_validator("include_tags", "exclude_tags", mode="before")
77+
@field_validator(
78+
"include_tags", "exclude_tags", "tags_all", "tags_any", "tags_none",
79+
mode="before",
80+
)
7481
@classmethod
7582
def _split_csv_tags(cls, v):
7683
# Accept "a,b,c" or ["a","b"] (we are liberal in what we accept)
@@ -154,13 +161,20 @@ def _normalize_tags_field(cls, v):
154161

155162

156163
class TagsRefineQuery(BaseModel):
157-
include_tags: list[str] = Field(default_factory=list)
158-
exclude_tags: list[str] = Field(default_factory=list)
164+
# Deprecated spellings: include_tags ≡ tags_all, exclude_tags ≡ tags_none.
165+
include_tags: list[str] = Field(default_factory=list, deprecated=True)
166+
exclude_tags: list[str] = Field(default_factory=list, deprecated=True)
167+
tags_all: list[str] = Field(default_factory=list)
168+
tags_any: list[str] = Field(default_factory=list)
169+
tags_none: list[str] = Field(default_factory=list)
159170
name_contains: str | None = None
160171
metadata_filter: dict[str, Any] | None = None
161172
limit: conint(ge=1, le=1000) = 100
162173

163-
@field_validator("include_tags", "exclude_tags", mode="before")
174+
@field_validator(
175+
"include_tags", "exclude_tags", "tags_all", "tags_any", "tags_none",
176+
mode="before",
177+
)
164178
@classmethod
165179
def _split_csv_tags(cls, v):
166180
if v is None:

app/assets/database/queries/asset_reference.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,8 @@ def list_references_page(
268268
order: str | None = None,
269269
after_cursor_value: object | None = None,
270270
after_cursor_id: str | None = None,
271+
# Appended last so pre-existing positional callers keep binding correctly.
272+
any_tags: Sequence[str] | None = None,
271273
) -> tuple[list[AssetReference], dict[str, list[str]], int]:
272274
"""List references with pagination, filtering, and sorting.
273275
@@ -293,7 +295,7 @@ def list_references_page(
293295
escaped, esc = escape_sql_like_string(name_contains)
294296
base = base.where(AssetReference.name.ilike(f"%{escaped}%", escape=esc))
295297

296-
base = apply_tag_filters(base, include_tags, exclude_tags)
298+
base = apply_tag_filters(base, include_tags, exclude_tags, any_tags)
297299
base = apply_metadata_filter(base, metadata_filter)
298300

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

351353
total = int(session.execute(count_stmt).scalar_one() or 0)

app/assets/database/queries/common.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,13 @@ def apply_tag_filters(
6060
stmt: sa.sql.Select,
6161
include_tags: Sequence[str] | None = None,
6262
exclude_tags: Sequence[str] | None = None,
63+
any_tags: Sequence[str] | None = None,
6364
) -> sa.sql.Select:
64-
"""include_tags: every tag must be present; exclude_tags: none may be present."""
65+
"""include_tags: every tag must be present; any_tags: at least one must be
66+
present; exclude_tags: none may be present."""
6567
include_tags = normalize_tags(include_tags)
6668
exclude_tags = normalize_tags(exclude_tags)
69+
any_tags = normalize_tags(any_tags)
6770

6871
if include_tags:
6972
for tag_name in include_tags:
@@ -74,6 +77,14 @@ def apply_tag_filters(
7477
)
7578
)
7679

80+
if any_tags:
81+
stmt = stmt.where(
82+
exists().where(
83+
(AssetReferenceTag.asset_reference_id == AssetReference.id)
84+
& (AssetReferenceTag.tag_name.in_(any_tags))
85+
)
86+
)
87+
7788
if exclude_tags:
7889
stmt = stmt.where(
7990
~exists().where(

app/assets/database/queries/tags.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,8 @@ def list_tag_counts_for_filtered_assets(
340340
name_contains: str | None = None,
341341
metadata_filter: dict | None = None,
342342
limit: int = 100,
343+
# Appended last so pre-existing positional callers keep binding correctly.
344+
any_tags: Sequence[str] | None = None,
343345
) -> dict[str, int]:
344346
"""Return tag counts for assets matching the given filters.
345347
@@ -359,7 +361,7 @@ def list_tag_counts_for_filtered_assets(
359361
escaped, esc = escape_sql_like_string(name_contains)
360362
ref_sq = ref_sq.where(AssetReference.name.ilike(f"%{escaped}%", escape=esc))
361363

362-
ref_sq = apply_tag_filters(ref_sq, include_tags, exclude_tags)
364+
ref_sq = apply_tag_filters(ref_sq, include_tags, exclude_tags, any_tags)
363365
ref_sq = apply_metadata_filter(ref_sq, metadata_filter)
364366
ref_sq = ref_sq.subquery()
365367

app/assets/services/asset_management.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,8 @@ def list_assets_page(
279279
sort: str = "created_at",
280280
order: str = "desc",
281281
after: str | None = None,
282+
# Appended last so pre-existing positional callers keep binding correctly.
283+
any_tags: Sequence[str] | None = None,
282284
) -> ListAssetsResult:
283285
"""List assets with optional cursor pagination.
284286
@@ -317,6 +319,7 @@ def list_assets_page(
317319
owner_id=owner_id,
318320
include_tags=include_tags,
319321
exclude_tags=exclude_tags,
322+
any_tags=any_tags,
320323
name_contains=name_contains,
321324
metadata_filter=metadata_filter,
322325
limit=fetch_limit,

app/assets/services/tagging.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,16 @@ def list_tag_histogram(
8585
name_contains: str | None = None,
8686
metadata_filter: dict | None = None,
8787
limit: int = 100,
88+
# Appended last so pre-existing positional callers keep binding correctly.
89+
any_tags: Sequence[str] | None = None,
8890
) -> dict[str, int]:
8991
with create_session() as session:
9092
return list_tag_counts_for_filtered_assets(
9193
session,
9294
owner_id=owner_id,
9395
include_tags=include_tags,
9496
exclude_tags=exclude_tags,
97+
any_tags=any_tags,
9598
name_contains=name_contains,
9699
metadata_filter=metadata_filter,
97100
limit=limit,

0 commit comments

Comments
 (0)