asyncio.get_event_loop() is deprecated in Python 3.10 and will be
removed. Replace with asyncio.get_running_loop() at five call sites
that all run inside async def functions where a running loop is
guaranteed: two async_generate helpers in diffusion.py and three
run_in_executor sites in routes/inference.py (audio synth, diffusion
load, streaming chat). Addresses the gemini-code-assist bot review.
Round 41 review findings (2 P1, 5/12 reviewers consensus on the dominant one):
1. routes/export.py: load_checkpoint already refuses 409 when training
or another export is active, but /export/{merged,base,gguf,lora} and
/cleanup went through _export_public_window without those checks.
A user could start training, then trigger an export (or cleanup),
and both would double-own the GPU. Factor the training-active and
export-active guards into _raise_if_training_active_for_export and
_raise_if_export_active_for_export, call them inside the context
manager so all /export/* + /cleanup share the same fail-closed
semantics as load_checkpoint, and wrap /cleanup with the window.
2. core/inference/diffusion.py: DiffusionBackend.unload_model cleared
_pipe / _repo_id / _family / ... under _lock BEFORE _release(old)
and _drain_cuda_cache. Between the lock release and cache drain,
status() reported is_loaded=False / is_loading=False, so the
helper-busy check (which OR-s those two) could let an AI Assist
GGUF backend start while diffusion VRAM was still being freed.
Set _loading=True inside the lock as a busy marker before clearing
the slot, and only clear it in a finally after release + drain
complete.
Round 40 review findings (5 P1 + 1 P2 + 3 P3):
P1:
1. routes/export.py: wrap /export/merged, /export/base, /export/gguf,
/export/lora in a public-load window so backend.export_*() running
in a worker thread cannot be torn down by a concurrent workload that
sees is_export_active() == False during the pre-active gap.
2. utils/datasets/llm_assist.py: add public_load_pending_for(workload)
helper. routes/inference.py: _release_export_for now refuses 503
when export is mid-handoff.
3. models/models.py: AddScanFolderRequest.path now rejects control
characters and embedded hf_ tokens before being logged or reflected.
4. models/training.py: local_datasets and local_eval_datasets list
entries get the same control-char / embedded-token validators that
model_name / hf_dataset already have.
5. models/training.py: format_type joins the validator list (copied
into training_kwargs and into trainer log lines).
6. models/export.py: _validate_save_directory now rejects embedded hf_
tokens (already covered other identifier fields).
P2:
7. images-page.tsx:162: defer the mount fetchAndUpdateStatus call
through setTimeout(..., 0) so it does not trip
react-hooks/set-state-in-effect on scoped lint.
P3 cleanup:
8. core/inference/diffusion.py: drop unused gguf_basename assignment.
9. core/inference/diffusion.py + routes/inference.py: drop unused
owned_names computation from the chat-release helpers; the final
sweep intentionally no longer filters by that snapshot.
Round 39 review findings (2 P1 + 1 P2):
1. routes/datasets.py: validate raw multipart filename before sanitize so
smuggled control chars (NUL, newline) are rejected at the same boundary
as the JSON path in seed.py. Previously _sanitize_filename stripped the
control chars first, letting raw inputs slip past the validator.
2. routes/inference.py: extend RuntimeError -> 503 mapping in /images/load
to classify "Another GPU workload is mid-handoff" as retryable, so the
backend-surfaced phrasing matches the route-level 503 already returned
from _raise_if_helper_advisor_busy.
3. core/inference/diffusion.py: redact hf_ tokens in the local-path branch
of _display_repo_id so a leaf directory named hf_<token> cannot leak
into UI labels or structured logs.
Round 38 P1: R35e added the public_load_pending() check to the
route-side _raise_if_helper_advisor_busy, but the backend-side
_raise_if_helper_advisor_busy_for_diffusion (used by direct
DiffusionBackend.load_model callers AND called transitively from
the route via backend.load_model) never got the same parity
check. That left a window where:
* /api/training/start published the "training" pending marker
via the route helper
* a script/test calling DiffusionBackend.load_model() directly
passed the backend's helper-busy snapshot, never checked
public_load_pending(), and proceeded to destructive owner
teardown + GPU allocation while training was still pending.
Add the parity check with a kw-only `excluding` parameter on
public_load_pending so a route-wrapped backend call can ignore
the marker its own route already published (route publishes
"diffusion"; backend publishes the separate "diffusion-backend"
tag). load_model gains ignore_public_load_pending_workload to
thread the route's tag through; the diffusion route passes
"diffusion" so the backend's atomic check does not self-block
on the route's own publication.
Verified by smoke test: route-wrapped backend with excluding=
"diffusion" allowed during route's diffusion pending; direct
backend call refused with RuntimeError "Another GPU workload is
mid-handoff" when training is pending. 86 backend tests pass.
Re-fix the round-34 CI regression: the round-34 attempt gated the
accelerate preflight on enable_model_cpu_offload, but the parameter
defaults to True so tests that did not explicitly opt out still
hit the missing-accelerate path. Removed the accelerate preflight
entirely; transformers' PyTorch backend already pulls accelerate
as a hard dep on every supported install path, so the duplicate
find_spec guard is redundant in practice and the missing-package
case will still surface a clean ModuleNotFoundError from the
offload code itself if the user somehow lands there without it.
Round 34 P1 cross-block: extend the seed.py multipart filename
validators (round 33) to /api/datasets/upload. Both routes echo
the filename back to the client and persist it, so per the
asymmetric-fix rule the validators must match. Now rejects
control characters and embedded HF tokens in file.filename in
both upload entry points.
86 targeted backend tests pass.
Backend CI on Python 3.11 failed 15 diffusion tests after R30's
accelerate preflight because the CI test environment does not
install accelerate, but the tests mock from_pretrained and never
exercise the CPU-offload path that actually needs it.
Gate the find_spec("accelerate") check on enable_model_cpu_offload
so the dependency is only required for the path that uses it.
transformers preflight stays unconditional (it is always touched
by from_pretrained). Tests with offload=False (the default) pass
without accelerate; production loads with offload=True still get
the fail-fast unload-protection guard the original round-30 fix
added.
97 targeted backend tests pass (test_diffusion_routes,
test_diffusion_backend, test_inference_model_validation,
test_data_recipe_seed, test_training_raw_support,
test_export_log_cursor).
Three round-32 reviewer findings, plus documentation cleanup for
the local-path Tauri/FE plumbing gap.
Concurrency: direct DiffusionBackend.load_model callers now publish
the helper/advisor pending marker symmetrically (round 32 P1 #3).
_raise_if_helper_advisor_busy_for_diffusion gains an optional
publish_pending flag; load_model passes True so the destructive
unload window is gated by a "diffusion-backend" tag published
under _HELPER_ADVISOR_START_LOCK. The route layer's "diffusion"
tag and the backend's "diffusion-backend" tag refcount
independently (sum > 0 still blocks helper starts), so neither
side's clear can erase the other's still-active marker. The
existing _release_chat_backend_for_diffusion(check_helper_advisor=
True) path stays snapshot-only (publish_pending defaults False) so
test / direct callers of that helper do not leak a counter.
Validation: export save_directory now rejects ALL ASCII control
characters (round 32 P1, save_directory tab finding). The earlier
CR / LF only guard missed TAB / VT / FF / DEL, which a caller
could smuggle past the export worker's logged subprocess argv.
Documentation: DiffusionLoadRequest.repo_id and base_repo updated
to reflect that local-path support is gated on a Tauri /
frontend load-diffusion-model directory lease producer that has
not shipped yet (round 32 P1 #1 from multiple reviewers). The
backend lease boundary is correct; what is missing is the FE /
native side that mints the matching grant. Until that lands,
local paths through the Images route always 400 with "Native
path grant is required", which the docstring now spells out.
Skipped (consistent with prior rounds):
* Hub-pin findings (R32 P1 #4-#6): live B200 install with
huggingface_hub==0.36.2 + transformers==4.57.6 + diffusers==
0.37.1 verifiably imports Flux2KleinPipeline. Empirical
justification documented in R30 / R30 follow-up commit msgs.
* Tauri / native enum surgery (R32 P1 #1, 6 votes): real
architectural work but out of scope for this PR's Python
surface. Documented now; FE / Rust ticket to follow.
98 targeted backend tests pass (test_diffusion_routes,
test_diffusion_backend, test_inference_model_validation,
test_models_get_model_config_case_resolution, test_data_recipe_seed,
test_training_raw_support, test_export_log_cursor).
Four actionable findings from round 30. Skipped P1 #1 / #2 / #3
(huggingface-hub bump in studio.txt / single-env / colab-new) because
the live B200 Studio that successfully generated FLUX.2 klein images
runs the exact combo the reviewer flags as broken:
huggingface_hub 0.36.2 + transformers 4.57.6 + diffusers 0.37.1
Flux2KleinPipeline: True (imports cleanly)
The is_offline_mode ImportError only fires with transformers 5.x, and
the standard install path pins transformers==4.57.6 via constraints.
The round 26 fix bumped no-torch-runtime.txt + pyproject huggingfacenotorch
where the --no-deps install path can land on transformers 5.x; that
remains the correct surface.
1. core/inference/diffusion.py: preflight transformers + accelerate
via importlib.util.find_spec BEFORE any destructive GPU-owner
unload. Diffusers can expose stub pipeline classes when
transformers / accelerate are missing, so the load used to drop
chat first and fail later inside from_pretrained. find_spec
keeps existing tests that stub these modules passing because no
real module is executed (round 30 P1 #11).
2. models/export.py ExportGGUFRequest.quantization_method: extend
the embedded HF token validator to this field too. Round 23
added the control-char guard but not the token guard; the value
is forwarded into worker command lines and reflected in error /
success text (round 30 P1 #5).
3. models/data_recipe.py SeedInspectUploadRequest: add
_no_control_chars + _reject_embedded_hf_token field_validators
to filename and to each entry of file_names. Mirrors the sibling
SeedInspectRequest.dataset_name hardening (round 30 P1 #6).
4. frontend/src/features/images/images-page.tsx: defer the initial
refreshStatus() call via queueMicrotask so the synchronous
setRefreshingStatus(true) inside it does not trip the
react-hooks/set-state-in-effect lint on mount (round 30 P2 #12).
Deferred (need larger surgery / out of scope for this round):
P1 #4 native_path_lease for diffusion local-path loads
P1 #7-#10 helper/advisor + public-start window mutual lock symmetry
Tests: 98 targeted (diffusion + cached_gguf + inference_validation)
pass locally; frontend npm run typecheck passes.
Five actionable findings from round 29 reviewer aggregate, plus an
origin/main merge that absorbs the chat_templates.py fix landed in
PR #5763. Skipped #4 / #5 (studio.txt + constraints.txt hub bump)
because CI evidence from round 26 contradicts that suggestion; the
real broken combo only happens via the --no-deps no-torch path
which is already bumped in no-torch-runtime.txt + pyproject.toml.
1. core/inference/diffusion.py: round 28 reordered
_release_chat_backend_for_diffusion BEFORE
_release_other_gpu_owners_for_diffusion to surface the helper /
advisor busy check early, but that meant the chat unload inside
_release_chat_backend_for_diffusion now fired before the
training / export conflict check in the second helper. A direct
backend caller (tests, scripts) or a route-precheck race with a
newly-started training run would then unload the user's chat and
then 409 with nothing loaded. Split the helper busy check into
_raise_if_helper_advisor_busy_for_diffusion (cheap, no side
effects), keep _release_chat_backend_for_diffusion as the
actual chat unload with an opt-out flag, and reorder load_model
to: (a) helper check, (b) training / export check + idle export
shutdown, (c) chat unload. All raises now fire BEFORE any
destructive unload.
2. Merge origin/main: absorbs af6504f9 (PR #5763
chat_templates.py find() guards + the new
tests/python/test_construct_chat_template_validation.py
regression test). Removes the 101-line stale-rebase silent
revert that round 29 reviewer 5 and 8 flagged.
3. frontend/src/features/images/images-page.tsx: supportsNegativePrompt
now also honours customFamily when no model is loaded yet, so a
Custom HF repo with family flux.2 / flux.2-klein correctly hides
the negative prompt field instead of silently sending it.
4. routes/inference.py /images/generate: report the ACTUAL PNG
width / height from PIL Image.size instead of echoing back the
requested payload values. FLUX-family pipelines round to
vae_scale_factor * 2, so a request for 520x520 lands as 512x512
internally; metadata now matches the bytes on the wire.
Tests: 98 targeted (diffusion + cached_gguf + inference_validation)
and frontend npm run typecheck pass locally.
Twelve actionable P1/P2 findings from round 28 reviewer aggregate.
Skipped #3 (studio.txt huggingface-hub bump) because the empirical
CI evidence in round 26 contradicts that suggestion: bumping the
pin there breaks installs that apply constraints.txt
(transformers==4.57.6 requires hub<1.0). The actual broken combo
only happens via the --no-deps no-torch path which is already
bumped in no-torch-runtime.txt and pyproject.toml huggingfacenotorch.
1. utils/datasets/llm_assist.py: split _HELPER_ADVISOR_REFCOUNT
into CACHE vs GPU counters. helper_advisor_owns_repo (used by
delete-cache) reads CACHE; helper_advisor_busy (used by public
handoffs) reads GPU. precache_helper_gguf now registers with
gpu_owner=False so a background pre-cache download does not
503 every chat / training / export / diffusion load.
2. utils/datasets/llm_assist.py: introduce _HELPER_ADVISOR_START_LOCK
and wrap the busy precheck + register pair in _run_with_helper
and _run_multi_pass_advisor. Two concurrent helper / advisor
invocations could both pass _gpu_workload_busy_for_helper before
either registered, then OOM each other.
3. utils/datasets/llm_assist.py: _gpu_workload_busy_for_helper now
also returns True when another helper/advisor already holds the
private LlamaCppBackend.
4. routes/inference.py: add _raise_if_helper_advisor_busy(workload)
that 503s when AI Assist owns the GPU. Wire it into both chat
load branches (GGUF + safetensors) BEFORE the existing
_release_export_for / _release_diffusion_for calls so we do not
first tear down an idle export / diffusion just to fail on the
helper check.
5. routes/training.py + routes/export.py + diffusion.load_model:
call the helper-busy check FIRST before any release helper
fires. Mirrors the chat-load ordering.
6. routes/inference.py _release_llama_for: poll
loading_model_identifier for up to 5 s after unload_model() so a
cancelled pending GGUF download has time to clear its
identifier. Mirrors the same wait round 26 added to the explicit
/api/inference/unload route.
7. core/inference/diffusion.py _release_chat_backend_for_diffusion:
same 5 s settling wait for cancelled pending GGUF downloads.
8. models/inference.py LoadRequest: validate every llama_extra_args
entry through _no_control_chars + _reject_embedded_hf_token.
The list was forwarded verbatim to a logged llama-server command
line, so a smuggled control char or hf_... token would land in
logs and subprocess args.
9. routes/models.py /gguf-download-progress: apply
_validate_logged_identifier to repo_id and variant, matching the
round 24 hardening on the adjacent generic /download-progress.
10. routes/inference.py diffusion-load RuntimeError classifier:
treat "AI Assist ..." messages as retryable 503 instead of 400
(round 28 P2 #15). Mirrors the round 18/19 markers for chat
unload failures.
Tests: 105 targeted + 1768 broader backend tests pass locally.
Round 27 findings (Opus parallel concurrency + frontend reviews).
Backend P1 fixes:
1. utils/datasets/llm_assist.py: the round 26 helper/advisor active
registry used a plain set, so two concurrent helper / advisor
loads of the same DEFAULT_HELPER_MODEL_REPO would both
set.add() (no-op the second time) and then the first finally
set.discard() would underflow the registration while the second
call was still mmap'ing the GGUF. Switch to a Counter with
proper refcount increment/decrement so the repo stays registered
until the last user releases it.
2. routes/inference.py _release_chat_for and
core/inference/diffusion.py _release_chat_backend_for_diffusion:
helper/advisor GGUF runs on a PRIVATE LlamaCppBackend (round 26
P1 #1), so the global llama checks below could not see them.
A user-driven /training/start, /export/load-checkpoint, or
/images/load would skip the unload and allocate FLUX VRAM on top
of the helper's resident weights, OOMing on 16-24 GB consumer
GPUs. Both release paths now consult helper_advisor_busy() and
fail 503 (or RuntimeError for the in-backend path) so the user
retries instead of double-owning VRAM.
Frontend P2 fixes:
3. studio/frontend/src/features/images/images-page.tsx: handleUnload
now calls refreshStatus() in the catch path so a partial unload
(503 from the backend) does not leave the UI showing a stale
"Loaded:" label. Matches the handleLoad pattern.
4. images-page.tsx: when status.is_loading is true, auto-poll
refreshStatus every 2 s so the user sees real progress instead
of a frozen "Loading..." label until they manually click Refresh.
5. images-page.tsx: aria-label="Inference steps" / "Guidance scale"
on the two sliders so screen readers can announce them.
6. images-page.tsx: defensive (r.guidance_scale ?? 0).toFixed(1)
in the results caption so a future backend that serialises
NaN/None for guidance does not throw at render.
Tests: 105 targeted (diffusion + cached_gguf + inference_validation)
and 1768 broader backend tests pass locally. Frontend
`npm run typecheck` passes.
P1 #1 + #2 + #6: extended the chat / diffusion / training
identifier hardening to every export-side request model.
ExportCommonOptions (parent of ExportMergedModelRequest /
ExportBaseModelRequest / ExportLoRAAdapterRequest) now applies
_no_control_chars and _reject_embedded_hf_token to repo_id and
base_model_id; ExportGGUFRequest gets the same on its repo_id
plus a control-char check on quantization_method; and
LoadCheckpointRequest validates checkpoint_path. Previously
"/api/export/*" accepted newline-smuggled identifiers and
URL-form ``hf_xxxxx`` tokens that flowed into log lines.
P1 #3 + #4: ``_run_with_helper`` and ``_run_multi_pass_advisor``
now use a shared ``_gpu_workload_busy_for_helper`` that gates on
diffusion (round 22 already), training, AND export. The round 22
guard only checked diffusion, so the dataset helper / advisor
could still load llama-server on top of an active training run
or a resident export checkpoint. Each step fails closed
(unverifiable status counts as busy) so the user's primary
workload is preserved.
P1 #5: PublishDatasetRequest in models/data_recipe.py also
applies the identifier hardening to repo_id; the publish path
previously accepted control characters and URL-form tokens.
P1 #7-10: added _validate_logged_identifier helper to
routes/models.py and applied it to the path / query parameter
endpoints that flow into logger.info(...) calls --
``/config/{model_name}``, ``/check-vision/{model_name}``,
``/check-embedding/{model_name}``, ``/gguf-variants``. Mapped
the validator's ValueError to HTTP 422 so the client sees the
same shape as a Pydantic validation failure.
P2 #11 + #12: ``Loading diffusion model %s`` and
``Diffusion load failed for %s`` log lines route ``repo_id`` /
``effective_base`` through ``_display_repo_id`` (collapses
absolute local paths to the leaf, still scrubs HF tokens)
instead of plain ``_redact_hf_tokens``. The error path was
already collapsed in the user-facing 400 / RuntimeError, but
the structured-log lines kept the full path.
All 97 diffusion + training-validation + related tests pass
locally.
P1 #1: ``TrainingStartRequest.model_name`` now runs the same
control-character and embedded-HF-token validators that the chat
and diffusion request models gained in rounds 5 / 15 / 20 / 21.
``/api/training/start`` previously accepted newline / tab /
control characters and URL-form ``hf_xxxxx`` tokens that flowed
into structured-log sinks via "Loading model %s" lines.
P1 #2: ``_run_with_helper`` in ``utils/datasets/llm_assist.py``
now skips the helper GGUF when the diffusion image backend
reports loaded / loading. The public chat / training / export
routes already do this through ``_release_diffusion_for``, but
this dataset-side helper loaded llama-server directly with no
diffusion guard, so an Images-page allocation would race the
helper for VRAM. New ``_diffusion_image_model_busy`` helper
fails closed (treats status() failure as busy) so the resident
image model is preserved instead of being overwritten.
P1 #3: same ``_diffusion_image_model_busy`` guard added to
``_run_multi_pass_advisor`` (the dataset conversion advisor),
which has the same direct llama.cpp load shape.
P2 #4: the early "Could not infer a diffusion family" RuntimeError
now routes ``repo_id`` through ``_display_repo_id`` before
formatting. A local absolute path that did not match any known
family used to leak the operator's filesystem layout via the 400
response body, last_error, and log line.
All 97 diffusion + training-validation + related tests pass
locally.
P1 #1 + #2: ``LoadRequest._no_embedded_hf_tokens`` and
``ValidateModelRequest._no_embedded_hf_tokens`` now cover
``gguf_variant`` in addition to ``model_path``. A caller could
pass a variant like ``Q4_K_M-hf_xxxxxxxx`` that flowed into
structured log sinks via the GGUF resolver path; the matching
``DiffusionLoadRequest`` validator already covered every string
field, so this restores parity.
P1 #3: ``/api/inference/unload`` now also matches the llama
``loading_model_identifier`` when picking the GGUF branch. A
pending GGUF download (``is_active`` still False,
``loading_model_identifier`` populated) used to fall through to
the safetensors branch and respond ``status="unloaded"`` while
llama-server kept downloading.
P1 #4 + #5: the final safetensors-handoff sweeps (route-level
``_release_safetensors_chat_for`` and backend
``_release_chat_backend_for_diffusion``) now check ``active_model_name``
and ``loading_models`` WITHOUT the initial ``owned_names`` filter.
A concurrent ``/load`` that landed AFTER the snapshot was
previously ignored, so a chat model that began loading during the
unload window let training / export / GGUF chat / diffusion start
anyway and race the new chat for VRAM.
P2 #6: added ``_preflight_diffusers_subfolder_config`` and
invoked it for GGUF loads with a transformer class
(``effective_base``, ``"transformer"``). A custom base companion
that had ``model_index.json`` but lacked
``transformer/config.json`` previously passed the round 19
preflight, unloaded chat, then failed inside
``from_single_file``.
P2 #7: ``_scrub_validation_obj`` in main.py also scrubs string
dict KEYS. Pydantic ``string_type`` errors surface ``input``
verbatim, and a malformed payload like
``{"repo_id": {"hf_xxxxx": "owner/repo"}}`` would otherwise leak
the token through the 422 response body.
All 85 diffusion-relevant + 35 model-validation tests pass
locally. Existing fakes for ``hf_hub_download`` updated to
accept the new ``subfolder=`` kwarg the round 21 preflight uses.
(P1 #3 cross-workload GPU handoff lock from round 20 is still
deferred; round 21's P1 #4 / #5 raised the sweep-level guarantee,
which closes the most common race without the deadlock risk of
holding a process-wide lock across the entire load.)
P1 #1: ``_preflight_full_diffusers_repo(effective_base, hf_token)``
now runs for every load mode, including the GGUF-with-auto-base
path. Round 19 only preflighted the full repo or an explicit
``base_repo``, so an auto-picked companion that turned out to be
gated / private / missing still unloaded the user's chat model
before ``from_pretrained`` failed. ``effective_base`` is the same
value that feeds every downstream allocation, so preflighting it
unconditionally catches all three modes.
P1 #2: ``diffusers.GGUFQuantizationConfig`` (which imports the
``gguf`` package at construction time) is now built up front,
inside the same try block that surfaces "Re-run Studio setup".
Previously the missing-dependency exception fired AFTER
``_release_other_gpu_owners_for_diffusion`` and
``_release_chat_backend_for_diffusion`` had already taken the
chat / export models down. The downstream from_single_file call
reuses the same ``quant_config`` reference.
P1 #4: ``studio/backend/requirements/studio.txt`` now lists
``diffusers>=0.37.0`` and ``gguf>=0.10.0``. These were only in
the extras files, so fresh standard Studio installs failed on
/images/load with the round 20 P1 #2 dependency error message.
P1 #5: ``LoadRequest``, ``UnloadRequest``, and
``ValidateModelRequest`` now apply the same control-character +
embedded-HF-token validators that ``DiffusionLoadRequest``
already had. /api/inference/load, /api/inference/validate, and
/api/inference/unload used to accept newline / tab / control
characters in ``model_path`` (log-line smuggling) and URL-form
``https://hf_xxxxx@huggingface.co/...`` (credential leak through
structured log sinks).
P2 #6: ``_collapse_local`` in the diffusion load-error scrubber
now resolves relative candidates and adds the absolute form to
the substring set. A relative ``exports/my-flux`` used to leak
``/mnt/disks/.../exports/my-flux/...`` via downstream library
errors because the scrubber only matched the original literal.
Replacement is longest-first so a leaf-only context survives.
All 85 diffusion-relevant + 35 related model-validation tests
pass locally.
(P1 #3 cross-workload GPU handoff lock is deferred: deserves a
focused design pass across /images/load, /chat/load (both
branches), /training/start, and /export/load to pick a lock
boundary that does not deadlock against the backend load locks
or stall the SSE log stream.)
P1 #1: ``_release_safetensors_chat_for`` now re-reads
``active_model_name`` and ``loading_models`` after each unload AND
runs a final sweep against the initial owned-name set. The previous
helper trusted ``unload_model() -> True`` even though the
orchestrator can respond ``unloaded`` while still holding weights
or a concurrent ``load`` can repopulate the tracker between calls.
Per-name and global post-state mismatches now raise HTTP 503 so
the caller retries.
P1 #2: same post-state guarantee inside
``_release_chat_backend_for_diffusion`` for direct backend
callers. ``DiffusionBackend.load_model`` now raises RuntimeError
when the safetensors tracker still owns a previously-resident
name after the unload, matching the route-level helper. The route
layer's existing classifier maps the new wording to HTTP 503.
P1 #3: ``DiffusionBackend.load_model`` now preflights the full
diffusers repo (or explicit GGUF ``base_repo``) via
``hf_hub_download(filename="model_index.json")`` BEFORE the
chat / export unload runs. The GGUF path was already covered by
the existing ``hf_hub_download(gguf_filename)`` round-trip; the
full-repo path used to skip validation and let a typo / private /
gated repo only surface inside ``from_pretrained`` AFTER the
user's chat model was already dropped. Local paths are checked
structurally (must be a directory containing ``model_index.json``)
so we do not network-round-trip for an on-disk miss. Error
messages route through ``_display_repo_id`` so an absolute
filesystem path does not leak the operator's layout.
P1 #6: ``/api/inference/unload`` (the direct chat unload endpoint)
now treats ``unload_model() -> False`` AND a leftover state
(``is_loaded`` / ``is_active`` / ``loading_model_identifier`` for
GGUF, ``active_model_name`` / ``loading_models`` for safetensors)
as 503 instead of unconditionally responding
``status="unloaded"``. The UI used to show the model as gone while
the backend still owned VRAM.
P2 #7: extended the /images/load RuntimeError -> HTTPException
marker list with ``still active or loading after unload`` and
``still loading after unload``. Round 18 introduced these exact
phrasings on the backend side; without the extension a retryable
unload failure was returning HTTP 400 to the user instead of 503.
P2 #8: removed the unused ``unsloth_backend = get_inference_backend()``
eager construction in the GGUF chat-load branch. Eager
construction made the GGUF-only path needlessly fail or pay
startup cost when the safetensors backend was unavailable / lazy;
``_release_safetensors_chat_for`` already handles that case as a
no-op.
All 85 diffusion-relevant + 98 related backend tests pass locally.
P1 #1: ``_release_llama_for()`` now verifies ``llama.unload_model``
did not return False AND that ``is_loaded`` / ``is_active`` /
``loading_model_identifier`` are all cleared after the call. The
previous version only treated raised exceptions as failure, so a
subprocess refusing to terminate or an in-flight GGUF download
let the next workload allocate on top.
P1 #2: ``DiffusionBackend._release_other_gpu_owners_for_diffusion``
now raises RuntimeError when ``exp._shutdown_subprocess`` fails on
a settled checkpoint. Direct backend callers used to log at debug
level and proceed toward diffusion allocation while the export
checkpoint still owned VRAM.
P1 #3 + P1 #7: ``/images/load`` no longer drops chat + idle export
before the cheap backend validation runs. ``DiffusionBackend.load_model``
already calls the strict ``_release_other_gpu_owners_for_diffusion``
and ``_release_chat_backend_for_diffusion`` helpers AFTER family
inference and GGUF filename checks pass, so the GPU is still
freed before allocation and a malformed payload no longer
silently unloads the user's chat / chat-export pair.
P1 #4: ``_release_chat_backend_for_diffusion`` now also rejects a
post-unload state where ``loading_model_identifier`` is still set,
matching the route-level ``_release_llama_for`` strictness. A GGUF
download mid-flight before the diffusion handoff used to slip
through and end up double-owning VRAM after diffusion allocated.
P1 #5: ``_release_diffusion_for`` no longer swallows a post-unload
``status()`` failure as ``after = {}``. Training / chat / export
handoffs need proof that the diffusion pipeline released VRAM;
the helper now raises HTTP 503 when the verification status call
itself raises, so the caller retries.
P1 #6: ``DiffusionBackend._release_other_gpu_owners_for_diffusion``
raises RuntimeError when ``get_export_backend()`` itself raises.
Direct backend callers used to silently ``return`` here and
proceed to GPU allocation without being able to verify export
ownership.
P1 #8: ``/training/start`` releases settled export BEFORE chat,
matching the chat-load helpers. If idle export shutdown fails the
user's chat model is preserved instead of being dropped for a
training run that never starts.
P2 #9: GGUF load-error scrubber also collapses ``local_gguf_path``,
the resolved HF cache path passed to
``transformer_cls.from_single_file()``. Without this an exception
like ``OSError: cannot load /home/alice/.cache/huggingface/.../flux.gguf``
would leak the operator's filesystem layout through ``last_error``
and ``/images/status``.
All 85 diffusion-relevant backend tests pass locally.
P1: route-layer chat/diffusion/export releases were still
asymmetric. Training start and export load called
``diff_backend.unload_model`` inside a best-effort try/except so a
wedged diffusion backend let the next workload allocate over the
top of the resident pipeline and OOM. Both now use the strict
``_release_diffusion_for`` helper from routes.inference, which
raises HTTPException 503 on status/unload failure or post-check
mismatch.
P2 #9: diffusion load exceptions can include the absolute local
repo / base / gguf path verbatim (FileNotFoundError, OSError from
diffusers / safetensors). The path flows into ``_last_error``,
which ``status()`` returns to every authenticated session. Collapse
the known repo_id / effective_base / gguf_filename paths to their
leaf name before storing the error, mirroring the
``_display_repo_id`` convention used for the public repo label.
P2 #10: when ``repo_id`` is an absolute local path,
``detect_family`` matched _FAMILY_EXCLUDE deny lists against the
full path, so models stored under a parent directory containing
``qwen-image-edit`` or ``3.5`` were misclassified as None. Reduce
the family-detection needle to the leaf directory when the input
looks like a filesystem path; Hub-style ``owner/repo`` ids
continue to use the original needle so existing detection rules
keep working.
P2 #12: ``gguf_filename`` was missing from the
``_reject_embedded_hf_token`` validator. A URL-form quant path
like ``https://hf_xxxxx@huggingface.co/.../flux.gguf`` would be
stored on ``DiffusionBackend._gguf_filename`` and surface in
status() / log lines. Extend the validator to gguf_filename so the
token is dropped before it can leak.
All 85 diffusion-relevant backend tests pass locally.
Round 16 reviewer aggregate (logs/review_round16_aggregate.md):
P1 fixes:
- routes/models.py /delete-cached llama guard pairs loading_id with
loading_hf_variant so deleting a different cached quant (Q8_0)
while another variant (Q4_K_M) is loading is no longer blocked.
- core/inference/diffusion.py load_model now calls
_release_other_gpu_owners_for_diffusion BEFORE
_release_chat_backend_for_diffusion. The other-owners helper
RAISES on active training/export, so a route -> worker race or
direct backend caller no longer drops the user's chat model
before the diffusion load is refused.
- routes/models.py /delete-cached diffusion guard fails CLOSED
(503) on HF cache scan failure instead of silently falling
through to repo-id-only matching, which could miss a loaded
local snapshot path.
- routes/inference.py _release_llama_for and
_release_safetensors_chat_for now raise 503 on actual unload
failure (exception or False return), so new GPU workloads do
not start while the old chat process still owns VRAM.
- core/inference/diffusion.py status() now takes
include_internal=False by default and only exposes the
guard-facing active_*/pending_* paths when callers opt in. The
public /api/inference/images/status route gets the redacted
payload; routes/models.py delete guards pass
include_internal=True so they still see the raw paths.
- core/inference/diffusion.py generate_image_with_metadata routes
the response model through _display_repo_id so /images/generate
cannot echo back an absolute local path.
P2 fixes:
- routes/inference.py /images/load now maps backend "Could not
verify training/export status" to 503 instead of 409, matching
the route-level pre-check.
- core/inference/diffusion.py _release_other_gpu_owners_for_diffusion
raises "Could not verify export status" when the
is_export_active() probe itself raises, instead of silently
treating it as active export.
- core/inference/diffusion.py detect_family compares compact family
spellings (Flux2Klein) against per-token compact strings so
unsloth/Flux2Klein-GGUF matches the flux.2-klein family without
matching the embedded substring inside flux.20.
- main.py installs a RequestValidationError handler that scrubs
hf_xxxxx tokens out of the 422 response body so a rejected
``repo_id`` containing a URL-embedded HF token does not echo it
back to the browser.
Tests:
- 3 new regression cases (Flux2Klein compact alias, public status
redaction, generate_image_with_metadata redaction).
- All 75 diffusion backend + route tests pass.
Round 15 reviewer aggregate (logs/review_round15_aggregate.md):
P1 fixes:
- core/inference/llama_cpp.py publishes loading_model_identifier +
loading_hf_variant AFTER acquiring _serial_load_lock; previously
a queued second load could overwrite or clear the identifier
currently in flight, breaking delete-safety and GPU handoff guards.
- routes/models.py /delete-finetuned compares the pending llama
load against loading_hf_variant (new), not the stale hf_variant
from the previous loaded model. Without this, a Q4-loaded
directory loading Q8 would still accept a Q8 delete.
- core/inference/diffusion.py _release_other_gpu_owners_for_diffusion
now also raises when training is active so direct backend callers
cannot bypass the route layer's 409 guard. Mirrors the
export-active check the same helper already enforces.
- routes/models.py /delete-cached diffusion guard compares owned
diffusion paths against the HF cache root for the target repo
via _all_hf_cache_scans + _is_path_under. Without this, loading
from a local models--owner--model/snapshots/<sha> path let the
cache delete proceed while the snapshot was still mmap'd.
- models/inference.py DiffusionLoadRequest refuses URL-embedded
hf_xxxxx tokens in repo_id / base_repo at the API boundary, so
the value never reaches self._repo_id and status() can never
echo it back to other authenticated sessions.
P2 fixes:
- core/inference/diffusion.py status() routes UI-facing repo_id /
base_repo through _display_repo_id, which collapses absolute
local paths to the leaf name (delete guards still see the full
path via active_*/pending_*).
- routes/inference.py /images/load maps backend RuntimeError that
reports an export/training conflict to HTTP 409 instead of 400.
- core/inference/diffusion.py detect_family now uses token-boundary
matching so owner/flux.20-model does not collide with flux.2.
P3 fixes:
- tests/test_diffusion_routes.py drops the partial routes.inference
module from sys.modules if exec_module() raises, so the real
ImportError surfaces instead of a misleading AttributeError on
follow-up tests.
Tests:
- 5 new regression cases (display_repo_id, token-boundary family
detection, training-active raise from backend helper, embedded HF
token rejection).
- All 72 diffusion backend + route tests pass.
Round 14 reviewer aggregate (logs/review_round14_aggregate.md):
P1 fixes:
- routes/export.py /load-checkpoint now runs the active-export 409
guard BEFORE the chat / diffusion unloads, so a rejected request
no longer tears down unrelated GPU state.
- core/inference/llama_cpp.py wraps the WHOLE load_model body in a
single try/finally that publishes loading_model_identifier across
download, metadata read, VRAM settle, process spawn, and health
check. Done via a thin load_model wrapper around the existing
body (renamed _load_model_impl) to avoid reindenting hundreds of
lines.
- routes/models.py /delete-finetuned now checks
loading_model_identifier so a pending HF GGUF download cannot
have its destination directory rmtree'd before llama-server
spawns.
- core/inference/diffusion.py stores the original caller-supplied
gguf_filename (e.g. ``BF16/model.gguf``) in a new self._gguf_filename
field and exposes it as active_gguf_filename. UI-facing
gguf_filename still collapses to basename for the panel.
- routes/models.py /delete-cached llama guard now allows safe
different-variant deletes when hf_variant differs, matching the
diffusion path's variant-aware behaviour.
- core/inference/diffusion.py tracks self._cpu_offload_enabled and
forces a CPU torch.Generator when offload is on, so seeded
generation no longer crashes on CUDA hosts with the default offload
enabled.
P2 fixes:
- core/inference/diffusion.py detect_family normalises mixed
separators (``Qwen_Image-Edit-GGUF``, ``Qwen-Image_Edit-GGUF``,
``QwenImageEdit-GGUF``) so every Qwen-Image-Edit spelling is
excluded from the base Qwen-Image family.
- core/inference/diffusion.py logger.info / logger.error in
load_model run repo_id and effective_base through _redact_hf_tokens
so URL-embedded ``hf_xxxxx`` tokens never reach structured-log
sinks.
- core/inference/diffusion.py _release_other_gpu_owners_for_diffusion
now raises RuntimeError when an export job is active instead of
logging and continuing, so direct backend callers cannot bypass
the route layer's 409 guard.
- core/inference/diffusion.py full-diffusers repo / base_repo paths
expand ``~`` via _expand_existing_local_path so
``repo_id="~/models/my-flux"`` no longer falls through to the Hub.
Tests:
- 5 new regression cases (mixed Qwen-Image-Edit separators, token
redaction, status full-filename, CPU offload generator device,
staging Windows leaf already-set sanity).
- All 68 diffusion backend + route tests pass.
Round 13 follow-up: on Windows Path('/etc/passwd').is_absolute()
returns False because POSIX absolute paths read as drive-relative,
which let the traversal check fall through to resolve(strict=True)
and crash with a raw FileNotFoundError instead of the friendlier
RuntimeError. Add a PurePosixPath check + explicit leading-separator
guard and wrap the resolve() in try/except so a missing path inside
the chosen repo is reported as 'Local repo path does not contain ...'
on every OS.
Pre-existing 59 diffusion backend + route tests still pass; staging
Windows Diffusion CI was failing on this exact case.
Round 13 reviewer aggregate (logs/review_round13_aggregate.md):
P1 fixes:
- routes/export.py load_checkpoint refuses (409) when an export job
is currently active, mirroring the chat/diffusion/training handoff
guards. ``is_export_active`` absence is tolerated for older / mocked
backends.
- core/inference/diffusion.py local-path GGUF loader now accepts
relative directories (Studio exports surface as ``exports/my-flux``)
and confines ``gguf_filename`` to the chosen repo via
``_resolve_local_gguf_child``: absolute filenames, ``..`` segments,
and Windows separators are rejected before any file is opened.
- core/inference/diffusion.py status() exposes ``active_gguf_filename``
alongside the pending variant so delete guards can pair each owned
repo with the GGUF variant it actually owns.
- routes/models.py cache delete + finetuned delete adopt a shared
``_diffusion_owned_targets`` + ``_variant_delete_is_safe_for_owned_gguf``
helper. Per-variant deletes during a swap-in-flight cannot remove
the active variant while the pending variant is loading.
- core/inference/llama_cpp.py publishes ``loading_model_identifier``
before ``_download_gguf`` starts and clears it in ``finally``. Cache
delete (routes/models.py) and the cross-workload release helpers
(routes/inference.py::_release_llama_for and
diffusion.py::_release_chat_backend_for_diffusion) consult it so a
multi-GB HF download cannot be rmtree'd or be ignored by /images/load
while still in flight.
P2 fixes:
- core/inference/diffusion.py adds
``generate_image_with_metadata`` + ``async_generate_with_metadata``;
/images/generate uses it so the response model/family reflect the
pipeline that actually produced the image even if an unload races
the route.
- core/inference/diffusion.py: ``base_repo`` only applies when picking
a GGUF quant. Filling Base diffusers repo while loading a full
diffusers repo no longer silently swaps the load target.
- core/inference/diffusion.py: failed device placement / offload now
drops pipe + transformer references explicitly before drain so
partial allocations cannot keep VRAM around.
- core/inference/diffusion.py: torch/diffusers imports surface as a
clear RuntimeError naming the missing dependency.
- core/inference/diffusion.py: _smart_base_repo splits on both POSIX
and Windows separators so ``C:\\Users\\me\\base\\FLUX.2-klein-4B-GGUF``
no longer picks the Base 4B variant via the parent dir.
Tests:
- 6 new regression cases (Windows leaf, traversal/backslash rejection,
relative-dir local load, metadata snapshot, lock serialisation).
- All 59 diffusion backend + route tests pass.
Round 12 reviewer findings.
Backend correctness (P1)
* core/inference/diffusion.py load_model: GGUF branch now
handles an absolute local directory passed as repo_id by
joining Path(repo_id) / gguf_filename directly instead of
handing the path to hf_hub_download (which raises
HFValidationError because the path is not 'namespace/repo').
Closes round 12 review #1 -- the load request advertised
'local path' support but actually only worked for Hub repo ids.
Delete guard precision (P1)
* routes/models.py /delete-finetuned + /delete-cached:
diffusion guard now consults gguf_filename from status()
and ALLOWS per-variant deletes that target a different quant
than the one the loaded pipeline is reading. Loading
'Q4_K_S' no longer blocks deleting 'Q8_0' from the same
repo / export directory (round 12 reviews #3 and #4).
Accelerator (P2)
* core/inference/diffusion.py _drain_cuda_cache: also calls
torch.mps.empty_cache() when the MPS backend is the
active accelerator. Apple Silicon swaps now actually return
held VRAM instead of leaving it pinned in the Metal
allocator (round 12 review #10).
Smart base repo (P2)
* core/inference/diffusion.py _smart_base_repo: only inspects
the LAST segment of the repo id / path for the 'base' / '9b'
tokens. A namespace like baseorg/FLUX.2-klein-4B-GGUF or
a parent directory like /home/me/.cache/base/... no
longer falsely selects the Base variant (round 12 review #9).
Round 11 reviewer findings.
Backend lifecycle (P1)
* core/inference/diffusion.py _release_other_gpu_owners_for_
diffusion: now re-checks is_export_active() locally before
calling _shutdown_subprocess. The route layer already 409s on
active exports, but defence-in-depth means direct backend
callers (tests, scripts, future routes that forget the
higher-level guard) can no longer terminate an in-flight
export and corrupt the user's partial output.
* routes/inference.py standard chat-load path: the duplicate
inline 'if exp_backend.current_checkpoint -> _shutdown_subprocess'
block was removed. _release_export_for above already handles
settled checkpoints and skips active ones; the inline block
was the round 11 #2 asymmetric fix surface.
Routing / error mapping (P2)
* routes/training.py start_training: except HTTPException:
raise was inserted before the broad except Exception:
handler so the 409 raised by _raise_if_training_active /
_raise_if_export_active reaches the client intact instead of
being swallowed into a 500.
State publishing (P2)
* core/inference/diffusion.py load_model: success path now
clears _loading + _pending_* under _lock BEFORE returning
self.status(), so the response payload reports the resident
pipeline cleanly (no stale is_loading=true / pending_*). The
finally block remains idempotent for error / early-raise paths.
* core/inference/diffusion.py status(): nulls family /
pipeline_class while a swap is in flight (pending_repo set
and != active_repo). Previously the response paired pending
model B's repo_id with model A's family, producing a
combination that never existed.
Validation
* models/inference.py: DiffusionLoadRequest.repo_id and
base_repo length caps bumped from 256 to 1024; gguf_filename
bumped from 256 to 512. The earlier caps rejected realistic
Studio export paths (deeply nested outputs / exports
directories, especially on Windows).
Dependencies
* pyproject.toml huggingfacenotorch + studio/backend/
requirements/no-torch-runtime.txt: floor gguf at >=0.10.0
to match the diffusers requirement. Unconstrained pin allowed
a resolver to install older gguf releases that raise at
single-file load time.
Round 9 reviewer flagged a pile of handoff asymmetries: every
GPU-owning lifecycle change (training, export, chat, images) needed
its own bespoke unload sequence and they had drifted out of sync.
Some skipped llama-server is_active; some missed safetensors
loading_models; export and training did not check is_export_active.
Backend handoff (P1)
* routes/inference.py: new _release_chat_for / _release_export_for
helpers. Both treat llama-server as held when is_loaded OR
is_active, safetensors as held when active_model_name OR
loading_models is non-empty, and export as held when
current_checkpoint OR is_export_active. Both helpers run their
unloads in worker threads so async routes do not block the
event loop.
* routes/training.py: replaces its bespoke inline llama / safe /
export unload sequence with await _release_chat_for / _release_
export_for.
* routes/export.py: same swap for the chat unload chain (export
still does NOT call _release_export_for on itself).
* routes/inference.py GGUF + standard chat-load paths: now use
_release_export_for to drop a settled export, and the standard
path's llama unload now also handles is_active=True (round 9
review #8).
Backend reject-on-active export (P1 #5)
* routes/inference.py: new _raise_if_export_active. Symmetric
with _raise_if_training_active: a long-running export is
refused with HTTP 409 instead of being silently killed when
/images/load or /load arrives. Diffusion / images load and
both chat-load paths call it.
* core/inference/diffusion.py _release_other_gpu_owners_for_
diffusion: no longer tears down an in-flight export job. Only
drops a SETTLED export checkpoint (current_checkpoint
populated, is_export_active False). Round 9 review #5 -- the
previous behavior could terminate an in-flight export and
leave a partial output artifact.
Token leak via logger.exception (P1 #6)
* core/inference/diffusion.py: load-failure logging now uses
logger.error(..., exc_msg) with the already-scrubbed string
and exc_info=False. logger.exception() with the raw Exception
would expose any hf_... token that diffusers / huggingface_hub
embedded in the message or traceback locals, defeating the
earlier in-flight scrub.
Dependency pinning (P1 #11)
* pyproject.toml: huggingfacenotorch optional extra now pins
diffusers>=0.37.0. Previously the floor was only set in
studio/backend/requirements/no-torch-runtime.txt, so a normal
pip install would resolve diffusers 0.36.0 (no
Flux2KleinPipeline) and the default curated FLUX.2 klein
Images model would fail at runtime.
Cache-delete exact match (P1 #14)
* routes/models.py /delete-cached: llama.cpp and safetensors
guards now match on exact repo-id (case-insensitive) instead
of prefix. Diffusion guard already does this; the chat guards
were the remaining surface where loading org/model-v2
blocked deleting org/model.
Round 8 reviewer surfaced event-loop stalls (blocking unload from
async routes), incomplete VRAM handoff coverage (is_active /
loading_models / is_export_active not checked), token leaks via
exception messages, /v1 exposure, and several fail-open paths.
Async / event-loop
* routes/inference.py /images/unload, GGUF chat-load handoff,
safetensors chat-load handoff: blocking DiffusionBackend.unload
pushed onto asyncio.to_thread. unload takes _load_lock +
_generate_lock and can block for the full duration of an
in-flight load / generation, which was freezing the FastAPI
worker, SSE stream, and hardware poller for minutes.
* routes/export.py + routes/training.py: same to_thread wrap on
diffusion unload during checkpoint / training start.
GPU-owner handoff completeness
* core/inference/diffusion.py _release_chat_backend_for_diffusion:
llama-server now also unloaded when is_active=True (mid-download
/ startup), not only when is_loaded; flushed in-flight
safetensors loads from loading_models too.
* core/inference/diffusion.py _release_other_gpu_owners_for_
diffusion: export shutdown now also fires when
is_export_active() returns True (checkpoint not yet assigned).
Security / scrubbing
* core/inference/diffusion.py: load failure paths now scrub
hf_token from both _last_error AND the raised RuntimeError
message (the previous scrub only cleared frame locals).
Falls back to a regex strip of hf_[A-Za-z0-9]{20,} to
catch tokens that came in via huggingface_hub default caching.
* routes/inference.py: image lifecycle endpoints moved from
router to studio_router so they no longer answer under
the /v1 OpenAI-compat prefix. Studio-only side effects
(download multi-GB GGUFs, unload chat, etc.) should not be
reachable via an OpenAI-compat client.
* models/inference.py: control-char validator now also rejects
tab. Some log sinks split fields on tab; allowing it left a
log-injection surface.
Fail-closed delete guards
* routes/models.py /delete-cached: llama.cpp and safetensors
branches now fail closed with 503 when their status check
raises (matches the diffusion-side guard added earlier).
* routes/export.py: split the try/except around the training
backend so import failure falls back to 'skip' (no
core.training in this build) while a runtime failure of
get_training_backend()/is_training_active() fails closed.
* routes/models.py /delete-finetuned: diffusion guard now also
compares against relative path candidates (Path.resolve() works
on relative input). Previously a load with a relative repo_id
bypassed the guard.
CUDA cleanup ordering
* core/inference/diffusion.py: split _release() (drops local +
gc.collect) from _drain_cuda_cache() (torch.cuda.empty_cache).
Callers now drain AFTER nulling every reference so the
allocator actually reclaims the freed slabs (previously
empty_cache ran while caller still held a local, which left
the cache pinned).
Generate response (P2 #16)
* routes/inference.py: response uses status()['active_repo_id']
instead of the UI-facing repo_id, so a queued /images/load
promoting a pending model cannot mislabel the just-rendered
image with the new model's identity.
Test wiring
* tests/test_diffusion_routes.py: mount inf.studio_router on the
test app so /images/* routes are reachable now that they live
on the Studio-only router.
Round 7 reviewer surfaced a handful of swap-window races, fail-open
guards, and seed precision mismatches. This commit closes them.
Lifecycle / state (P1)
* core/inference/diffusion.py: status() now emits active_repo_id,
active_base_repo, pending_repo_id, pending_base_repo, and
pending_gguf_filename alongside the existing UI-facing fields.
During a swap (model A loaded, model B loading) the previous
coalesced 'repo_id or pending_repo_id' hid the loading target
from delete guards. Splitting the fields lets guards block
deletion of either repo currently owned by the backend.
* core/inference/diffusion.py: generate_image() now takes
_generate_lock BEFORE snapshotting _pipe / _device. Snapshotting
outside the lock let a concurrent unload/load clear or replace
the backend between the snapshot and the forward, so the freed
or swapped pipeline would still run.
Symmetric handoffs (P1)
* routes/export.py: training-active check now runs BEFORE the
chat / inference / diffusion unload helpers, so a 409 does not
leave the user's chat session torn down for nothing. Also
explicitly fails CLOSED with 503 when is_training_active()
raises.
* routes/inference.py: _raise_if_training_active now fails closed
with 503 when the training backend is importable but its status
check raises. The previous best-effort log-and-continue could
let chat / diffusion loads collide with unverifiable training.
Delete guards (P1)
* routes/models.py /delete-cached: chat guard now also blocks
when llama-server is_active (i.e. mid-download) and when the
inference backend's loading_models set contains the target.
Round 7 review #7 flagged that the PR's diffusion-side loading
guard had no chat-side parallel, so deleting a chat repo while
it was downloading could still race the cache.
* routes/models.py /delete-cached: diffusion guard iterates the
new active_* + pending_* status fields so a delete during a
swap is refused on either repo.
* routes/models.py /delete-finetuned: same active_+ pending
handling, plus the guard now also refuses deletes of a parent
directory that contains the loaded pipeline (round 7 review #6:
rm -rf /exports/flux-model/ could unlink model_index.json that
the live pipeline is reading via mmap).
Seed precision (P2)
* models/inference.py + routes/inference.py: DiffusionGenerate-
Response now carries seed_str alongside the existing numeric
seed. Seeds above Number.MAX_SAFE_INTEGER are rounded by
JSON.parse in the browser; seed_str ships full decimal
precision for display and reproduction.
* frontend/api.ts: DiffusionGenerateResponse types seed_str;
images-page.tsx prefers seed_str over seed in the figure
caption so the displayed value reproduces the image.
* frontend/api.ts: stringifyWithBigInt no longer regex-replaces
sentinel strings over the full JSON output. It pulls the seed
BigInt out, JSON-serialises the remaining payload, and splices
the seed's decimal digits into the resulting object literal at
the known position. Avoids the round 7 #10 case where a
user-supplied prompt equal to '__bigint__:123' was rewritten
into a JSON integer and rejected as a non-string prompt.
Custom HF repo (P2)
* frontend/images-page.tsx: custom panel now exposes a 'Base
diffusers repo' input that maps to DiffusionLoadRequest.
base_repo. Required when a private / mirrored GGUF needs a
non-default base (e.g. a 9B Klein transformer would otherwise
fall back to the 4B base default).
Round 6 reviewers identified several races between load / unload /
generate and several fail-open delete guards. This commit closes
them by widening the lock scope, publishing the pending load
target through status(), and switching delete guards to
fail-closed.
Lifecycle (P1)
* core/inference/diffusion.py: load_model now also takes
_generate_lock. Previous behavior released and reallocated the
pipeline while a generation forward was still iterating
denoising steps, corrupting scheduler state and stacking VRAM.
The forward only briefly touches _lock, so taking it on the
load path does not introduce a deadlock.
* core/inference/diffusion.py: unload_model now also takes
_generate_lock. Without it, /images/unload returned
is_loaded=False while a slow forward was still running, which
let chat / training / export handoffs allocate VRAM on top of
the still-resident pipeline.
* core/inference/diffusion.py: previous pipeline release now
happens BEFORE from_single_file / from_pretrained. Switching
FLUX.2 klein 4B -> 9B on a 16-24 GB GPU was failing because
the new transformer allocation overlapped the old pipe's
residency.
* core/inference/diffusion.py: failed pipeline from_pretrained
now explicitly releases the just-loaded transformer; previously
its weights stayed pinned to GPU until GC and made the next
load more likely to OOM.
Pending-target / delete guards (P1)
* core/inference/diffusion.py: load_model now publishes
_pending_repo_id / _pending_base_repo / _pending_gguf_filename
under _lock at the start of the call (and refreshes
_pending_base_repo when the smart-base / repo defaults resolve).
status() exposes those as 'repo_id' / 'base_repo' /
'gguf_filename' during is_loading=True so delete guards can see
the target before _repo_id is set on success.
* routes/models.py /delete-cached + /delete-finetuned: diffusion
status check now fails CLOSED (HTTP 503) when status() raises.
Both guards previously logged and continued, which could let a
delete proceed against a repo whose status was unverifiable.
* routes/models.py: is_loading is also blocked on both guards
so a mid-download / mid-from_pretrained rmtree is refused.
Symmetric handoffs (P1)
* routes/export.py: /load-checkpoint now refuses with HTTP 409
when training is active instead of calling stop_training().
Chat and /images/load did the same after round 5; export was
the remaining asymmetry that would silently kill a long
training run.
* routes/training.py, routes/inference.py (GGUF and standard
chat), routes/export.py: diffusion handoff now treats
is_loading as is_loaded. The diffusion backend's unload waits
on _load_lock + _generate_lock so an in-flight load completes
first.
Requirements (P1)
* requirements/studio.txt: pin python-multipart explicitly. The
Studio routes package's eager router imports include
routes/datasets.py whose FastAPI UploadFile/File validation
crashes with RuntimeError without it in fresh test envs.
Frontend (P2)
* features/images/api.ts + images-page.tsx: seed handling now
accepts the full [-2^63, 2^64 - 1] range via BigInt. The
previous safe-integer cap rejected valid uint64 seeds the
backend accepts. A small stringify helper emits BigInts as JSON
integers without touching the rest of the payload.
Tests
* test_diffusion_routes.py: load routes/inference.py via
importlib.spec_from_file_location to avoid triggering
routes/__init__.py (which would pull in training / datasets /
data_recipe imports unrelated to diffusion tests).
* test_diffusion_backend.py: status() during is_loading shows
pending repo + base; unload waits for in-flight generation.
Round 5 reviewer findings, mostly symmetric-lifecycle and input
validation gaps the earlier rounds left open.
Backend lifecycle (P1)
* routes/training.py: training start now also unloads the GGUF
llama-server subprocess; was previously only unloading the
safetensors backend, so starting training while a GGUF chat
model was loaded kept the subprocess pinned to VRAM.
* routes/inference.py: new _raise_if_training_active helper. Both
GGUF and standard chat loads, plus /api/inference/images/load,
now refuse with HTTP 409 when training is active instead of
silently stopping training to free VRAM.
* core/inference/diffusion.py: _release_other_gpu_owners_for_
diffusion no longer stops active training. The route layer
refuses the request first, so reaching the helper with training
live would only happen from programmatic backend calls; better
to surface OOM than terminate a long training run.
* core/inference/diffusion.py: BF16 dtype is now gated on
torch.cuda.is_bf16_supported. Pascal/Turing GPUs report
is_available()=True but lack BF16 ALUs; FLUX kernels then fail
inside from_pretrained. Falls back to FP16 instead of refusing.
* core/inference/diffusion.py: GGUF transformer allocation and
pipeline allocation now run AFTER releasing chat/export GPU
owners; previously from_single_file ran first and could OOM
before the intended VRAM handoff happened.
* routes/models.py: /delete-cached now also blocks delete when
diffusion is_loading=True (not just is_loaded); concurrent
delete during hf_hub_download / from_single_file would have
raced the rmtree.
* routes/models.py: /delete-finetuned now also checks the
diffusion backend before unlinking a Studio outputs/exports
path. A user who exported a FLUX LoRA locally and loaded it via
/images/load could previously rmtree the directory the
diffusion backend was reading from.
Backend correctness / safety (P2)
* core/inference/diffusion.py: _FAMILY_EXCLUDE for qwen-image now
also covers qwen_image_edit / qwenimageedit underscore spellings
so '...qwen_image_edit-GGUF' no longer misdetects as Qwen-Image.
* core/inference/diffusion.py: detect_family now scans
_FULL_REPO_FAMILIES in addition to _FAMILIES, so SDXL repos
(stabilityai/stable-diffusion-xl-base-1.0) are auto-detected
instead of failing with 'Could not infer a diffusion family'.
* core/inference/diffusion.py: generate_image now uses a separate
_generate_lock for the pipeline forward instead of holding
_lock for the whole call. status() polls and concurrent unload
requests no longer block for the full minutes-long generation.
* routes/models.py: diffusion delete guard now uses exact repo-id
match instead of prefix match; previously loading 'org/model-v2'
would block deleting unrelated cached 'org/model'.
* models/inference.py: DiffusionLoadRequest now rejects ASCII
control characters in repo_id / gguf_filename / base_repo /
family via field_validator (closes log-injection surface from
authenticated callers). Also caps lengths at 256 chars.
* models/inference.py: DiffusionGenerateRequest seed is now
bounded to the int64/uint64 range; previously a huge seed
(e.g. 2**100) passed Pydantic then crashed inside
torch.Generator.manual_seed with 'Overflow when unpacking long
long'.
Frontend (P2)
* features/images/images-page.tsx: Custom HF repo panel now
exposes a Pipeline family override dropdown; previously the
backend supported it via DiffusionLoadRequest.family but the UI
had no way to send it, so custom repos whose names did not
contain a hard-coded substring failed to load.
* features/images/images-page.tsx: handleLoad now re-fetches
status on error. The backend clears its old pipeline before
allocating the replacement; a failed swap previously left the
UI showing 'Loaded:' with Generate enabled until manual
refresh.
Tests (10 new)
* underscore qwen-image-edit exclusion + SDXL full-repo detection
* BF16 fallback when is_bf16_supported() returns False
* status() does not block while generate_image holds _generate_lock
* route layer rejects control chars in repo_id
* route layer rejects 2**100 seeds (uint64-max boundary accepted)
* route layer happy-path with negative-prompt true_cfg_scale
forwarding (Qwen/Flux) and skip-when-no-neg (distilled CFG)
QwenImagePipeline and FluxPipeline treat guidance_scale as the
distilled CFG factor and expose true_cfg_scale as the real
classifier-free guidance knob. Negative prompts only steer the
output when true_cfg_scale > 1, so forwarding only guidance_scale
left Qwen-Image on the default true_cfg_scale=4.0 and the user's
slider value silently ineffective for negative prompts.
When the loaded pipeline accepts both negative_prompt and
true_cfg_scale and the caller supplies a non-empty negative
prompt, forward guidance_scale through both kwargs so the
negative prompt actually steers generation. When no negative
prompt is supplied, true_cfg_scale is left at the model default
to avoid switching distilled CFG models into real-CFG mode (which
would double inference cost and degrade quality).
Adds two regression tests covering the forward-when-negative and
skip-when-no-negative paths.
