From 72254e0a81fbb6717c5cbc7112eb8324cb74be33 Mon Sep 17 00:00:00 2001 From: Wasim Yousef Said Date: Sun, 21 Jun 2026 14:49:07 +0200 Subject: [PATCH] Tighten comments for PR #6493 (#6539) --- studio/backend/tests/test_llama_cpp_update.py | 15 +------ studio/backend/utils/llama_cpp_update.py | 19 +++----- .../src/components/llama-update-banner.tsx | 30 ++++--------- .../src/features/chat/chat-settings-sheet.tsx | 3 +- .../src/hooks/use-llama-update-check.ts | 45 +++++-------------- 5 files changed, 29 insertions(+), 83 deletions(-) diff --git a/studio/backend/tests/test_llama_cpp_update.py b/studio/backend/tests/test_llama_cpp_update.py index 1b509fea6c..4ceffbf75b 100644 --- a/studio/backend/tests/test_llama_cpp_update.py +++ b/studio/backend/tests/test_llama_cpp_update.py @@ -84,12 +84,7 @@ def _write_install( asset: str | None = None, release_tag: str | None = None, ) -> str: - """Create a fake prebuilt install tree and return the llama-server path. - - ``asset`` is the bundle filename recorded in the marker; omit it to model an - older marker that predates asset-based ROCm forwarding (backward compat). - ``release_tag`` is the full release tag (e.g. a ``b9596-mix-`` mix - build); defaults to ``tag`` for a plain prebuilt.""" + """Create a fake prebuilt install and return the llama-server path.""" bin_dir = dir_ / "build" / "bin" bin_dir.mkdir(parents = True, exist_ok = True) binary = bin_dir / "llama-server" @@ -436,7 +431,6 @@ def test_start_update_happy_path(monkeypatch, tmp_path): assert res["job"]["from_tag"] == "b9493" assert res["job"]["progress"] == 0.0 - # Wait for the background worker. deadline = time.time() + 10 while time.time() < deadline: job = upd.get_update_status()["job"] @@ -446,14 +440,11 @@ def test_start_update_happy_path(monkeypatch, tmp_path): assert job["state"] == "success", job assert job["to_tag"] == "b9518" assert job["reload_required"] is False - # Installer was invoked with the resolved install dir + latest + repo. assert "--install-dir" in captured["cmd"] assert str(install_dir) in captured["cmd"] assert "--llama-tag" in captured["cmd"] and "latest" in captured["cmd"] assert "unslothai/llama.cpp" in captured["cmd"] - # Progress lines were parsed and success pins progress at 1.0. assert job["progress"] == 1.0 - # The worker asks the installer for fine-grained progress milestones. assert popen_kwargs["env"]["UNSLOTH_PROGRESS_PERCENT_STEP"] == "5" @@ -653,7 +644,7 @@ def test_start_update_installer_missing_refuses(monkeypatch, tmp_path): class _FakeBackend: - """Minimal stand-in for LlamaCppBackend's update-coordination surface.""" + """Fake backend for update coordination.""" def __init__(self): import threading @@ -689,7 +680,6 @@ def test_update_sets_maintenance_flag_and_unloads(monkeypatch, tmp_path): seen = {} def _on_start(cmd): - # The maintenance flag must be set while the installer runs. seen["flag_during_install"] = backend._llama_update_in_progress _write_install(install_dir, "b9518") @@ -706,7 +696,6 @@ def test_update_sets_maintenance_flag_and_unloads(monkeypatch, tmp_path): assert backend.unloaded is True assert upd.get_update_status()["job"]["reload_required"] is True assert seen.get("flag_during_install") is True - # Cleared in the finally so model loads work again after the swap. assert backend._llama_update_in_progress is False diff --git a/studio/backend/utils/llama_cpp_update.py b/studio/backend/utils/llama_cpp_update.py index 4e15fbe074..8648b053d5 100644 --- a/studio/backend/utils/llama_cpp_update.py +++ b/studio/backend/utils/llama_cpp_update.py @@ -417,8 +417,7 @@ def _run_update(install_dir: Path, repo: str, asset: Optional[str], script: Path backend = None model_was_active = False try: - # Maintenance state so no load starts a server from the half-swapped binary - # (and the old binary is freed for the swap). Fails open without a backend. + # Block loads and free the binary while the installer swaps it. try: from routes.inference import get_llama_cpp_backend backend = get_llama_cpp_backend() @@ -432,8 +431,7 @@ def _run_update(install_dir: Path, repo: str, asset: Optional[str], script: Path try: with backend._serial_load_lock: backend._llama_update_in_progress = True - # is_active covers the loading/unhealthy window is_loaded misses - # (a live process also locks the exe on Windows during the swap). + # Active processes can lock the exe on Windows. if getattr(backend, "is_active", False): model_was_active = True backend.unload_model() @@ -452,8 +450,7 @@ def _run_update(install_dir: Path, repo: str, asset: Optional[str], script: Path ] cmd.extend(_rocm_install_args(asset)) logger.info("llama update: installing", cmd = " ".join(cmd)) - # Stream the installer output so download percent lines feed - # job["progress"]; finer milestones via UNSLOTH_PROGRESS_PERCENT_STEP. + # Stream progress lines into job["progress"]. env = dict(os.environ, UNSLOTH_PROGRESS_PERCENT_STEP = "5") proc = subprocess.Popen( cmd, @@ -494,12 +491,8 @@ def _run_update(install_dir: Path, repo: str, asset: Optional[str], script: Path tail = "".join(tail_lines).strip()[-1500:] raise RuntimeError(f"installer exited {returncode}: {tail or 'no output'}") - # New UNSLOTH_PREBUILT_INFO.json is on disk; drop the in-memory AND the - # on-disk freshness caches, then re-prime the 24h disk cache with the - # true newest, so the banner can't linger on a stale same-base value - # after the swap. drop_disk matters when the refresh below can't reach - # GitHub: without it, latest_published_release would replay the stale - # disk value; with it, latest reads as None and the banner fails open. + # Drop stale caches so the banner re-checks the swapped marker. + # If GitHub is offline, latest stays unknown and the banner fails open. reset_caches(drop_disk = True) try: latest_published_release(repo, force_refresh = True) @@ -532,7 +525,7 @@ def _run_update(install_dir: Path, repo: str, asset: Optional[str], script: Path finished_at = _utcnow(), ) finally: - # Lift the maintenance state so model loads work again, success or not. + # Always clear maintenance state. if backend is not None: try: backend._llama_update_in_progress = False diff --git a/studio/frontend/src/components/llama-update-banner.tsx b/studio/frontend/src/components/llama-update-banner.tsx index f53e6e0401..840383de90 100644 --- a/studio/frontend/src/components/llama-update-banner.tsx +++ b/studio/frontend/src/components/llama-update-banner.tsx @@ -8,12 +8,10 @@ import { toast } from "@/lib/toast"; import { cn } from "@/lib/utils"; import { Download } from "lucide-react"; import { type ReactElement, useEffect, useRef, useState } from "react"; -// Backend progress is coarse (5% steps, ~0.9 max) and the extract tail emits no -// signal. Creep toward this cap so the bar keeps moving rather than freezing. +// Creep toward this cap between coarse backend progress updates. const RUNNING_CAP = 0.95; -// Smoothed 0..1 bar progress: eases toward real `progress`, trickles toward a -// ceiling when idle, animates to 100% when `done`. Resets to 0 on each start. +// Smooth coarse backend progress without freezing between milestones. function useSmoothedProgress( active: boolean, progress: number | null, @@ -35,8 +33,7 @@ function useSmoothedProgress( let raf = 0; let last = performance.now(); const tick = (now: number) => { - // rAF timestamps can predate the performance.now() captured above, so - // clamp dt at 0 to keep the first frame from stepping backwards. + // Guard against a first rAF timestamp before the captured start time. const dt = Math.max(0, Math.min((now - last) / 1000, 0.1)); last = now; const current = displayRef.current; @@ -74,18 +71,11 @@ function useSmoothedProgress( interface LlamaUpdateBannerProps { enabled?: boolean; - // false: fill the parent instead of self-anchoring, so banners can stack in a - // shared container. true (default) keeps standalone desktop mounts working. + // false fills a shared stack; true self-anchors. positioned?: boolean; } -/** - * Non-invasive "Update llama.cpp" affordance. Appears bottom-right ~1s after a - * newer prebuilt is detected and stays up until the user explicitly acts on it - * (X, Update, or Remind me later). Clicking Update swaps the prebuilt in place - * via POST /api/llama/update. Can be turned off entirely in Settings -> - * General -> Notifications (on by default). - */ +/** Bottom-right llama.cpp update toast. */ export function LlamaUpdateBanner({ enabled = true, positioned = true, @@ -114,24 +104,20 @@ export function LlamaUpdateBanner({ const show = visible && status != null && (status.update_available || applying); const sizeBytes = status?.update_size_bytes ?? null; - // Round to whole MB; these prebuilts are hundreds of MB. const sizeLabel = sizeBytes && sizeBytes > 0 ? `${Math.round(sizeBytes / (1024 * 1024))} MB` : null; const updateProgress = status?.job.progress ?? null; const jobSucceeded = status?.job.state === "success"; - // Drives the bar so it animates continuously; aria reports the real value. + // Display value animates; aria uses the real progress. const displayProgress = useSmoothedProgress( applying, updateProgress, jobSucceeded, ); - // Render with no enter/exit animation. An opacity/transform transition (in or - // out) promotes a GPU compositing layer whose creation or teardown can flash - // for a frame on real displays, which reads as a flicker on appear and on - // dismiss. A plain conditional mount appears and leaves cleanly. + // Avoid opacity/transform transitions; GPU layer churn can flash. return show ? (