Commit Graph
13 Commits
Author SHA1 Message Date
Jason Hoku 31aa5dc692 fix: upscale_runner — fall back to item.model when meta.model is empty
The previous fix correctly identified that meta.model was empty (printing
'Checkpoint (empty) not found'), but the actual right behavior isn't to
error out — it's to fall back to a per-item model. Items always store
their own model name, so even if meta.model wasn't recorded for whatever
reason (legacy manifest, write race, older session format), the items
themselves know.

Scan target_items for the first non-empty model name, use that as the
session base. Florence2's from_manifest mode would handle per-item model
differences on top anyway; this just gives us a valid starting point.

The 'checkpoint not found' error message is preserved for the case where
the picked model genuinely isn't on disk — that's still an actionable
user-facing problem.
2026-05-18 23:55:34 -07:00
Jason Hoku 5bf3a56e27 fix: detect missing checkpoint file and surface clear error
User report on a different machine: 'AttributeError: NoneType has no attribute
lower' at comfy/utils.py:126 inside load_torch_file. Root cause: manifest's
meta.model references a checkpoint that isn't installed on this machine —
folder_paths.get_full_path returns None, that None propagates through
load_checkpoint_guess_config to load_torch_file, crashes cryptically.

Two defensive checks:

1. upscale_runner._run_upscale_thread: validates the session checkpoint
   resolves to a real file BEFORE calling load_checkpoint. If not, sets
   job['error'] with a clear actionable message ('Copy the .safetensors
   file over, or regenerate the images on this machine') and exits early.

2. florence2_hires._get_or_load_checkpoint_lora: same check before the
   fresh-load path (different per-item model). On missing file, falls back
   to the session base with a one-line log so the upscale still completes
   instead of crashing the whole job.

Tests: added folder_paths.get_full_path to the root conftest stub returning
a synthetic path so mocked-loader tests don't trip the new file-check.
96/96 still passing.
2026-05-18 16:00:34 -07:00
Jason Hoku 97d46fb51a fix: dashboard florence2 OOM — keep_model_loaded=False + pre-load VRAM hygiene
User insight: 'why didn't this happen in the Builder UI inline path?' revealed
the actual root cause. Inline path runs Florence2 inside the active workflow
(no stale-model accumulation possible). Dashboard upscale is a separate
background thread that calls load_checkpoint() on top of whatever the prior
generation left in VRAM — ComfyUI's model_management doesn't auto-evict those
prior handles because Python refs keep them alive.

So even with my same-model short-circuit fix, the dashboard path stacks:
  prior_gen_model + new_session_model + Florence2 + KSampler activations
= OOM on 8 GiB cards.

Two changes:

1. florence2_hires.py: Florence2Run now uses keep_model_loaded=False. After
   each detection Florence2 moves to CPU; my _FLORENCE2_MODEL_CACHE still
   holds the Python ref so the next call moves it back to GPU automatically
   (~1-2s, cheap). Florence2 stops competing with KSampler and the session
   checkpoint for VRAM during the inpaint stage.

2. upscale_runner.py: at the START of _run_upscale_thread, before
   load_checkpoint, run gc.collect() + soft_empty_cache() + torch.cuda.empty_cache().
   This forces ComfyUI to evict prior-generation model handles that no longer
   have active references but weren't LRU-evicted yet. Inline path didn't
   need this because Comfy knew its model was actively in use.
2026-05-18 12:43:49 -07:00
Jason Hoku d4b88b0420 fix: florence2 — avoid duplicate model load + soft_empty_cache between items
User report: 'Allocation on device 0 would exceed allowed memory' OOM mid
Florence2 beam search on a 2-image batch with 8 GiB VRAM. Currently allocated
15.5 GiB on what should be a 3-5 GiB workload.

Root cause: dashboard upscale_runner ALREADY loaded meta.model into VRAM as
its session base. My from_manifest path then called load_checkpoint() for
item.model — same checkpoint name, but a SECOND full copy in VRAM (a fresh
ModelPatcher with its own UNet+CLIP+VAE weights). For SDXL that's ~7 GiB
duplicated. Plus the Florence2 model on top.

Fix:
- Pass session_model_name from upscale_runner / orchestrator into Florence2
- _get_or_load_checkpoint_lora short-circuits when item.model == session_model_name:
  reuse the session's base handles and just apply the item's LoRAs on top
  (load_loras_for_preencoding is a cheap patch, no weight reload)
- Different model name → still does a fresh full load (correct behavior)
- Add comfy.model_management.soft_empty_cache() at end of run_florence2_step
  to release intermediate KSampler latents and beam-search buffers before
  the next item iterates

96/96 tests still green (the new branch is covered by the existing
test_from_manifest_caches_by_combo + test_from_builder_returns_fallback
cases; same-model short-circuit doesn't change observable behavior for
those mocks).
2026-05-18 12:16:10 -07:00
Jason Hoku 49b1406cff feat: upscale_runner — florence2_hires mode dispatch + manifest entry builders
Adds an elif block to _run_upscale_thread that handles mode='florence2_hires',
delegating per-image work to florence2_hires.run_florence2_step() and using
two new pure-Python helpers (build_florence2_manifest_entry,
build_florence2_no_detection_entry) to construct manifest entries.

The pure-Python helpers are unit-tested in test_florence2_manifest_entry.py;
the wiring itself relies on the user's smoke test (mirroring the LTX pattern).
2026-05-18 01:33:11 -07:00
Jason HokuandClaude Opus 4.7 1f5adeb799 fix: extend image_format plumbing to remote VAE / upscales / distributed
Commit 490c519 added a webp/png/jpg dropdown but only flush_batch_with_vae
honored it. Three other save sites still hardcoded WEBP:
- flush_batch_with_remote_vae (image_generation.py)
- upscale_runner.py (3 sites in inline + deferred upscale paths)
- distribution_worker.py (PIL bytes shipped back to master)

Now all save sites read image_format from session_settings (orchestrator)
or the job spec (distributed) and match the format/extension.

Changes:
- remote_vae.py: RemoteVAEDecodeWorker accepts image_format kwarg; _worker
  uses it to pick format/extension at save time (the real save site for the
  remote VAE path — flush_batch_with_remote_vae only queues to the worker)
- generation_orchestrator.py: passes image_format to all RemoteVAEDecodeWorker
  constructors (initialize_remote_vae + 3 per-config sites); adds image_format
  to all 8 existing_data["meta"] writes so upscale_runner can read it from disk
- upscale_runner.py: reads image_format from manifest meta at thread start;
  uses _fmt/_ext locals at all 3 save sites (seedvr2, latent-decode, PIL)
- distribution_manager.py: injects _session_image_format into worker job
  config alongside existing _session_cooldown passthrough
- distribution_worker.py: reads _session_image_format from config; encodes
  bytes in the chosen format; stamps _image_ext on meta so master uses it
- distribution_routes.py: pops _image_ext from meta to build filename with
  correct extension before writing bytes to disk

Default behavior unchanged: all paths fall back to webp if image_format
is absent or unrecognised.

Code review found this in 490c519.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 13:29:52 -07:00
Jason HokuandClaude Opus 4.6 1e574d6927 fix: insert upscaled items at start of manifest for correct sort order
Dashboard fast-path sort assumes newest items are at the start of the
items array. append() put upscaled images at the end (oldest position),
making them appear as the oldest items when sorting by newest.

Changed both SeedVR2 and regular upscale paths from append() to
insert(0, ...) so upscaled images appear at the top (newest position).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-12 14:51:50 -07:00
Jason HokuandClaude Opus 4.6 c5265d0e83 fix: initialize total_upscale_duration in upscale_runner SeedVR2 path
UnboundLocalError when SeedVR2 upscale completed — total_upscale_duration
was referenced but never initialized. Added initialization to 0 at the
start of each per-image loop.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-12 14:23:31 -07:00
Jason HokuandClaude Opus 4.6 20a4677971 fix: add SeedVR2 mode to dashboard upscale_runner.py
The dashboard upscale (⬆ button) goes through upscale_runner.py, not
generation_orchestrator.py. SeedVR2 mode was only wired into the
inline orchestrator path — dashboard upscales silently produced no
output because mode="seedvr2" resulted in empty combos list.

Now upscale_runner.py handles mode=="seedvr2":
- Opens source image from disk as PIL (no latent decode needed)
- Calls seedvr2_upscale() from image_generation.py
- Saves result as _upscaled.webp in images directory
- Creates manifest entry with proper gen_index, file URL, metadata
- Supports chained steps (SeedVR2 → next step gets PIL result)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-12 13:26:01 -07:00
Jason HokuandClaude Opus 4.6 50359eac40 fix: upscale runner manifest entries, sorting, and VRAM cleanup
- Add "file" URL to manifest entries (dashboard needs /view?filename=...
  URL, not just the filename field)
- Exclude "file" and "gen_index" from source item copy to prevent
  stale values leaking through
- Assign gen_index = len(manifest_data["items"]) so upscaled images
  sort as newest instead of near their source image
- Send all new items per image via update_data event, not just the last
- Set PromptServer.last_prompt_id before sampling (KSampler progress
  callbacks require it, fixes AttributeError)
- Clear VRAM and RAM (soft_empty_cache, gc.collect, cuda.empty_cache)
  on completion, cancellation, and error via finally block

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-18 12:34:30 -07:00
Jason HokuandClaude Opus 4.6 b342127f83 fix: extract filename from manifest file URL in upscale runner
Manifest items use 'file' with a /view?filename=... URL, not a
'filename' field. Now parses the URL to extract the actual filename
when 'filename' is not directly available.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-18 00:12:50 -07:00
Jason HokuandClaude Opus 4.6 ef7cc81edb fix: dashboard upscale load_checkpoint args and empty preset body
- load_checkpoint() needs target_model_name, ckpt_name, use_remote_vae
  positional args — was being called with just model_name
- Preset save endpoints now validate for empty request body before
  JSON parsing to avoid "Expecting value" errors

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-17 23:36:41 -07:00
Jason HokuandClaude Opus 4.6 b0c480c2a5 feat: create upscale_runner.py for dashboard async upscaling
Background thread loads models, runs full pipeline chains, saves
upscaled images, updates manifests, and sends progress events.
Supports single image and all-favorited modes with cancel support.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-16 15:30:56 -07:00