Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(assets): treat explicit empty hash filter as exact-match miss
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.
  • Loading branch information
mattmillerai committed Jul 9, 2026
commit 65eac2ba820ae2c152d3d5a1e0d79f1b6676ebc9
14 changes: 9 additions & 5 deletions app/assets/api/schemas_in.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
mattmillerai marked this conversation as resolved.
Outdated
include_public: bool = True
Comment thread
mattmillerai marked this conversation as resolved.

# Accept either a JSON string (query param) or a dict
Expand Down Expand Up @@ -102,9 +104,11 @@ def _normalize_hash(cls, v):
# Normalize for an exact match against stored hashes (which are
# lowercase `blake3:<hex>`). 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")
Expand Down
6 changes: 4 additions & 2 deletions app/assets/database/queries/asset_reference.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
mattmillerai marked this conversation as resolved.
Outdated

base = apply_tag_filters(base, include_tags, exclude_tags)
Expand Down Expand Up @@ -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)
Expand Down
45 changes: 45 additions & 0 deletions tests-unit/assets_test/test_list_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)."""
Expand Down