From 08a4be9508829b0be452e41e1a11a53961e8db13 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Tue, 30 Jun 2026 02:22:06 -0700 Subject: [PATCH 1/3] feat: implement remaining listAssets contract fields (display_name, hash filter, include_public) Bring GET /api/assets into param-for-param parity with the projected openapi.yaml listAssets contract for the three remaining fields: - Add display_name to the Asset response schema (nullable, mirrors name) and populate it from ref.name in _build_asset_response, covering list, get, create, update, and upload responses uniformly. - Add the hash query param to ListAssetsQuery (named hash per the spec, not asset_hash) with before-validation strip/lower normalization, and thread it through list_assets_page -> list_references_page, filtering both the page and count statements on Asset.hash for consistency. - Accept include_public (bool, default true) for contract parity; it is inert in core (no public asset pool) and intentionally not passed to the service layer. Cursor pagination and size optionality are untouched. Add integration tests covering display_name mirroring, exact hash match, unknown-hash empty page, and include_public acceptance. --- app/assets/api/routes.py | 1 + app/assets/api/schemas_in.py | 21 +++++ .../database/queries/asset_reference.py | 6 ++ app/assets/services/asset_management.py | 2 + tests-unit/assets_test/test_list_filter.py | 81 +++++++++++++++++++ 5 files changed, 111 insertions(+) diff --git a/app/assets/api/routes.py b/app/assets/api/routes.py index 43e60094c..13ff6a38c 100644 --- a/app/assets/api/routes.py +++ b/app/assets/api/routes.py @@ -228,6 +228,7 @@ async def list_assets_route(request: web.Request) -> web.Response: exclude_tags=q.exclude_tags, name_contains=q.name_contains, metadata_filter=q.metadata_filter, + asset_hash=q.hash, limit=q.limit, offset=q.offset, sort=sort, diff --git a/app/assets/api/schemas_in.py b/app/assets/api/schemas_in.py index 38a942b7b..d55d68704 100644 --- a/app/assets/api/schemas_in.py +++ b/app/assets/api/schemas_in.py @@ -54,6 +54,16 @@ class ListAssetsQuery(BaseModel): exclude_tags: list[str] = Field(default_factory=list) name_contains: str | None = None + # Filter to assets whose content hash matches exactly. Param name is `hash` + # per the projected openapi.yaml listAssets contract (the response-body field + # is `asset_hash`; the query param is `hash`). + hash: str | None = None + + # Declared for cloud/core contract parity. Core has no public asset pool, so + # this is inert: results are always the caller's own assets. Accepted (not + # rejected) so the FE needs no isCloud branch. + include_public: bool = True + # Accept either a JSON string (query param) or a dict metadata_filter: dict[str, Any] | None = None @@ -86,6 +96,17 @@ class ListAssetsQuery(BaseModel): return out return v + @field_validator("hash", mode="before") + @classmethod + def _normalize_hash(cls, v): + # Normalize for an exact match against stored hashes (which are + # lowercase `blake3:`). Liberal in what we accept — no pattern + # enforcement; a non-matching value simply yields an empty page. + if isinstance(v, str): + v = v.strip().lower() + return v or None + return v + @field_validator("metadata_filter", mode="before") @classmethod def _parse_metadata_json(cls, v): diff --git a/app/assets/database/queries/asset_reference.py b/app/assets/database/queries/asset_reference.py index 967b0e43a..f478be7d3 100644 --- a/app/assets/database/queries/asset_reference.py +++ b/app/assets/database/queries/asset_reference.py @@ -261,6 +261,7 @@ def list_references_page( limit: int = 100, offset: int = 0, name_contains: str | None = None, + asset_hash: str | None = None, include_tags: Sequence[str] | None = None, exclude_tags: Sequence[str] | None = None, metadata_filter: dict | None = None, @@ -293,6 +294,9 @@ def list_references_page( escaped, esc = escape_sql_like_string(name_contains) base = base.where(AssetReference.name.ilike(f"%{escaped}%", escape=esc)) + if asset_hash: + base = base.where(Asset.hash == asset_hash) + base = apply_tag_filters(base, include_tags, exclude_tags) base = apply_metadata_filter(base, metadata_filter) @@ -345,6 +349,8 @@ def list_references_page( count_stmt = count_stmt.where( AssetReference.name.ilike(f"%{escaped}%", escape=esc) ) + if asset_hash: + count_stmt = count_stmt.where(Asset.hash == asset_hash) count_stmt = apply_tag_filters(count_stmt, include_tags, exclude_tags) count_stmt = apply_metadata_filter(count_stmt, metadata_filter) diff --git a/app/assets/services/asset_management.py b/app/assets/services/asset_management.py index a4c8b5a75..984211fe5 100644 --- a/app/assets/services/asset_management.py +++ b/app/assets/services/asset_management.py @@ -274,6 +274,7 @@ def list_assets_page( exclude_tags: Sequence[str] | None = None, name_contains: str | None = None, metadata_filter: dict | None = None, + asset_hash: str | None = None, limit: int = 20, offset: int = 0, sort: str = "created_at", @@ -319,6 +320,7 @@ def list_assets_page( exclude_tags=exclude_tags, name_contains=name_contains, metadata_filter=metadata_filter, + asset_hash=asset_hash, limit=fetch_limit, offset=offset, sort=sort, diff --git a/tests-unit/assets_test/test_list_filter.py b/tests-unit/assets_test/test_list_filter.py index d1cba87b3..712bc221b 100644 --- a/tests-unit/assets_test/test_list_filter.py +++ b/tests-unit/assets_test/test_list_filter.py @@ -306,6 +306,87 @@ def test_list_assets_invalid_query_rejected(http: requests.Session, api_base: st assert body["error"]["code"] == error_code +def test_list_assets_display_name_emitted(http, api_base, asset_factory, make_asset_bytes): + """`display_name` is emitted for every populated asset and is derived from + the storage path (it ends with the stored filename).""" + scope = f"lf-dispname-{uuid.uuid4().hex[:6]}" + tags = ["models", "checkpoints", "unit-tests", scope] + asset_factory("dn_a.safetensors", tags, {}, make_asset_bytes("dn_a", 700)) + asset_factory("dn_b.safetensors", tags, {}, make_asset_bytes("dn_b", 700)) + + r = http.get( + api_base + "/api/assets", + params={"include_tags": f"unit-tests,{scope}", "limit": "50"}, + timeout=120, + ) + body = r.json() + assert r.status_code == 200, body + assert body["assets"], "expected at least one asset" + for asset in body["assets"]: + assert "display_name" in asset, "populated asset must emit display_name" + assert asset["display_name"], "stored asset must have a path-derived display_name" + assert asset["display_name"].endswith(asset["name"]) + + +def test_list_assets_hash_filter_exact_match(http, api_base, asset_factory, make_asset_bytes): + """`hash` filters to assets whose content hash matches exactly.""" + scope = f"lf-hash-{uuid.uuid4().hex[:6]}" + tags = ["models", "checkpoints", "unit-tests", scope] + a = asset_factory("hf_a.safetensors", tags, {}, make_asset_bytes("hf_a", 1024)) + b = asset_factory("hf_b.safetensors", tags, {}, make_asset_bytes("hf_b", 2048)) + + target = a["hash"] + assert target and a["hash"] != b["hash"], "fixtures must have distinct content hashes" + + r = http.get( + api_base + "/api/assets", + params={"hash": target, "limit": "50"}, + timeout=120, + ) + body = r.json() + assert r.status_code == 200, body + names = [x["name"] for x in body["assets"]] + assert names == [a["name"]] + assert body["total"] == 1 + + +def test_list_assets_hash_filter_no_match(http, api_base, asset_factory, make_asset_bytes): + """A well-formed but unknown hash returns an empty page (200).""" + scope = f"lf-hash-none-{uuid.uuid4().hex[:6]}" + tags = ["models", "checkpoints", "unit-tests", scope] + asset_factory("hn_a.safetensors", tags, {}, make_asset_bytes("hn_a", 800)) + + unknown = "blake3:" + ("0" * 64) + r = http.get( + api_base + "/api/assets", + params={"hash": unknown, "limit": "50"}, + timeout=120, + ) + body = r.json() + assert r.status_code == 200, body + assert body["assets"] == [] + assert body["total"] == 0 + + +def test_list_assets_include_public_accepted(http, api_base, asset_factory, make_asset_bytes): + """`include_public` is accepted for contract parity; core results are always + the caller's own assets regardless of its value (the param is inert).""" + scope = f"lf-incpub-{uuid.uuid4().hex[:6]}" + tags = ["models", "checkpoints", "unit-tests", scope] + a = asset_factory("ip_a.safetensors", tags, {}, make_asset_bytes("ip_a", 900)) + + for value in ("false", "true"): + r = http.get( + api_base + "/api/assets", + params={"include_tags": f"unit-tests,{scope}", "include_public": value, "limit": "50"}, + timeout=120, + ) + body = r.json() + assert r.status_code == 200, body + names = [x["name"] for x in body["assets"]] + assert a["name"] in names, f"caller's own asset must be returned (include_public={value})" + + def test_list_assets_name_contains_literal_underscore( http, api_base, From 65eac2ba820ae2c152d3d5a1e0d79f1b6676ebc9 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Wed, 1 Jul 2026 00:41:42 -0700 Subject: [PATCH 2/3] fix(assets): treat explicit empty hash filter as exact-match miss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review findings on the listAssets contract PR: - Empty `?hash=` no longer collapses to "no filter" (which returned a full unfiltered page); it now stays "" and is matched exactly, yielding an empty page — consistent with the documented "malformed -> empty page" contract. Omitting the param entirely still disables the filter. - Guard `list_references_page` on `asset_hash is not None` (page + count) so the present-empty vs omitted distinction is preserved. - Add tests for hash normalization (uppercase/whitespace) and for the explicit-empty -> empty-page behavior. - Clarify the `include_public` comment: core reads are owner-scoped with no separate public pool, so the flag is inert here; cloud enforces it in its own service layer. --- app/assets/api/schemas_in.py | 14 +++--- .../database/queries/asset_reference.py | 6 ++- tests-unit/assets_test/test_list_filter.py | 45 +++++++++++++++++++ 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/app/assets/api/schemas_in.py b/app/assets/api/schemas_in.py index d55d68704..6f045d32f 100644 --- a/app/assets/api/schemas_in.py +++ b/app/assets/api/schemas_in.py @@ -59,9 +59,11 @@ class ListAssetsQuery(BaseModel): # is `asset_hash`; the query param is `hash`). hash: str | None = None - # Declared for cloud/core contract parity. Core has no public asset pool, so - # this is inert: results are always the caller's own assets. Accepted (not - # rejected) so the FE needs no isCloud branch. + # Declared for cloud/core contract parity. In core, reads are owner-scoped + # (owner_id == "") and there is no separate shared/public pool for this flag + # to include or exclude, so it is inert here and intentionally not threaded + # into the query. Accepted (not rejected) so the FE needs no isCloud branch; + # cloud enforces the flag in its own service layer. include_public: bool = True # Accept either a JSON string (query param) or a dict @@ -102,9 +104,11 @@ class ListAssetsQuery(BaseModel): # Normalize for an exact match against stored hashes (which are # lowercase `blake3:`). Liberal in what we accept — no pattern # enforcement; a non-matching value simply yields an empty page. + # An explicitly-supplied-but-empty value (`?hash=`) stays `""` so it + # is treated as an exact-match miss (empty page), not silently dropped + # to "no filter" — omit the param entirely to disable the filter. if isinstance(v, str): - v = v.strip().lower() - return v or None + return v.strip().lower() return v @field_validator("metadata_filter", mode="before") diff --git a/app/assets/database/queries/asset_reference.py b/app/assets/database/queries/asset_reference.py index f478be7d3..734777377 100644 --- a/app/assets/database/queries/asset_reference.py +++ b/app/assets/database/queries/asset_reference.py @@ -294,7 +294,9 @@ def list_references_page( escaped, esc = escape_sql_like_string(name_contains) base = base.where(AssetReference.name.ilike(f"%{escaped}%", escape=esc)) - if asset_hash: + # `is not None` (not truthiness): an explicit empty hash is an exact-match + # miss (empty page), while an omitted hash (None) disables the filter. + if asset_hash is not None: base = base.where(Asset.hash == asset_hash) base = apply_tag_filters(base, include_tags, exclude_tags) @@ -349,7 +351,7 @@ def list_references_page( count_stmt = count_stmt.where( AssetReference.name.ilike(f"%{escaped}%", escape=esc) ) - if asset_hash: + if asset_hash is not None: count_stmt = count_stmt.where(Asset.hash == asset_hash) count_stmt = apply_tag_filters(count_stmt, include_tags, exclude_tags) count_stmt = apply_metadata_filter(count_stmt, metadata_filter) diff --git a/tests-unit/assets_test/test_list_filter.py b/tests-unit/assets_test/test_list_filter.py index 712bc221b..6111e8415 100644 --- a/tests-unit/assets_test/test_list_filter.py +++ b/tests-unit/assets_test/test_list_filter.py @@ -368,6 +368,51 @@ def test_list_assets_hash_filter_no_match(http, api_base, asset_factory, make_as assert body["total"] == 0 +def test_list_assets_hash_filter_normalizes_case_and_whitespace( + http, api_base, asset_factory, make_asset_bytes +): + """`hash` is trimmed and lowercased before matching, so an upper-cased, + space-padded value still matches the stored lowercase hash.""" + scope = f"lf-hashnorm-{uuid.uuid4().hex[:6]}" + tags = ["models", "checkpoints", "unit-tests", scope] + a = asset_factory("hnorm_a.safetensors", tags, {}, make_asset_bytes("hnorm_a", 1024)) + + target = a["hash"] + assert target == target.lower(), "stored hash is expected to be lowercase" + messy = f" {target.upper()} " + + r = http.get( + api_base + "/api/assets", + params={"hash": messy, "limit": "50"}, + timeout=120, + ) + body = r.json() + assert r.status_code == 200, body + names = [x["name"] for x in body["assets"]] + assert names == [a["name"]] + assert body["total"] == 1 + + +def test_list_assets_hash_filter_empty_returns_empty_page( + http, api_base, asset_factory, make_asset_bytes +): + """An explicitly-supplied but empty `hash` (`?hash=`) is an exact-match miss + and returns an empty page, rather than silently disabling the filter.""" + scope = f"lf-hashempty-{uuid.uuid4().hex[:6]}" + tags = ["models", "checkpoints", "unit-tests", scope] + asset_factory("he_a.safetensors", tags, {}, make_asset_bytes("he_a", 800)) + + r = http.get( + api_base + "/api/assets", + params={"hash": "", "limit": "50"}, + timeout=120, + ) + body = r.json() + assert r.status_code == 200, body + assert body["assets"] == [] + assert body["total"] == 0 + + def test_list_assets_include_public_accepted(http, api_base, asset_factory, make_asset_bytes): """`include_public` is accepted for contract parity; core results are always the caller's own assets regardless of its value (the param is inert).""" From 0e14070ee32f58c41b8f048e54644efd9eefa27c Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Wed, 8 Jul 2026 22:36:43 -0700 Subject: [PATCH 3/3] test(assets): align new list-filter tests with master tag and display_name semantics Post-rebase alignment with master's namespaced-tag work (#14511): - models uploads now require a model_type: tag, so the new fixtures use model_type:checkpoints instead of a plain checkpoints tag. - display_name is now path-derived (category prefix + hash-based stored filename) rather than mirroring name; the schema field and its population already landed on master, so this branch only keeps the list-response coverage. --- tests-unit/assets_test/test_list_filter.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests-unit/assets_test/test_list_filter.py b/tests-unit/assets_test/test_list_filter.py index 6111e8415..c5e870137 100644 --- a/tests-unit/assets_test/test_list_filter.py +++ b/tests-unit/assets_test/test_list_filter.py @@ -3,7 +3,7 @@ import uuid import pytest import requests -from helpers import assert_hash_fields_consistent +from helpers import assert_hash_fields_consistent, get_asset_filename def test_list_assets_paging_and_sort(http: requests.Session, api_base: str, asset_factory, make_asset_bytes): @@ -307,10 +307,10 @@ def test_list_assets_invalid_query_rejected(http: requests.Session, api_base: st def test_list_assets_display_name_emitted(http, api_base, asset_factory, make_asset_bytes): - """`display_name` is emitted for every populated asset and is derived from - the storage path (it ends with the stored filename).""" + """`display_name` is emitted for every populated asset in list responses, + derived from the storage path (category prefix + hash-based stored filename).""" scope = f"lf-dispname-{uuid.uuid4().hex[:6]}" - tags = ["models", "checkpoints", "unit-tests", scope] + tags = ["models", "model_type:checkpoints", "unit-tests", scope] asset_factory("dn_a.safetensors", tags, {}, make_asset_bytes("dn_a", 700)) asset_factory("dn_b.safetensors", tags, {}, make_asset_bytes("dn_b", 700)) @@ -324,14 +324,14 @@ def test_list_assets_display_name_emitted(http, api_base, asset_factory, make_as assert body["assets"], "expected at least one asset" for asset in body["assets"]: assert "display_name" in asset, "populated asset must emit display_name" - assert asset["display_name"], "stored asset must have a path-derived display_name" - assert asset["display_name"].endswith(asset["name"]) + expected = "checkpoints/" + get_asset_filename(asset["asset_hash"], ".safetensors") + assert asset["display_name"] == expected def test_list_assets_hash_filter_exact_match(http, api_base, asset_factory, make_asset_bytes): """`hash` filters to assets whose content hash matches exactly.""" scope = f"lf-hash-{uuid.uuid4().hex[:6]}" - tags = ["models", "checkpoints", "unit-tests", scope] + tags = ["models", "model_type:checkpoints", "unit-tests", scope] a = asset_factory("hf_a.safetensors", tags, {}, make_asset_bytes("hf_a", 1024)) b = asset_factory("hf_b.safetensors", tags, {}, make_asset_bytes("hf_b", 2048)) @@ -353,7 +353,7 @@ def test_list_assets_hash_filter_exact_match(http, api_base, asset_factory, make def test_list_assets_hash_filter_no_match(http, api_base, asset_factory, make_asset_bytes): """A well-formed but unknown hash returns an empty page (200).""" scope = f"lf-hash-none-{uuid.uuid4().hex[:6]}" - tags = ["models", "checkpoints", "unit-tests", scope] + tags = ["models", "model_type:checkpoints", "unit-tests", scope] asset_factory("hn_a.safetensors", tags, {}, make_asset_bytes("hn_a", 800)) unknown = "blake3:" + ("0" * 64) @@ -374,7 +374,7 @@ def test_list_assets_hash_filter_normalizes_case_and_whitespace( """`hash` is trimmed and lowercased before matching, so an upper-cased, space-padded value still matches the stored lowercase hash.""" scope = f"lf-hashnorm-{uuid.uuid4().hex[:6]}" - tags = ["models", "checkpoints", "unit-tests", scope] + tags = ["models", "model_type:checkpoints", "unit-tests", scope] a = asset_factory("hnorm_a.safetensors", tags, {}, make_asset_bytes("hnorm_a", 1024)) target = a["hash"] @@ -399,7 +399,7 @@ def test_list_assets_hash_filter_empty_returns_empty_page( """An explicitly-supplied but empty `hash` (`?hash=`) is an exact-match miss and returns an empty page, rather than silently disabling the filter.""" scope = f"lf-hashempty-{uuid.uuid4().hex[:6]}" - tags = ["models", "checkpoints", "unit-tests", scope] + tags = ["models", "model_type:checkpoints", "unit-tests", scope] asset_factory("he_a.safetensors", tags, {}, make_asset_bytes("he_a", 800)) r = http.get( @@ -417,7 +417,7 @@ def test_list_assets_include_public_accepted(http, api_base, asset_factory, make """`include_public` is accepted for contract parity; core results are always the caller's own assets regardless of its value (the param is inert).""" scope = f"lf-incpub-{uuid.uuid4().hex[:6]}" - tags = ["models", "checkpoints", "unit-tests", scope] + tags = ["models", "model_type:checkpoints", "unit-tests", scope] a = asset_factory("ip_a.safetensors", tags, {}, make_asset_bytes("ip_a", 900)) for value in ("false", "true"):