Cancel an evicted safetensors load, spare the arbiter for CPU-only chat, fetch clips lazily
Four fixes from the latest review round: - The GPU arbiter's chat evictor only cancelled the llama.cpp side. The orchestrator publishes active_model_name once its worker reports success, so an in-flight safetensors load was visible only as an entry in loading_models and finished onto the GPU after ownership had transferred. Cancel every pending load, and give the safetensors branch the post-load ownership recheck the GGUF branch already had. - A manual gpu_layers=0 GGUF load runs on the CPU with the GPUs hidden from the child, yet it took the arbiter unconditionally: it cancelled a running image or video generation for a model needing no VRAM, then held CHAT ownership so the next GPU workload unloaded it for nothing. Gate the acquire on the same predicate the launch-time CPU-only mask uses, as the image and video loaders gate on their resolved device. - The staged-download hook subscribes per repo, not per job, so another job on the same repo advanced the staged queue (starting a load whose scoped files were still downloading) or wiped a queue that was still running. Compare the variant each callback carries, like the chat page's auto-load does. - The video gallery fetched every record of a page into an object URL that lives until the page closes: 50 clips at tens to hundreds of MB each, for cards the user may never scroll to. Fetch a clip as its card nears the strip's edge, plus the selected one the player needs.
This commit is contained in:
Daniel Han
parent
6c08c17267
commit
0add1accfd
8 changed files with 400 additions and 16 deletions
|
|
@ -45,6 +45,14 @@ def _evict_chat() -> None:
|
|||
orchestrator = get_inference_backend()
|
||||
if orchestrator.active_model_name:
|
||||
orchestrator.unload_model(orchestrator.active_model_name)
|
||||
# An in-flight safetensors load has no active_model_name yet (it is published only once the
|
||||
# worker reports success), so the unload above misses it and the load would finish onto the
|
||||
# GPU we just granted away. cancel_load discards the loading marker BEFORE tearing the worker
|
||||
# down, so a load parked between retries (or in _wait_response) observes the removal and
|
||||
# aborts instead of publishing. It runs off the lifecycle gate, which the load itself holds
|
||||
# for its whole duration, so this cannot deadlock.
|
||||
for pending in list(getattr(orchestrator, "loading_models", ()) or ()):
|
||||
orchestrator.cancel_load(pending)
|
||||
# Kill the subprocess too: its base CUDA context holds VRAM diffusion needs.
|
||||
orchestrator._shutdown_subprocess(timeout = 5.0)
|
||||
# The driver reclaims the killed VRAM asynchronously; wait for it to settle before diffusion
|
||||
|
|
|
|||
|
|
@ -1412,6 +1412,35 @@ def chat_load_active() -> bool:
|
|||
return _CHAT_LOADS_IN_FLIGHT > 0
|
||||
|
||||
|
||||
def zero_vram_chat_load(
|
||||
gpu_memory_mode: str,
|
||||
gpu_layers: int,
|
||||
extra_args: Optional[list] = None,
|
||||
needs_mmproj: bool = False,
|
||||
speculative_type: Optional[str] = None,
|
||||
) -> bool:
|
||||
"""Whether a GGUF load will hold no VRAM at all, decided from the request.
|
||||
|
||||
A deliberate manual zero-offload load runs on the CPU and is launched with the GPUs hidden
|
||||
from the child (see ``_cpu_only_zero_offload``), so it neither needs the GPU arbiter nor
|
||||
should evict an image/video pipeline for it. This mirrors that launch-time mask so both
|
||||
agree: an mmproj, a GPU drafter, a device pin or a surviving tensor mode all keep the GPUs
|
||||
visible, and the mask is skipped on Vulkan. ``--mmproj`` and ``--model-draft`` are added by
|
||||
the backend rather than being present in ``extra_args``, so their intent is passed in.
|
||||
"""
|
||||
if gpu_memory_mode != "manual" or gpu_layers != 0:
|
||||
return False
|
||||
# Any speculative mode may launch a GPU drafter; only the request's own knobs are known
|
||||
# here, so treat every non-empty selection as GPU-bearing rather than guess.
|
||||
if needs_mmproj or speculative_type:
|
||||
return False
|
||||
if LlamaCppBackend._is_vulkan_backend():
|
||||
return False
|
||||
return not LlamaCppBackend._zero_offload_keeps_gpu_visible(
|
||||
[str(arg) for arg in (extra_args or [])], os.environ
|
||||
)
|
||||
|
||||
|
||||
def hf_gguf_load_in_flight(hf_repo: str) -> bool:
|
||||
"""Return whether a GGUF load is active for hf_repo."""
|
||||
key = (hf_repo or "").strip().lower()
|
||||
|
|
@ -2539,6 +2568,22 @@ class LlamaCppBackend:
|
|||
"""Requested --gpu-layers for manual mode (-1 when not manual)."""
|
||||
return self._gpu_layers
|
||||
|
||||
@property
|
||||
def holds_no_vram(self) -> bool:
|
||||
"""Whether the resident server is a confirmed zero-VRAM launch.
|
||||
|
||||
True only for a deliberate manual zero-offload load whose launched argv carried no GPU
|
||||
companion, pin or tensor mode, so the child was started with the GPUs hidden. The GPU
|
||||
arbiter uses this to leave an image/video pipeline alone when re-asserting ownership for
|
||||
such a model. ``_gpu_offload_active`` is None when no GPU was detected at all (nothing to
|
||||
arbitrate) and True when something still reached the GPU, so both keep the normal path.
|
||||
"""
|
||||
return (
|
||||
self._gpu_memory_mode == "manual"
|
||||
and self._gpu_layers == 0
|
||||
and self._gpu_offload_active is False
|
||||
)
|
||||
|
||||
@property
|
||||
def n_cpu_moe(self) -> int:
|
||||
"""MoE expert layers manual mode kept on CPU (--n-cpu-moe); 0 = none."""
|
||||
|
|
|
|||
|
|
@ -4395,7 +4395,7 @@ async def _load_model_impl(
|
|||
# validation so a doomed load (bad id, unsupported gpu_ids on GGUF, training 409) can't
|
||||
# evict a working image/video model and then error. Mirrors the image/video loaders,
|
||||
# which validate before acquire_for.
|
||||
from core.inference.gpu_arbiter import acquire_for, current_owner, CHAT
|
||||
from core.inference.gpu_arbiter import acquire_for, current_owner, release, CHAT
|
||||
|
||||
# ── Already-loaded check: skip reload if the exact model is active ──
|
||||
backend = get_inference_backend()
|
||||
|
|
@ -4433,8 +4433,11 @@ async def _load_model_impl(
|
|||
_gguf_is_audio = getattr(llama_backend, "_is_audio", False)
|
||||
# Requested GGUF chat model already resident: assert CHAT ownership (no-op when
|
||||
# held) to correct a drifted arbiter owner. Guaranteed-success path, so evicting
|
||||
# here is correct.
|
||||
await asyncio.to_thread(acquire_for, CHAT)
|
||||
# here is correct -- unless the resident server is a confirmed zero-VRAM one
|
||||
# (manual zero offload, GPUs hidden from the child), which coexists with an
|
||||
# image/video pipeline and so must not evict it to re-announce itself.
|
||||
if not llama_backend.holds_no_vram:
|
||||
await asyncio.to_thread(acquire_for, CHAT)
|
||||
return LoadResponse(
|
||||
status = "already_loaded",
|
||||
model = model_log_label
|
||||
|
|
@ -4645,13 +4648,36 @@ async def _load_model_impl(
|
|||
# image and video loads do: a chat load holds no llama-server process until its GGUF has
|
||||
# downloaded, so a competing Images/Video acquire in that window found nothing to evict
|
||||
# and both then allocated VRAM at once. With the marker, that evictor cancels this load.
|
||||
from core.inference.llama_cpp import chat_load_in_flight
|
||||
from core.inference.llama_cpp import chat_load_in_flight, zero_vram_chat_load
|
||||
|
||||
await asyncio.to_thread(
|
||||
acquire_for,
|
||||
CHAT,
|
||||
lambda: gguf_load_stack.enter_context(chat_load_in_flight()),
|
||||
# ...but only when this load will actually use the GPU, exactly as the image and video
|
||||
# loaders gate on their resolved device. A manual gpu_layers=0 GGUF load runs on the CPU
|
||||
# with the GPUs hidden from the child, so taking the arbiter for it would cancel a
|
||||
# running image/video generation for a model that needs no VRAM, and leave CHAT recorded
|
||||
# as owner so the next GPU workload pointlessly unloads it.
|
||||
chat_load_needs_gpu = not (
|
||||
config.is_gguf
|
||||
and await asyncio.to_thread(
|
||||
zero_vram_chat_load,
|
||||
request.gpu_memory_mode,
|
||||
request.gpu_layers,
|
||||
extra_llama_args,
|
||||
bool(config.is_vision and not extra_args_disable_mmproj(extra_llama_args)),
|
||||
request.speculative_type,
|
||||
)
|
||||
)
|
||||
if chat_load_needs_gpu:
|
||||
await asyncio.to_thread(
|
||||
acquire_for,
|
||||
CHAT,
|
||||
lambda: gguf_load_stack.enter_context(chat_load_in_flight()),
|
||||
)
|
||||
else:
|
||||
# The marker still goes up (the download manager's handshake reads it, and it keeps
|
||||
# this load cancellable). Any stale CHAT claim is dropped AFTER the load, not here:
|
||||
# this load may still be replacing a GPU-backed chat model, and releasing up front
|
||||
# would let an image/video load allocate alongside the model not yet unloaded.
|
||||
gguf_load_stack.enter_context(chat_load_in_flight())
|
||||
|
||||
# ── GGUF path: load via llama-server ──────────────────────
|
||||
if config.is_gguf:
|
||||
|
|
@ -4816,8 +4842,9 @@ async def _load_model_impl(
|
|||
# An Images/Video acquire can land in the gap between the acquire above and
|
||||
# load_model clearing the cancel event, so its cancellation is lost and this load
|
||||
# spawns anyway. Ownership survives that gap: whoever took the GPU keeps it, and this
|
||||
# load undoes itself rather than leaving two models resident on one device.
|
||||
if current_owner() != CHAT:
|
||||
# load undoes itself rather than leaving two models resident on one device. A
|
||||
# zero-VRAM load never took ownership, so it has nothing to lose and never yields.
|
||||
if chat_load_needs_gpu and current_owner() != CHAT:
|
||||
await asyncio.to_thread(llama_backend.unload_model)
|
||||
raise HTTPException(
|
||||
status_code = 409,
|
||||
|
|
@ -4826,6 +4853,13 @@ async def _load_model_impl(
|
|||
"so the load was cancelled. Unload that model, then try again."
|
||||
),
|
||||
)
|
||||
if not chat_load_needs_gpu:
|
||||
# Zero-VRAM load done, and whatever GPU-backed chat model it replaced went with
|
||||
# it, so drop a now-stale CHAT claim: leaving it would make the next image/video
|
||||
# load "evict" a server holding nothing. Owner-guarded, so it no-ops when an
|
||||
# image/video model took the GPU while this was loading -- which is fine, they
|
||||
# coexist.
|
||||
await asyncio.to_thread(release, CHAT)
|
||||
|
||||
logger.info(
|
||||
f"Loaded GGUF model via llama-server: {model_log_label if native_grant_backed else config.identifier}"
|
||||
|
|
@ -4945,6 +4979,23 @@ async def _load_model_impl(
|
|||
detail = f"Failed to load model: {model_log_label if native_grant_backed else config.display_name}",
|
||||
)
|
||||
|
||||
# Same guard the GGUF branch runs above: an Images/Video acquire can land in the gap
|
||||
# between this load's cancellation and its publish, so the eviction is lost and the
|
||||
# model lands anyway. Ownership survives that gap, so this load undoes itself rather
|
||||
# than leaving two models resident on one device.
|
||||
if current_owner() != CHAT:
|
||||
await asyncio.to_thread(backend.unload_model, config.identifier)
|
||||
# The worker's base CUDA context outlives the model unload, so kill it too --
|
||||
# that VRAM is exactly what the image/video pipeline just took the GPU for.
|
||||
await asyncio.to_thread(backend._shutdown_subprocess, 5.0)
|
||||
raise HTTPException(
|
||||
status_code = 409,
|
||||
detail = (
|
||||
"An image or video model took the GPU while this model was loading, "
|
||||
"so the load was cancelled. Unload that model, then try again."
|
||||
),
|
||||
)
|
||||
|
||||
logger.info(
|
||||
f"Loaded model: {model_log_label if native_grant_backed else config.identifier}"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -249,3 +249,104 @@ def test_evict_chat_cancels_a_chat_load_that_has_not_spawned_yet(monkeypatch):
|
|||
# The marker is released with the load, so a later eviction is a no-op again.
|
||||
arb._evict_chat()
|
||||
assert unloaded == [True]
|
||||
|
||||
|
||||
def test_evict_chat_cancels_an_in_flight_safetensors_load(monkeypatch):
|
||||
# The orchestrator publishes active_model_name only once its worker reports success, so an
|
||||
# in-flight safetensors load is visible ONLY as an entry in loading_models. Gating the
|
||||
# cancellation on active_model_name let that worker finish after ownership transferred and
|
||||
# allocate the model alongside the image/video pipeline.
|
||||
import core.inference as core_inference
|
||||
import routes.inference as routes_inference
|
||||
|
||||
cancelled: list[str] = []
|
||||
|
||||
class _FakeLlama:
|
||||
is_active = False
|
||||
is_loaded = False
|
||||
|
||||
def unload_model(self):
|
||||
pass
|
||||
|
||||
def _wait_for_vram_settle(self, *, since_kill):
|
||||
pass
|
||||
|
||||
class _FakeOrchestrator:
|
||||
active_model_name = None # not published yet: the load is still running
|
||||
loading_models = {"unsloth/Qwen3-4B"}
|
||||
|
||||
def unload_model(self, name):
|
||||
raise AssertionError("unload_model must not run for an unpublished load")
|
||||
|
||||
def cancel_load(self, name):
|
||||
cancelled.append(name)
|
||||
return True
|
||||
|
||||
def _shutdown_subprocess(self, timeout = 5.0):
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(routes_inference, "get_llama_cpp_backend", lambda: _FakeLlama())
|
||||
monkeypatch.setattr(core_inference, "get_inference_backend", lambda: _FakeOrchestrator())
|
||||
|
||||
arb._evict_chat()
|
||||
|
||||
assert cancelled == ["unsloth/Qwen3-4B"]
|
||||
|
||||
|
||||
def test_evict_chat_cancels_every_pending_load_over_a_live_snapshot(monkeypatch):
|
||||
# cancel_load discards the marker it cancels, so iterate a snapshot rather than the live
|
||||
# set (mutating during iteration raises) and cancel each pending entry.
|
||||
import core.inference as core_inference
|
||||
import routes.inference as routes_inference
|
||||
|
||||
cancelled: list[str] = []
|
||||
|
||||
class _FakeLlama:
|
||||
is_active = False
|
||||
is_loaded = False
|
||||
|
||||
def unload_model(self):
|
||||
pass
|
||||
|
||||
def _wait_for_vram_settle(self, *, since_kill):
|
||||
pass
|
||||
|
||||
class _FakeOrchestrator:
|
||||
active_model_name = None
|
||||
|
||||
def __init__(self):
|
||||
self.loading_models = {"a/one", "b/two"}
|
||||
|
||||
def cancel_load(self, name):
|
||||
self.loading_models.discard(name)
|
||||
cancelled.append(name)
|
||||
return True
|
||||
|
||||
def _shutdown_subprocess(self, timeout = 5.0):
|
||||
pass
|
||||
|
||||
orchestrator = _FakeOrchestrator()
|
||||
monkeypatch.setattr(routes_inference, "get_llama_cpp_backend", lambda: _FakeLlama())
|
||||
monkeypatch.setattr(core_inference, "get_inference_backend", lambda: orchestrator)
|
||||
|
||||
arb._evict_chat()
|
||||
|
||||
assert sorted(cancelled) == ["a/one", "b/two"]
|
||||
assert orchestrator.loading_models == set()
|
||||
|
||||
|
||||
def test_the_safetensors_load_yields_a_gpu_it_lost_while_loading():
|
||||
# Mirror of the GGUF branch's guard: an Images/Video acquire can land in the gap between the
|
||||
# eviction and the load's publish, so the load has to undo itself instead of leaving two
|
||||
# models resident. Without it only the GGUF branch was safe.
|
||||
from pathlib import Path
|
||||
|
||||
route_src = (Path(__file__).resolve().parent.parent / "routes" / "inference.py").read_text(
|
||||
encoding = "utf-8"
|
||||
)
|
||||
load_impl = route_src[route_src.index("async def _load_model_impl") :]
|
||||
unsloth_load = load_impl.index("success = await asyncio.to_thread(\n backend.load_model,")
|
||||
tail = load_impl[unsloth_load:]
|
||||
guard = tail.index("if current_owner() != CHAT:")
|
||||
assert "await asyncio.to_thread(backend.unload_model, config.identifier)" in tail[guard:]
|
||||
assert tail.index("status_code = 409", guard) > guard
|
||||
|
|
|
|||
|
|
@ -1154,3 +1154,86 @@ def test_cmd_companion_ignores_cpu_forced_drafter():
|
|||
# mmproj still counts even alongside a CPU drafter.
|
||||
cmd = ["llama-server", "-md", "d.gguf", "--spec-draft-ngl", "0", "--mmproj", "p.gguf"]
|
||||
assert has(cmd, {}) is True
|
||||
|
||||
|
||||
# ── Zero-offload loads and the GPU arbiter ───────────────────────────
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def not_vulkan(monkeypatch):
|
||||
# The Vulkan probe reads the installed prebuilt; pin it so these assert the predicate.
|
||||
monkeypatch.setattr(LlamaCppBackend, "_is_vulkan_backend", staticmethod(lambda *a, **k: False))
|
||||
|
||||
|
||||
def test_zero_vram_chat_load_only_for_a_deliberate_cpu_only_offload(not_vulkan):
|
||||
# Manual + gpu_layers=0 is the one shape that launches with the GPUs hidden from the child,
|
||||
# so it is the one shape allowed to skip the GPU arbiter. Auto (or any pinned layer count)
|
||||
# puts the model on the GPU and must still evict an image/video pipeline.
|
||||
zero = llama_cpp_module.zero_vram_chat_load
|
||||
assert zero("manual", 0) is True
|
||||
assert zero("auto", 0) is False
|
||||
assert zero("manual", 1) is False
|
||||
assert zero("manual", -1) is False
|
||||
|
||||
|
||||
def test_zero_vram_chat_load_refuses_every_gpu_companion(not_vulkan):
|
||||
# The launch-time mask keeps the GPUs visible for a device pin, tensor mode, an mmproj or a
|
||||
# drafter, so those loads DO hold VRAM. --mmproj and --model-draft are added by the backend
|
||||
# rather than carried in the extras, so their intent arrives as flags.
|
||||
zero = llama_cpp_module.zero_vram_chat_load
|
||||
assert zero("manual", 0, ["--device", "CUDA0"]) is False
|
||||
assert zero("manual", 0, ["-dev", "CUDA0"]) is False
|
||||
assert zero("manual", 0, ["--split-mode", "tensor"]) is False
|
||||
assert zero("manual", 0, ["--model-draft", "/tmp/draft.gguf"]) is False
|
||||
assert zero("manual", 0, [], True) is False
|
||||
assert zero("manual", 0, [], False, "model") is False
|
||||
# A CPU-pinned device and a CPU-forced drafter keep it zero-VRAM.
|
||||
assert zero("manual", 0, ["--device", "none"]) is True
|
||||
assert zero("manual", 0, ["--model-draft", "/tmp/d.gguf", "--spec-draft-ngl", "0"]) is True
|
||||
|
||||
|
||||
def test_zero_vram_chat_load_is_skipped_on_vulkan(monkeypatch):
|
||||
# Vulkan builds are exempt from the CPU-only mask at launch, so the arbiter gate must match.
|
||||
monkeypatch.setattr(LlamaCppBackend, "_is_vulkan_backend", staticmethod(lambda *a, **k: True))
|
||||
assert llama_cpp_module.zero_vram_chat_load("manual", 0) is False
|
||||
|
||||
|
||||
def test_holds_no_vram_needs_the_launch_to_have_confirmed_it():
|
||||
# The property reports the LAUNCHED server, so it follows _gpu_offload_active: True (something
|
||||
# still reached the GPU) and None (no GPU detected at all) both keep the normal arbiter path.
|
||||
backend = LlamaCppBackend()
|
||||
backend._gpu_memory_mode = "manual"
|
||||
backend._gpu_layers = 0
|
||||
backend._gpu_offload_active = False
|
||||
assert backend.holds_no_vram is True
|
||||
backend._gpu_offload_active = True
|
||||
assert backend.holds_no_vram is False
|
||||
backend._gpu_offload_active = None
|
||||
assert backend.holds_no_vram is False
|
||||
backend._gpu_offload_active = False
|
||||
backend._gpu_layers = 20
|
||||
assert backend.holds_no_vram is False
|
||||
backend._gpu_layers = 0
|
||||
backend._gpu_memory_mode = "auto"
|
||||
assert backend.holds_no_vram is False
|
||||
|
||||
|
||||
def test_a_cpu_only_chat_load_does_not_take_the_gpu_arbiter():
|
||||
# A load that needs no VRAM must not evict a resident Images/Video pipeline (nor leave CHAT
|
||||
# recorded as owner, which would make the next GPU workload unload it for nothing). The
|
||||
# in-flight marker still goes up, and the post-load ownership recheck -- which would 409 a
|
||||
# load that never acquired -- is gated on the same flag.
|
||||
route_src = (Path(_BACKEND_DIR) / "routes" / "inference.py").read_text(encoding = "utf-8")
|
||||
load_impl = route_src[route_src.index("async def _load_model_impl") :]
|
||||
assert "chat_load_needs_gpu = not (" in load_impl
|
||||
gate = load_impl.index("chat_load_needs_gpu = not (")
|
||||
acquire = load_impl.index("if chat_load_needs_gpu:", gate)
|
||||
# The stale CHAT claim is dropped only AFTER the load: this load may still be replacing a
|
||||
# GPU-backed chat model, and releasing before it would let an image/video load allocate
|
||||
# alongside the model not yet unloaded.
|
||||
release = load_impl.index("await asyncio.to_thread(release, CHAT)", acquire)
|
||||
assert load_impl.index("if not chat_load_needs_gpu:", acquire) < release
|
||||
assert load_impl.index("success = await load_with_tensor_fallback(", acquire) < release
|
||||
assert "if chat_load_needs_gpu and current_owner() != CHAT:" in load_impl
|
||||
# The already-loaded fast path re-asserts ownership too; same exemption.
|
||||
assert "if not llama_backend.holds_no_vram:" in load_impl
|
||||
|
|
|
|||
|
|
@ -59,14 +59,27 @@ export function useStagedDownload({
|
|||
kind: DOWNLOAD_KIND.MODEL,
|
||||
repoId: current?.repoId ?? "__staged_download_idle__",
|
||||
activeVariant,
|
||||
onComplete: () => {
|
||||
// The listener subscription is per REPO, not per job, and one repo can have several jobs in
|
||||
// flight -- the Models tab downloading a chat quant of the same repo, say. Each callback
|
||||
// carries the variant it fired for, so drop the ones that aren't this staged entry: a
|
||||
// sibling's completion would otherwise advance the queue (and load a model whose scoped
|
||||
// files are still downloading), and its failure would wipe a queue still running. The chat
|
||||
// page's auto-load filters the same way.
|
||||
onComplete: (variant) => {
|
||||
if ((variant ?? null) !== activeVariant) return;
|
||||
const remaining = (queue ?? []).slice(1);
|
||||
advance();
|
||||
// Every entry is on disk, so the load will find its cache warm.
|
||||
if (remaining.length === 0) onReady();
|
||||
},
|
||||
onError: () => setQueue(null),
|
||||
onCancelled: () => setQueue(null),
|
||||
onError: (variant) => {
|
||||
if ((variant ?? null) !== activeVariant) return;
|
||||
setQueue(null);
|
||||
},
|
||||
onCancelled: (variant) => {
|
||||
if ((variant ?? null) !== activeVariant) return;
|
||||
setQueue(null);
|
||||
},
|
||||
});
|
||||
|
||||
useEffect(() => {
|
||||
|
|
|
|||
|
|
@ -701,6 +701,48 @@ export function VideoPage({ active = true }: { active?: boolean }) {
|
|||
}
|
||||
}, []);
|
||||
|
||||
// Fetching a clip pulls its whole MP4 into an object URL that lives until the page closes, so
|
||||
// fetching a full gallery page (PAGE_SIZE records, each tens to hundreds of MB for a longer
|
||||
// clip) up front pinned hundreds of MB -- gigabytes over a few "load more" pages -- for cards
|
||||
// the user may never scroll to, and starved the one they were waiting on. Fetch a card as it
|
||||
// nears the viewport instead; the tile already shows a spinner until its src lands.
|
||||
// The cards are observed from here rather than through a ref on each tile: the tile is a
|
||||
// Tooltip trigger, whose asChild clone owns that ref. Re-runs per page of records, so cards
|
||||
// appended by "load more" are picked up and removed ones are dropped with the observer.
|
||||
const stripRef = useRef<HTMLDivElement | null>(null);
|
||||
useEffect(() => {
|
||||
const root = stripRef.current;
|
||||
if (!root || typeof IntersectionObserver === "undefined") return;
|
||||
const io = new IntersectionObserver(
|
||||
(entries) => {
|
||||
for (const entry of entries) {
|
||||
if (!entry.isIntersecting) continue;
|
||||
const id = (entry.target as HTMLElement).dataset.clipId;
|
||||
const clip = id ? videos.find((v) => v.id === id) : undefined;
|
||||
if (clip) void ensureSrc(clip);
|
||||
}
|
||||
},
|
||||
// rootMargin is added to the ROOT's box only, never to an intermediate clipping ancestor,
|
||||
// so the root has to be the strip itself: a card scrolled past its right edge is clipped
|
||||
// by the strip, and a margin on the default viewport root would never reach it. The strip
|
||||
// scrolls horizontally, so the sideways margin is the one that matters -- it starts a
|
||||
// card's fetch a few tiles before it is scrolled to, so it is ready on arrival.
|
||||
{ root, rootMargin: "0px 600px" },
|
||||
);
|
||||
for (const card of root.querySelectorAll("[data-clip-id]")) io.observe(card);
|
||||
return () => io.disconnect();
|
||||
}, [videos, ensureSrc]);
|
||||
|
||||
// The preview player is what the user actually watches, so the selected clip is fetched
|
||||
// whether or not its card is on screen: a selection restored across a tab switch, a freshly
|
||||
// generated clip, or a pick made just before the strip scrolls it out of view.
|
||||
useEffect(() => {
|
||||
if (!selected) return;
|
||||
void (async () => {
|
||||
await ensureSrc(selected);
|
||||
})();
|
||||
}, [selected, ensureSrc]);
|
||||
|
||||
const loadGallery = useCallback(async () => {
|
||||
try {
|
||||
const page = await getVideoGallery(0, PAGE_SIZE);
|
||||
|
|
@ -708,7 +750,11 @@ export function VideoPage({ active = true }: { active?: boolean }) {
|
|||
galleryCache.hasMore = page.has_more;
|
||||
setVideos(page.videos);
|
||||
setHasMore(page.has_more);
|
||||
page.videos.forEach((video) => void ensureSrc(video));
|
||||
// No visibility signal without IntersectionObserver (jsdom / an old webview), so keep the
|
||||
// eager fetch there rather than render a strip of permanent spinners.
|
||||
if (typeof IntersectionObserver === "undefined") {
|
||||
page.videos.forEach((video) => void ensureSrc(video));
|
||||
}
|
||||
} catch {
|
||||
// Best-effort: a failed gallery load shouldn't block the page.
|
||||
}
|
||||
|
|
@ -727,7 +773,9 @@ export function VideoPage({ active = true }: { active?: boolean }) {
|
|||
});
|
||||
galleryCache.hasMore = page.has_more;
|
||||
setHasMore(page.has_more);
|
||||
page.videos.forEach((video) => void ensureSrc(video));
|
||||
if (typeof IntersectionObserver === "undefined") {
|
||||
page.videos.forEach((video) => void ensureSrc(video));
|
||||
}
|
||||
} catch {
|
||||
// transient; the user can scroll again to retry
|
||||
} finally {
|
||||
|
|
@ -1810,6 +1858,7 @@ export function VideoPage({ active = true }: { active?: boolean }) {
|
|||
|
||||
{(videos.length > 0 || busy === "generating") && (
|
||||
<div
|
||||
ref={stripRef}
|
||||
className="hover-scrollbar flex shrink-0 items-stretch gap-2 overflow-x-auto border-t border-foreground/10 p-3"
|
||||
onScroll={(e) => {
|
||||
// Near the right edge: pull the next older page (infinite scroll).
|
||||
|
|
@ -1829,6 +1878,7 @@ export function VideoPage({ active = true }: { active?: boolean }) {
|
|||
<TooltipTrigger asChild={true}>
|
||||
<button
|
||||
type="button"
|
||||
data-clip-id={video.id}
|
||||
onClick={() => setSelectedId(video.id)}
|
||||
className="relative flex h-16 w-24 shrink-0 flex-col justify-end overflow-hidden rounded-[10px] bg-muted/40 outline-none ring-1 ring-transparent transition-shadow hover:ring-border focus-visible:ring-2 focus-visible:ring-ring"
|
||||
>
|
||||
|
|
|
|||
|
|
@ -724,3 +724,36 @@ def test_chat_picker_routes_diffusion_picks_to_their_page():
|
|||
assert "diffusionPageForTask" in body and "navigateToPage" in body
|
||||
# Task-scoped pickers (already on those pages) must select normally.
|
||||
assert "if (!task)" in body
|
||||
|
||||
|
||||
def test_staged_download_callbacks_only_answer_their_own_variant():
|
||||
"""subscribeJobListeners is per repo, not per job, so a staged entry hears every job on
|
||||
that repo (the Models tab fetching a chat quant of the same repo, say). Each callback
|
||||
carries the variant it fired for: without comparing it, a sibling job's completion advanced
|
||||
the staged queue and started a load whose scoped files were still downloading, and its
|
||||
failure wiped a queue that was still running."""
|
||||
src = _read("features/hub/download-manager/use-staged-download.ts")
|
||||
for callback in ("onComplete", "onError", "onCancelled"):
|
||||
handler = re.search(rf"{callback}: \(variant\) => \{{\n(.*?)\n \}},", src, re.S)
|
||||
assert handler, f"{callback} does not take the variant"
|
||||
assert "(variant ?? null) !== activeVariant" in handler.group(1), callback
|
||||
|
||||
|
||||
def test_video_gallery_fetches_clips_as_their_cards_come_into_view():
|
||||
"""Each gallery record's src is a blob holding the whole MP4 until the page closes, so
|
||||
fetching a full page of them up front pinned hundreds of MB (gigabytes across "load more"
|
||||
pages) for cards the user may never scroll to. Fetch on visibility instead, and always
|
||||
fetch the selected clip, since that is the one the preview player plays."""
|
||||
src = _read("features/video/video-page.tsx")
|
||||
assert "new IntersectionObserver(" in src
|
||||
assert "ref={stripRef}" in src and "data-clip-id={video.id}" in src
|
||||
assert 'root.querySelectorAll("[data-clip-id]")' in src
|
||||
# rootMargin is added to the root box only, so the strip (the clipping scroller) has to
|
||||
# BE the root, or the prefetch margin never reaches a card clipped past its edge.
|
||||
assert '{ root, rootMargin: "0px 600px" }' in src
|
||||
# The only surviving whole-page fetches are the no-IntersectionObserver fallbacks.
|
||||
eager = list(re.finditer(r"page\.videos\.forEach\(\(video\) => void ensureSrc\(video\)\)", src))
|
||||
assert eager, "the jsdom/old-webview fallback fetch is missing"
|
||||
for match in eager:
|
||||
assert 'typeof IntersectionObserver === "undefined"' in src[max(0, match.start() - 260) : match.start()]
|
||||
assert re.search(r"if \(!selected\) return;\s*\n\s*void \(async \(\) => \{\s*\n\s*await ensureSrc\(selected\);", src)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue