Review feedback on the jobs cursor pagination:
- get_all_jobs now returns JobsPage, a NamedTuple, instead of a bare
4-tuple (callers unpack positionally either way).
- Early-out when the filtered job set is empty so paging code never has
to reason about indexing into an empty list. A malformed 'after'
cursor is still decoded first and rejected with INVALID_CURSOR.
- Document that job ids are server-assigned UUIDs, always present and
unique — the empty-string fallback in _job_id_key only shields
sorted() from a malformed dict, it is not part of the keyset
contract.
Mirror the cloud jobs cursor (BE-885) on the OSS Python server so the
frontend sees one contract across runtimes.
- apply_sorting now appends the job id as a tiebreaker, making (create_time,
id) a stable keyset; without it, ties could reorder between pages.
- get_all_jobs accepts an opaque 'after' cursor (honored only for created_at
sort, like cloud), keyset-filters the sorted in-memory list, and returns
has_more + a next_cursor. Minted in offset mode too so a client can bootstrap
into keyset pagination.
- server.py /api/jobs parses 'after', returns next_cursor in the pagination
object, and maps a malformed cursor to 400 INVALID_CURSOR.
- Reuses the shared utils.cursor codec (base64url JSON {s,v,id,o}) so the wire
format matches cloud and assets exactly.
Tests: asc/desc multi-page round-trip, same-create_time tiebreaker, last-page
no-cursor, offset-mode bootstrap, execution_duration ignores cursor, malformed
cursor raises.
The keyset cursor codec was asset-namespaced (app/assets/services/cursor.py)
but the wire format and encode/decode logic are domain-agnostic. Move it to
utils/cursor.py so the jobs endpoint can share one codec instead of importing
across domains or duplicating it.
tag_type was always "user" in practice — no code path ever set it to anything
else (no system/seeded classification was wired up) and nothing queried it. The
column, its ix_tags_tag_type index, and the TagUsage.type API field were dead
weight, so they're removed. Adds alembic migration 0004 to drop the column and
index.
Verified: asset-seeder tests pass; migration applies cleanly on a fresh SQLite
(tags retains only name; tag_type column + index dropped).
Co-authored-by: guill <jacob.e.segal@gmail.com>
The custom JSON escaping of <, >, &, U+2028, and U+2029 existed only to
keep the encoded cursor byte-identical with the Cloud implementation of
the same payload format. Cursors are opaque tokens, so byte-level
compatibility across implementations is not needed — plain json.dumps
output is sufficient. Remove the escaping helper and the byte-identity
test fixtures that pinned the wire format; keep round-trip coverage for
the affected characters.
cleanup_models_gc can be called once per load_models_gpu via
free_memory, which in turn can de-activate an active model via
this reset_cast_buffers.
cleanup_models_gc() could also come via obscure garbage collector
paths so limit reset_cast_buffers to the post-node callsite instead.
Addresses review feedback on cursor.py:
- Extract the inline escape chain into _apply_wire_compatible_json_escapes()
with a comment pinning it to the wire format's escape set, so the parity
intent is explicit rather than reading as an ad-hoc transform.
- Raise MAX_ENCODED_CURSOR_LENGTH to 8192 (comfortably above the ~5.2KB
worst-case the per-field caps can produce) and drop the mint-time length
guard. Encoder/decoder symmetry now holds by construction: the encoder
can't produce a cursor the decode path rejects, so there is no confusing
user-visible 'cursor too long' failure at mint time.
- Rewrite the two over-wire-cap tests to assert worst-case multibyte and
escape-heavy values mint and round-trip, instead of being rejected.
* feat(assets): extract image dimensions at ingest and emit on asset responses
Image assets now carry width/height under the existing `metadata` field on
asset responses, shaped as `{"kind": "image", "width": W, "height": H}`.
This lets consumers get original dimensions (e.g. for clients that render
server-side thumbnails and can't recover them from naturalWidth/Height)
without an extra round-trip.
Dimensions are written to AssetReference.system_metadata across three
ingest paths:
- Direct file ingest (upload, in-place registration): Pillow reads the
image header right after hashing, while the file is still in OS page
cache. Non-image MIME types are skipped without touching the file.
- From-hash registration: this path never reads the file bytes, so
dimensions are best-effort copied from any prior sibling reference of
the same asset that already carries kind=image metadata. Missing
siblings, non-image siblings, or absent dimension keys leave the new
reference's metadata unchanged.
- Scanner enrichment: extends the existing system_metadata write in
enrich_asset so scanner-registered images get the same treatment as
uploaded ones.
Existing system_metadata keys (e.g. safetensors fields written by the
enricher, download provenance) are preserved through merge. Existing
assets ingested before this change retain their current metadata — no
automatic backfill in this PR.
Tests cover image emission, non-image no-op, merge preservation, and the
from-hash sibling back-fill (including the no-sibling and non-image-sibling
cases).
* fix(assets): validate sibling dimensions before backfilling
Per CodeRabbit review on #13991: the previous loop accepted any sibling
with `kind == "image"` and copied whichever dimension keys happened to
be present, then returned. A partial sibling (kind set but missing or
invalid width/height) could persist incomplete metadata onto the new
reference even when a later sibling had valid dimensions.
Now we validate that the sibling has both width and height as positive
integers before adopting its dimensions, and continue scanning to the
next sibling otherwise.
* fix(assets): reject booleans in sibling dimension validation (use type-is)
Per CodeRabbit follow-up on #13991: bool is a subclass of int in Python,
so isinstance(True, int) is True. The previous strict-int gate would
have accepted width=True (truthy + > 0) as a valid dimension.
Realistic occurrence is low (extract_image_dimensions returns proper
ints, JSON doesn't serialize bools as numbers), but the validation gate
exists for defense-in-depth so it should be actually strict.
---------
Co-authored-by: guill <jacob.e.segal@gmail.com>
* mm: split off registration helper to doer and headroom calc
* pinned_memory: implement registration comfy side
Move away from Aimdo buffer registrations which seem fraught with
danger and do it comfy side. Just start with the basic move.
* pinned_memory: do registrations as portable memory
* pinned_memory: discard async errors on registration fail
Like the good ol days.
* pinned_memory: implement abs shortfall retry
If pinned registration happens to fail despite the previous budget
ensures, consider the allocation shortfall, ensure it again, and
try again. This allows comfy pins to interoperate with other software
that might be doing substantive pinning.
Matt Miller
guill
Kohaku-Blueleaf
Comfy Org PR Bot
comfyanonymous
Talmaj
Jukka Seppänen
rattus
Alexis Rolland
kelseyee
Silver
Daxiong (Lin)
John Pollock
Terry Jia
Alexander Piskun
Yousef R. Gamaleldin