From 151ec413dac617af178920a782b3e8561a2adc1b Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 28 Jul 2026 10:23:23 +0000 Subject: [PATCH 01/29] Restore HF cache repos hidden by a dangling ref from the Hub inventory scan_cache_dir raises CorruptedCacheException for a repo whose refs/ names a commit with no snapshots// directory, and drops that repo from .repos into .warnings. _compute_all_hf_cache_scans only reads .repos, so the repo vanished from /api/hub/cached-gguf with a 200 while /api/models/local still listed it. Studio produces this state itself: a metadata probe that 404s writes .no_exist// and refs/main= without materialising a snapshot, so any repo re-uploaded since it was downloaded goes invisible. Prune only refs that resolve to no snapshot on disk, which is lossless since huggingface_hub rewrites them on the next download. Repos with in-flight .incomplete blobs are skipped because a running download owns its ref until the snapshot lands. Gated on scan.warnings so a healthy cache is never walked twice and never written to, and any OSError degrades to the previous behaviour. --- studio/backend/hub/utils/inventory_scan.py | 58 ++++++++- .../tests/test_hf_cache_dangling_refs.py | 114 ++++++++++++++++++ 2 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 studio/backend/tests/test_hf_cache_dangling_refs.py diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index 058fdf9b65..5336591b70 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -125,13 +125,69 @@ def all_hf_cache_scans() -> list: flight.event.set() +def _prune_dangling_hf_cache_refs(cache_root: Path) -> int: + """Drop ``refs/`` files naming a commit with no snapshot on disk. + + ``scan_cache_dir`` raises CorruptedCacheException for such a repo and omits + it from ``.repos``, so an otherwise intact download disappears from the Hub + inventory while the model-picker's directory walk still lists it. Studio + creates this state itself: a metadata probe that 404s writes ``.no_exist/`` + plus ``refs/main`` at the live upstream HEAD without materialising that + snapshot, so any repo re-uploaded since it was downloaded goes invisible. + + Removing the ref is lossless -- it already resolves to nothing, and + huggingface_hub rewrites it on the next download. Returns the number of + refs removed. + """ + try: + repo_dirs = [ + entry for entry in cache_root.iterdir() if entry.is_dir() and "--" in entry.name + ] + except OSError: + return 0 + removed = 0 + for repo_dir in repo_dirs: + refs_dir = repo_dir / "refs" + snapshots_dir = repo_dir / "snapshots" + if not refs_dir.is_dir(): + continue + # A running download writes its ref before materialising the snapshot, + # so its ref is legitimately dangling for the length of the transfer. + try: + if repo_cache_dir_has_incomplete_blobs(repo_dir): + continue + ref_files = [entry for entry in refs_dir.rglob("*") if entry.is_file()] + except OSError: + continue + for ref in ref_files: + try: + commit = ref.read_text(encoding = "utf-8").strip() + except (OSError, UnicodeDecodeError): + continue + if not commit or (snapshots_dir / commit).is_dir(): + continue + try: + ref.unlink() + except OSError as exc: + logger.debug("Could not prune dangling HF ref %s: %s", ref, exc) + continue + logger.info("Pruned dangling HF cache ref %s -> %s (no snapshot on disk)", ref, commit) + removed += 1 + return removed + + def _compute_all_hf_cache_scans() -> list: from huggingface_hub import scan_cache_dir scans: list = [] for cache_root in hf_cache_roots(): try: - scans.append(scan_cache_dir(cache_dir = str(cache_root))) + scan = scan_cache_dir(cache_dir = str(cache_root)) + # Only a warned-about scan can be hiding a repo, so a healthy cache + # is never walked twice and never written to. + if scan.warnings and _prune_dangling_hf_cache_refs(cache_root): + scan = scan_cache_dir(cache_dir = str(cache_root)) + scans.append(scan) except Exception as exc: logger.warning("Could not scan HF cache %s: %s", cache_root, exc) return scans diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py new file mode 100644 index 0000000000..ecc06a991d --- /dev/null +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -0,0 +1,114 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +"""A dangling ``refs/`` must not hide an intact repo from the scan. + +``scan_cache_dir`` raises CorruptedCacheException for a repo whose ref names a +commit with no ``snapshots//`` directory and omits it from ``.repos``, +so the model stays visible in the model picker (a plain directory walk) while +disappearing from every Hub inventory endpoint that feeds chat auto-load. +""" + +from __future__ import annotations + +import os +import stat +from pathlib import Path + +import pytest + +from hub.utils import inventory_scan + +SNAPSHOT = "a" * 40 +UPSTREAM_HEAD = "b" * 40 + + +def _build_repo( + cache_root: Path, + *, + ref: str, + extra_refs: dict[str, str] | None = None, + incomplete: bool = False, +) -> Path: + """A cache repo holding one snapshot at ``SNAPSHOT``, shaped like HF's.""" + repo_dir = cache_root / "models--Org--Model" + blobs = repo_dir / "blobs" + snapshot = repo_dir / "snapshots" / SNAPSHOT + refs = repo_dir / "refs" + for directory in (blobs, snapshot, refs): + directory.mkdir(parents = True, exist_ok = True) + blob = blobs / ("c" * 40) + blob.write_bytes(b"\0") + os.symlink(os.path.relpath(blob, snapshot), snapshot / "Q4_K_M.gguf") + (refs / "main").write_text(ref, encoding = "utf-8") + for name, commit in (extra_refs or {}).items(): + (refs / name).write_text(commit, encoding = "utf-8") + if incomplete: + (blobs / "d0d0d0d0.incomplete").write_bytes(b"partial") + return repo_dir + + +def _ref_names(repo_dir: Path) -> list[str]: + return sorted(entry.name for entry in (repo_dir / "refs").rglob("*") if entry.is_file()) + + +def _scanned_repo_ids(cache_root: Path, monkeypatch) -> list[str]: + monkeypatch.setattr(inventory_scan, "hf_cache_roots", lambda: [cache_root]) + return [ + repo.repo_id + for scan in inventory_scan._compute_all_hf_cache_scans() + for repo in scan.repos + ] + + +def test_dangling_ref_no_longer_hides_an_intact_repo(tmp_path, monkeypatch): + repo_dir = _build_repo(tmp_path, ref = UPSTREAM_HEAD) + + assert _scanned_repo_ids(tmp_path, monkeypatch) == ["Org/Model"] + assert _ref_names(repo_dir) == [] + + +def test_healthy_cache_is_neither_pruned_nor_rescanned(tmp_path, monkeypatch): + import huggingface_hub + + repo_dir = _build_repo(tmp_path, ref = SNAPSHOT, extra_refs = {"v1.0": SNAPSHOT}) + calls: list[str] = [] + real_scan = huggingface_hub.scan_cache_dir + + def counting_scan(cache_dir = None): + calls.append(str(cache_dir)) + return real_scan(cache_dir = cache_dir) + + monkeypatch.setattr(huggingface_hub, "scan_cache_dir", counting_scan) + + assert _scanned_repo_ids(tmp_path, monkeypatch) == ["Org/Model"] + assert len(calls) == 1 + assert _ref_names(repo_dir) == ["main", "v1.0"] + + +def test_in_flight_download_keeps_its_dangling_ref(tmp_path, monkeypatch): + """A download writes its ref before the snapshot lands, so it owns it.""" + repo_dir = _build_repo(tmp_path, ref = UPSTREAM_HEAD, incomplete = True) + + assert _scanned_repo_ids(tmp_path, monkeypatch) == [] + assert _ref_names(repo_dir) == ["main"] + + +def test_only_the_dangling_ref_of_a_mixed_repo_is_pruned(tmp_path, monkeypatch): + repo_dir = _build_repo(tmp_path, ref = SNAPSHOT, extra_refs = {"stale": UPSTREAM_HEAD}) + + assert _scanned_repo_ids(tmp_path, monkeypatch) == ["Org/Model"] + assert _ref_names(repo_dir) == ["main"] + + +def test_unwritable_refs_dir_degrades_instead_of_raising(tmp_path, monkeypatch): + repo_dir = _build_repo(tmp_path, ref = UPSTREAM_HEAD) + refs_dir = repo_dir / "refs" + refs_dir.chmod(stat.S_IRUSR | stat.S_IXUSR) + if os.access(refs_dir, os.W_OK): + pytest.skip("filesystem does not enforce directory write permissions") + try: + assert _scanned_repo_ids(tmp_path, monkeypatch) == [] + assert _ref_names(repo_dir) == ["main"] + finally: + refs_dir.chmod(stat.S_IRWXU) From 4e087edfed5239eebc09034690888154385c7945 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 28 Jul 2026 10:23:29 +0000 Subject: [PATCH 02/29] Stop chat auto-load downloading the default model after a cached load fails Every catch in the auto-load sweep is parameterless, and the only gate before the Hub download branch is loadAttempts >= MAX_AUTO_LOAD_ATTEMPTS. With a single cached repo one failure is 1 < 3, so a genuine /api/inference/load rejection fell through to fetching Qwen3.5-4B unrequested and finished on a green success toast. Record the reason when /api/inference/load itself rejects, and stop at the gate when one is set: the user has models on disk, so the failure is the useful answer rather than an unrelated download. Enumeration hiccups and consent gates never reach /load, so they keep falling through, and a device with nothing cached never records a failure and still downloads the default. --- .../src/features/chat/api/chat-adapter.ts | 42 +- .../studio/test_chat_autoload_failure_gate.py | 384 ++++++++++++++++++ 2 files changed, 425 insertions(+), 1 deletion(-) create mode 100644 tests/studio/test_chat_autoload_failure_gate.py diff --git a/studio/frontend/src/features/chat/api/chat-adapter.ts b/studio/frontend/src/features/chat/api/chat-adapter.ts index a0be3ea640..73b2a44a6a 100644 --- a/studio/frontend/src/features/chat/api/chat-adapter.ts +++ b/studio/frontend/src/features/chat/api/chat-adapter.ts @@ -1548,6 +1548,26 @@ async function autoLoadSmallestModel(): Promise<{ let hadNonTrustFailure = false; let loadAttempts = 0; const skippedAutoLoadCandidates = new Set(); + // Why the last attempted load failed, set only when /api/inference/load + // itself rejected, so enumeration hiccups (variant listing, validate) keep + // their existing fall-through behaviour. Boxed because a plain `let` assigned + // only inside a nested function narrows to `null` under control-flow + // analysis, which would make every read below a `never`. + const loadFailure: { current: { label: string; detail: string } | null } = { + current: null, + }; + + function noteLoadFailure(label: string, error: unknown): void { + const detail = + error instanceof Error && error.message.trim() ? error.message.trim() : ""; + loadFailure.current = { + label, + // Older backends (and non-Error throws) carry no detail; still name the + // model that failed rather than silently fetching a different one. + detail: + detail || "The server did not report a reason. Check the Studio logs.", + }; + } async function canAutoLoad(payload: { model_path: string; @@ -1593,6 +1613,9 @@ async function autoLoadSmallestModel(): Promise<{ } const currentStore = useChatRuntimeStore.getState(); const modelPath = candidate.loadId ?? candidate.id; + const failureLabel = candidate.ggufVariant + ? `${candidate.id} (${candidate.ggufVariant})` + : candidate.id; const { config } = resolveInitialConfig(candidate.id, candidate.ggufVariant); const effectiveMaxSeqLength = resolveLoadMaxSeqLength({ modelId: candidate.id, @@ -1689,6 +1712,12 @@ async function autoLoadSmallestModel(): Promise<{ gpu_ids: effectiveGpuIds ?? undefined, } : {}), + }).catch((error: unknown) => { + // The sweep's parameterless catches discard this error, which is what let + // a genuine load failure fall through to the "no downloaded models" Hub + // download. Rethrowing keeps the awaited type and their control flow. + noteLoadFailure(failureLabel, error); + throw error; }); // Only persist the global preference when the value came from the global // settings. A per-model config's choice must stay load-local, or autoloading @@ -1958,8 +1987,19 @@ async function autoLoadSmallestModel(): Promise<{ // Cap also gates the default download, so total /api/inference/load // budget across cached + fallback is MAX_AUTO_LOAD_ATTEMPTS, not +1. - if (loadAttempts >= MAX_AUTO_LOAD_ATTEMPTS) { + // A cached model that was tried and failed stops here too: the user has + // models on disk, so the reason is the useful answer and pulling an + // unrelated default off the Hub is not. A device with nothing cached never + // sets loadFailure and still falls through to the download below. + if (loadAttempts >= MAX_AUTO_LOAD_ATTEMPTS || loadFailure.current) { toast.dismiss(toastId); + if (loadFailure.current) { + toast.error(`Could not load ${loadFailure.current.label}`, { + description: loadFailure.current.detail, + duration: 10000, + closeButton: true, + }); + } return { loaded: false, blockedByTrustRemoteCode: diff --git a/tests/studio/test_chat_autoload_failure_gate.py b/tests/studio/test_chat_autoload_failure_gate.py new file mode 100644 index 0000000000..39437b6af9 --- /dev/null +++ b/tests/studio/test_chat_autoload_failure_gate.py @@ -0,0 +1,384 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +"""A failed auto-load of a cached model must not become a Hub download. + +Runs the real ``autoLoadSmallestModel`` from chat-adapter.ts under node with the +module boundary stubbed, so these assert behaviour (which /api/inference/load +calls happen, what the user is told) rather than source text. The sweep's +catches are parameterless, so before the fix a cached repo whose load rejected +fell straight through to fetching an unrelated default model and reported +success for it. +""" + +import json +import os +import shutil +import subprocess +import textwrap +from pathlib import Path + +import pytest + +WORKDIR = Path(__file__).resolve().parents[2] + + +def _source_path(relative_path: str) -> Path: + direct = WORKDIR / relative_path + if direct.exists(): + return direct + return WORKDIR / "unsloth_repo" / relative_path + + +ADAPTER = _source_path("studio/frontend/src/features/chat/api/chat-adapter.ts") +TEMP = WORKDIR / "temp" / "chat_autoload_failure_gate" +DEFAULT_MODEL = "unsloth/Qwen3.5-4B-MTP-GGUF" + +# Stubs for everything autoLoadSmallestModel imports. Each scenario supplies the +# cache inventory and how /validate and /load answer for a given model_path. +PREAMBLE = """ +type LastLocalModelKind = "gguf" | "model"; +type GgufVariantDetail = { + quant?: string | null; + filename?: string | null; + downloaded?: boolean; + size_bytes: number; +}; +type ChatModelSummary = Record; + +export type Scenario = { + ggufRepos: any[]; + modelRepos: any[]; + variants: Record; + lastLoaded: any; + validate: (payload: any) => any; + load: (payload: any) => any; +}; + +export const EVENTS: any[] = []; +let SCENARIO: Scenario; +export function setScenario(scenario: Scenario) { + SCENARIO = scenario; + EVENTS.length = 0; + STORE = makeStore(); +} + +const GPU_LAYERS_AUTO = -1; + +function makeStore(): any { + const state: any = { + hfToken: null, + params: { maxSeqLength: 4096, checkpoint: "" }, + activeGgufVariant: null, + activePresetSource: null, + gpuMemoryMode: "auto", + selectedGpuIds: null, + models: [], + setCheckpoint: () => {}, + setModelRequiresTrustRemoteCode: () => {}, + setParams: (p: any) => { state.params = p; }, + setModels: (m: any[]) => { state.models = m; }, + }; + return state; +} +let STORE: any = makeStore(); +const useChatRuntimeStore = { + getState: () => STORE, + setState: (_p: any) => {}, +}; + +function createLoadingToastIcon() { return null; } +const toast: any = Object.assign( + (_msg: string, _opts?: any) => "toast-id", + { + message: (msg: string, opts?: any) => { + EVENTS.push({ kind: "toast.message", msg, description: opts?.description }); + return "toast-id"; + }, + success: (msg: string) => EVENTS.push({ kind: "toast.success", msg }), + error: (msg: string, opts?: any) => + EVENTS.push({ kind: "toast.error", msg, description: opts?.description }), + dismiss: () => EVENTS.push({ kind: "toast.dismiss" }), + info: (msg: string) => EVENTS.push({ kind: "toast.info", msg }), + }, +); + +async function tryAdoptServerActiveModel() { return false; } +function resolveSpeculativeSettingsForLoad() { + return { speculativeType: null, specDraftNMax: 0 }; +} +function readLastLocalModelLoad() { return SCENARIO.lastLoaded; } +function recordLastLocalModelLoad(_x: any) {} +function resolveInitialConfig(_id: string, _variant: any) { + return { config: { + customContextLength: null, maxSeqLength: null, gpuMemoryMode: null, + gpuLayers: null, nCpuMoe: null, selectedGpuIds: undefined, + speculativeType: null, specDraftNMax: null, chatTemplateOverride: null, + kvCacheDtype: null, tensorParallel: false, + } }; +} +function resolveLoadMaxSeqLength(args: any) { return args.maxSeqLength ?? 0; } +function resolveFitMaxSeqLength(..._a: any[]) { return 0; } +function resolveManualAutoCtxPin(..._a: any[]) { return null; } +async function ensureGpuDeviceCache() {} +function reconcilePersistedGpuIds(ids: any) { return ids; } +function saveSpeculativeType(_x: any) {} +function persistGpuMemoryModeOnLoad(..._a: any[]) {} +function reasoningCapsFromLoad(_x: any) { return {}; } +function resolveToolsEnabledOnLoad(_x: any) { return {}; } +function loadedGpuMemoryFields(_x: any) { return {}; } +function resolveLoadedSpeculativeSettings(_x: any) { return {}; } +function isMultimodalResponse(_x: any) { return false; } + +async function listCachedGguf() { return SCENARIO.ggufRepos as any; } +async function listCachedModels() { return SCENARIO.modelRepos as any; } +async function listGgufVariants(repoId: string, _b?: any, _c?: any) { + const entry = SCENARIO.variants[repoId]; + if (entry === "throw") throw new Error("variant listing failed"); + return entry ?? { variants: [] }; +} +async function validateModel(payload: any) { + const result = SCENARIO.validate(payload); + if (result instanceof Error) throw result; + return result; +} +async function loadModel(payload: any) { + const result = SCENARIO.load(payload); + EVENTS.push({ + kind: "loadModel", + model_path: payload.model_path, + gguf_variant: payload.gguf_variant ?? null, + rejected: result instanceof Error, + }); + if (result instanceof Error) throw result; + return result; +} +""" + +SCENARIO_HELPERS = """ + const GEMMA = { + repo_id: "unsloth/gemma-4-26B-A4B-it-qat-GGUF", + load_id: "unsloth/gemma-4-26B-A4B-it-qat-GGUF", + cache_path: + "/home/john-doe/.cache/huggingface/hub/models--unsloth--gemma-4-26B-A4B-it-qat-GGUF", + size_bytes: 15800000000, + }; + const GEMMA_VARIANTS = { + variants: [{ + quant: "UD-Q4_K_XL", + filename: "UD-Q4_K_XL/gemma-4-26B-A4B-it-qat-UD-Q4_K_XL.gguf", + downloaded: true, + size_bytes: 15800000000, + }], + }; + const OOM = + "Failed to load model: llama-server was stopped by the operating system " + + "(signal 9), most likely out of memory."; + const VALIDATE_OK = () => ({ + requires_trust_remote_code: false, + requires_security_review: false, + requires_transformers_upgrade: false, + }); + const LOADED = (payload) => ({ + model: payload.model_path, + is_gguf: true, + context_length: 32768, + }); + const scenario = (over) => ({ + ggufRepos: [], + modelRepos: [], + variants: {}, + lastLoaded: null, + validate: VALIDATE_OK, + load: LOADED, + ...over, + }); +""" + + +def _require_node(): + if shutil.which("node") is None: + pytest.skip("node not available") + if not ADAPTER.exists(): + pytest.skip("studio chat sources not present") + result = subprocess.run( + ["node", "--experimental-strip-types", "--version"], + capture_output = True, + text = True, + timeout = 5, + ) + if result.returncode != 0: + pytest.skip("node --experimental-strip-types not available") + + +def _build_harness(): + """Slice autoLoadSmallestModel and its helpers verbatim out of the adapter.""" + lines = ADAPTER.read_text(encoding = "utf-8").splitlines() + start = next( + (i for i, line in enumerate(lines) if line.startswith("const MAX_AUTO_LOAD_ATTEMPTS")), + None, + ) + end = next( + ( + i + for i, line in enumerate(lines) + if line.startswith("export function createOpenAIStreamAdapter") + ), + None, + ) + assert start is not None and end is not None and start < end, ( + "could not locate the auto-load region in chat-adapter.ts" + ) + body = "\n".join(lines[start:end]) + assert "async function autoLoadSmallestModel" in body + TEMP.mkdir(parents = True, exist_ok = True) + (TEMP / "harness.ts").write_text( + "// @ts-nocheck\n" + PREAMBLE + "\n" + body + "\nexport { autoLoadSmallestModel };\n", + encoding = "utf-8", + ) + + +def _run(scenario_expr: str) -> dict: + _require_node() + _build_harness() + script = textwrap.dedent( + """ + // @ts-nocheck + import { autoLoadSmallestModel, setScenario, EVENTS } from "./harness.ts"; + """ + ) + SCENARIO_HELPERS + textwrap.dedent( + f""" + setScenario({scenario_expr}); + const result = await autoLoadSmallestModel(); + console.log(JSON.stringify({{ result, events: EVENTS }})); + """ + ) + (TEMP / "run.mts").write_text(script, encoding = "utf-8") + completed = subprocess.run( + ["node", "--experimental-strip-types", "--no-warnings", "run.mts"], + cwd = str(TEMP), + capture_output = True, + text = True, + timeout = 60, + env = dict(os.environ, NODE_NO_WARNINGS = "1"), + ) + assert completed.returncode == 0, f"stderr: {completed.stderr}\nstdout: {completed.stdout}" + last = [line for line in completed.stdout.strip().splitlines() if line.strip()][-1] + return json.loads(last) + + +def _loaded_paths(out: dict) -> list[str]: + return [event["model_path"] for event in out["events"] if event["kind"] == "loadModel"] + + +def _toasts(out: dict, kind: str) -> list[dict]: + return [event for event in out["events"] if event["kind"] == kind] + + +def test_failed_cached_load_does_not_download_the_default_model(): + """The reported case: the only cached repo is enumerated fine but its load + OOMs, and the default GGUF would load. Auto-load must stop at the failure + instead of fetching a model the user never asked for.""" + out = _run( + "scenario({ ggufRepos: [GEMMA], variants: { [GEMMA.repo_id]: GEMMA_VARIANTS }," + " load: (p) => p.model_path === GEMMA.repo_id ? new Error(OOM) : LOADED(p) })" + ) + + assert _loaded_paths(out) == ["unsloth/gemma-4-26B-A4B-it-qat-GGUF"] + assert DEFAULT_MODEL not in _loaded_paths(out) + assert out["result"]["loaded"] is False + assert _toasts(out, "toast.success") == [] + assert not any( + "Downloading a small model" in event["msg"] for event in _toasts(out, "toast.message") + ) + + +def test_failed_cached_load_surfaces_the_backend_reason(): + out = _run( + "scenario({ ggufRepos: [GEMMA], variants: { [GEMMA.repo_id]: GEMMA_VARIANTS }," + " load: () => new Error(OOM) })" + ) + + [error] = _toasts(out, "toast.error") + assert error["msg"] == "Could not load unsloth/gemma-4-26B-A4B-it-qat-GGUF (UD-Q4_K_XL)" + assert "out of memory" in error["description"] + + +def test_load_rejection_without_a_message_still_names_the_model(): + """Old backends and non-Error throws carry no detail; the model that failed + must still be named rather than swapped for the default.""" + out = _run( + "scenario({ ggufRepos: [GEMMA], variants: { [GEMMA.repo_id]: GEMMA_VARIANTS }," + " load: () => new Error('') })" + ) + + assert DEFAULT_MODEL not in _loaded_paths(out) + [error] = _toasts(out, "toast.error") + assert "unsloth/gemma-4-26B-A4B-it-qat-GGUF (UD-Q4_K_XL)" in error["msg"] + assert error["description"] + + +def test_empty_device_still_downloads_the_default_model(): + """Nothing cached means nothing failed, so the download path is untouched.""" + out = _run("scenario({})") + + assert _loaded_paths(out) == [DEFAULT_MODEL] + assert out["result"]["loaded"] is True + assert _toasts(out, "toast.error") == [] + assert [event["msg"] for event in _toasts(out, "toast.success")] == [ + "Loaded Qwen3.5-4B-MTP (UD-Q4_K_XL)" + ] + + +def test_enumeration_failure_still_downloads_the_default_model(): + """A cached repo whose variants cannot be listed never reached /load, so it + keeps falling through: only a real load rejection changes behaviour.""" + out = _run( + "scenario({ ggufRepos: [GEMMA], variants: { [GEMMA.repo_id]: 'throw' } })" + ) + + assert _loaded_paths(out) == [DEFAULT_MODEL] + assert out["result"]["loaded"] is True + + +def test_consent_gated_candidate_still_downloads_the_default_model(): + """trust_remote_code / security review block the load before it is attempted, + which is a deferral rather than a failure.""" + out = _run( + "scenario({ ggufRepos: [GEMMA], variants: { [GEMMA.repo_id]: GEMMA_VARIANTS }," + " validate: (p) => p.model_path === GEMMA.repo_id" + " ? { requires_trust_remote_code: true, requires_security_review: false," + " requires_transformers_upgrade: false } : VALIDATE_OK() })" + ) + + assert _loaded_paths(out) == [DEFAULT_MODEL] + assert out["result"]["loaded"] is True + + +def test_attempt_cap_still_gates_the_default_download(): + """Four broken cached repos: the sweep keeps trying smaller candidates, the + cap stops it at three attempts, and no fifth load goes to the Hub.""" + out = _run( + "scenario({ ggufRepos: [1, 2, 3, 4].map((i) => ({ ...GEMMA, repo_id: `r${i}`," + " load_id: `r${i}`, size_bytes: i }))," + " variants: Object.fromEntries([1, 2, 3, 4].map((i) => [`r${i}`, GEMMA_VARIANTS]))," + " load: () => new Error(OOM) })" + ) + + assert _loaded_paths(out) == ["r1", "r2", "r3"] + assert DEFAULT_MODEL not in _loaded_paths(out) + + +def test_a_later_cached_model_can_still_load_after_an_earlier_failure(): + """One broken repo must not veto a working one: the sweep continues and the + failure toast is only for a sweep that ends with nothing loaded.""" + out = _run( + "scenario({ ggufRepos: [1, 2].map((i) => ({ ...GEMMA, repo_id: `r${i}`," + " load_id: `r${i}`, size_bytes: i }))," + " variants: { r1: GEMMA_VARIANTS, r2: GEMMA_VARIANTS }," + " load: (p) => p.model_path === 'r1' ? new Error(OOM) : LOADED(p) })" + ) + + assert _loaded_paths(out) == ["r1", "r2"] + assert out["result"]["loaded"] is True + assert _toasts(out, "toast.error") == [] From 1a20d55be2d701a4d960fb56c30ca5a92aaf2e76 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 10:26:51 +0000 Subject: [PATCH 03/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../tests/test_hf_cache_dangling_refs.py | 4 +--- .../studio/test_chat_autoload_failure_gate.py | 22 ++++++++++--------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index ecc06a991d..f348ac1a55 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -55,9 +55,7 @@ def _ref_names(repo_dir: Path) -> list[str]: def _scanned_repo_ids(cache_root: Path, monkeypatch) -> list[str]: monkeypatch.setattr(inventory_scan, "hf_cache_roots", lambda: [cache_root]) return [ - repo.repo_id - for scan in inventory_scan._compute_all_hf_cache_scans() - for repo in scan.repos + repo.repo_id for scan in inventory_scan._compute_all_hf_cache_scans() for repo in scan.repos ] diff --git a/tests/studio/test_chat_autoload_failure_gate.py b/tests/studio/test_chat_autoload_failure_gate.py index 39437b6af9..4a855ac0c2 100644 --- a/tests/studio/test_chat_autoload_failure_gate.py +++ b/tests/studio/test_chat_autoload_failure_gate.py @@ -226,9 +226,9 @@ def _build_harness(): ), None, ) - assert start is not None and end is not None and start < end, ( - "could not locate the auto-load region in chat-adapter.ts" - ) + assert ( + start is not None and end is not None and start < end + ), "could not locate the auto-load region in chat-adapter.ts" body = "\n".join(lines[start:end]) assert "async function autoLoadSmallestModel" in body TEMP.mkdir(parents = True, exist_ok = True) @@ -241,17 +241,21 @@ def _build_harness(): def _run(scenario_expr: str) -> dict: _require_node() _build_harness() - script = textwrap.dedent( - """ + script = ( + textwrap.dedent( + """ // @ts-nocheck import { autoLoadSmallestModel, setScenario, EVENTS } from "./harness.ts"; """ - ) + SCENARIO_HELPERS + textwrap.dedent( - f""" + ) + + SCENARIO_HELPERS + + textwrap.dedent( + f""" setScenario({scenario_expr}); const result = await autoLoadSmallestModel(); console.log(JSON.stringify({{ result, events: EVENTS }})); """ + ) ) (TEMP / "run.mts").write_text(script, encoding = "utf-8") completed = subprocess.run( @@ -333,9 +337,7 @@ def test_empty_device_still_downloads_the_default_model(): def test_enumeration_failure_still_downloads_the_default_model(): """A cached repo whose variants cannot be listed never reached /load, so it keeps falling through: only a real load rejection changes behaviour.""" - out = _run( - "scenario({ ggufRepos: [GEMMA], variants: { [GEMMA.repo_id]: 'throw' } })" - ) + out = _run("scenario({ ggufRepos: [GEMMA], variants: { [GEMMA.repo_id]: 'throw' } })") assert _loaded_paths(out) == [DEFAULT_MODEL] assert out["result"]["loaded"] is True From 7f7c8db32327fcfc913e81d6785d23ac8a724814 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 28 Jul 2026 10:52:45 +0000 Subject: [PATCH 04/29] Address the review on the auto-load failure path The ref sweep probed refs_dir.is_dir() outside its per-repo guard. That call propagates a permission error rather than returning False, so one unreadable repo escaped to the caller's whole-scan except and dropped every model in the cache, which is the fault this change exists to repair. The probe moves inside the guard. scan_cache_dir already raises on such a repo before this code runs, so the test is scoped to the sweep. Both send paths show a generic "No model loaded" toast whenever the sweep returns loaded: false, so the detailed reason was immediately buried by the generic retry advice. The sweep now reports whether it already surfaced a failure and the callers skip their own toast when it did. The node harness wrote one shared run.mts per invocation, so concurrent runners could execute each other's script. Each invocation gets its own directory. --- studio/backend/hub/utils/inventory_scan.py | 11 ++-- .../tests/test_hf_cache_dangling_refs.py | 23 +++++++- .../src/features/chat/api/chat-adapter.ts | 58 ++++++++++++------- .../studio/test_chat_autoload_failure_gate.py | 30 +++++++++- 4 files changed, 92 insertions(+), 30 deletions(-) diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index 5336591b70..6ef39af497 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -149,11 +149,14 @@ def _prune_dangling_hf_cache_refs(cache_root: Path) -> int: for repo_dir in repo_dirs: refs_dir = repo_dir / "refs" snapshots_dir = repo_dir / "snapshots" - if not refs_dir.is_dir(): - continue - # A running download writes its ref before materialising the snapshot, - # so its ref is legitimately dangling for the length of the transfer. + # is_dir() propagates a permission error rather than returning False, so + # it belongs inside the guard: one unreadable repo must not abort the + # sweep and drop the whole scan, which is the failure this repairs. try: + if not refs_dir.is_dir(): + continue + # A running download writes its ref before materialising the + # snapshot, so its ref is legitimately dangling while it transfers. if repo_cache_dir_has_incomplete_blobs(repo_dir): continue ref_files = [entry for entry in refs_dir.rglob("*") if entry.is_file()] diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index f348ac1a55..71fb0b92d1 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -29,9 +29,10 @@ def _build_repo( ref: str, extra_refs: dict[str, str] | None = None, incomplete: bool = False, + name: str = "models--Org--Model", ) -> Path: """A cache repo holding one snapshot at ``SNAPSHOT``, shaped like HF's.""" - repo_dir = cache_root / "models--Org--Model" + repo_dir = cache_root / name blobs = repo_dir / "blobs" snapshot = repo_dir / "snapshots" / SNAPSHOT refs = repo_dir / "refs" @@ -99,6 +100,26 @@ def test_only_the_dangling_ref_of_a_mixed_repo_is_pruned(tmp_path, monkeypatch): assert _ref_names(repo_dir) == ["main"] +def test_an_unreadable_repo_does_not_abort_the_prune_sweep(tmp_path): + """is_dir() propagates a permission error instead of returning False, so an + unguarded probe would escape to the caller's whole-scan except and drop + every model in the cache. The sweep must skip that repo and carry on. + + Scoped to the sweep because scan_cache_dir itself already raises on an + unreadable repo dir, which is upstream of this code and unchanged here. + """ + dangling = _build_repo(tmp_path, ref = UPSTREAM_HEAD) + locked = _build_repo(tmp_path, ref = SNAPSHOT, name = "models--Org--Locked") + locked.chmod(0) + if os.access(locked / "refs", os.R_OK): + pytest.skip("filesystem does not enforce directory permissions") + try: + assert inventory_scan._prune_dangling_hf_cache_refs(tmp_path) == 1 + assert _ref_names(dangling) == [] + finally: + locked.chmod(stat.S_IRWXU) + + def test_unwritable_refs_dir_degrades_instead_of_raising(tmp_path, monkeypatch): repo_dir = _build_repo(tmp_path, ref = UPSTREAM_HEAD) refs_dir = repo_dir / "refs" diff --git a/studio/frontend/src/features/chat/api/chat-adapter.ts b/studio/frontend/src/features/chat/api/chat-adapter.ts index 73b2a44a6a..1954dfdd59 100644 --- a/studio/frontend/src/features/chat/api/chat-adapter.ts +++ b/studio/frontend/src/features/chat/api/chat-adapter.ts @@ -1502,6 +1502,10 @@ function isAutoLoadableGgufVariant(variant: GgufVariantDetail | null): boolean { async function autoLoadSmallestModel(): Promise<{ loaded: boolean; blockedByTrustRemoteCode: boolean; + /** A specific load failure was already reported, so callers must not replace + * it with their generic "no model loaded" advice. Optional so every other + * return keeps its existing shape. */ + loadFailureReported?: boolean; }> { if (await tryAdoptServerActiveModel()) { return { loaded: true, blockedByTrustRemoteCode: false }; @@ -2004,6 +2008,7 @@ async function autoLoadSmallestModel(): Promise<{ loaded: false, blockedByTrustRemoteCode: blockedByTrustRemoteCode && !hadNonTrustFailure, + loadFailureReported: loadFailure.current !== null, }; } @@ -2162,19 +2167,23 @@ export function createOpenAIStreamAdapter( await waitForModelReady(abortSignal); } if (!useChatRuntimeStore.getState().params.checkpoint) { - const { loaded, blockedByTrustRemoteCode } = + const { loaded, blockedByTrustRemoteCode, loadFailureReported } = await autoLoadSmallestModel(); if (!loaded) { - toast.error( - blockedByTrustRemoteCode - ? "This model needs custom code approval" - : "No model loaded", - { - description: blockedByTrustRemoteCode - ? "Select it from the top bar to review and approve its custom code, or pick another model." - : "Pick a model in the top bar, then retry.", - }, - ); + // A reported load failure already names the model and the reason, + // so the generic advice would only bury it. + if (!loadFailureReported) { + toast.error( + blockedByTrustRemoteCode + ? "This model needs custom code approval" + : "No model loaded", + { + description: blockedByTrustRemoteCode + ? "Select it from the top bar to review and approve its custom code, or pick another model." + : "Pick a model in the top bar, then retry.", + }, + ); + } throw new Error("Load a model first."); } } @@ -2429,24 +2438,29 @@ export function createOpenAIStreamAdapter( // Prefer a model already loaded by the CLI/API before auto-loading. let loaded: boolean; let blockedByTrustRemoteCode: boolean; + let loadFailureReported: boolean | undefined; try { - ({ loaded, blockedByTrustRemoteCode } = + ({ loaded, blockedByTrustRemoteCode, loadFailureReported } = await autoLoadSmallestModel()); } catch (error) { clearSelectedImageEditReference(); throw error; } if (!loaded) { - toast.error( - blockedByTrustRemoteCode - ? "This model needs custom code approval" - : "No model loaded", - { - description: blockedByTrustRemoteCode - ? "Select it from the top bar to review and approve its custom code, or pick another model." - : "Pick a model in the top bar, then retry.", - }, - ); + // A reported load failure already names the model and the reason, so + // the generic advice would only bury it. + if (!loadFailureReported) { + toast.error( + blockedByTrustRemoteCode + ? "This model needs custom code approval" + : "No model loaded", + { + description: blockedByTrustRemoteCode + ? "Select it from the top bar to review and approve its custom code, or pick another model." + : "Pick a model in the top bar, then retry.", + }, + ); + } clearSelectedImageEditReference(); throw new Error("Load a model first."); } diff --git a/tests/studio/test_chat_autoload_failure_gate.py b/tests/studio/test_chat_autoload_failure_gate.py index 4a855ac0c2..7122e25aaa 100644 --- a/tests/studio/test_chat_autoload_failure_gate.py +++ b/tests/studio/test_chat_autoload_failure_gate.py @@ -15,6 +15,7 @@ import json import os import shutil import subprocess +import tempfile import textwrap from pathlib import Path @@ -245,7 +246,7 @@ def _run(scenario_expr: str) -> dict: textwrap.dedent( """ // @ts-nocheck - import { autoLoadSmallestModel, setScenario, EVENTS } from "./harness.ts"; + import { autoLoadSmallestModel, setScenario, EVENTS } from "../harness.ts"; """ ) + SCENARIO_HELPERS @@ -257,10 +258,13 @@ def _run(scenario_expr: str) -> dict: """ ) ) - (TEMP / "run.mts").write_text(script, encoding = "utf-8") + # Its own directory per invocation: a shared run.mts would let concurrent + # runners (pytest-xdist, or two suites at once) execute each other's script. + run_dir = Path(tempfile.mkdtemp(prefix = "run", dir = TEMP)) + (run_dir / "run.mts").write_text(script, encoding = "utf-8") completed = subprocess.run( ["node", "--experimental-strip-types", "--no-warnings", "run.mts"], - cwd = str(TEMP), + cwd = str(run_dir), capture_output = True, text = True, timeout = 60, @@ -384,3 +388,23 @@ def test_a_later_cached_model_can_still_load_after_an_earlier_failure(): assert _loaded_paths(out) == ["r1", "r2"] assert out["result"]["loaded"] is True assert _toasts(out, "toast.error") == [] + + +def test_reported_failure_is_flagged_so_callers_drop_the_generic_advice(): + """Both send paths show a generic "No model loaded" toast whenever the sweep + returns loaded: false. Without a flag that lands after the detailed toast, + making the retry advice the last thing the user sees.""" + out = _run( + "scenario({ ggufRepos: [GEMMA], variants: { [GEMMA.repo_id]: GEMMA_VARIANTS }," + " load: () => new Error(OOM) })" + ) + + assert out["result"]["loaded"] is False + assert out["result"]["loadFailureReported"] is True + + +def test_empty_device_does_not_flag_a_reported_failure(): + """Nothing was attempted, so the callers must keep their generic advice.""" + out = _run("scenario({ ggufRepos: [], models: [] })") + + assert out["result"].get("loadFailureReported") is not True From 4bad250bd1789f9cbeb4fd3025bf83f0c42849d3 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 28 Jul 2026 11:52:25 +0000 Subject: [PATCH 05/29] Leave a just-written ref alone during the prune sweep snapshot_download writes refs/ before it fetches the first file, so a starting download has a live ref with neither a snapshot directory nor an .incomplete blob yet. The incomplete-blob check alone therefore misses that window, and unlinking there strands the finished snapshot with no branch mapping, breaking later offline and local_files_only loads by revision. Skip refs younger than the settle window. A ref stranded by an upstream re-upload persists, so deferring it one sweep only postpones the repair. The node harness also still shared harness.ts across invocations even though each run got its own script, so build it inside the run directory too. --- studio/backend/hub/utils/inventory_scan.py | 12 ++++++++++++ .../tests/test_hf_cache_dangling_refs.py | 19 +++++++++++++++++++ .../studio/test_chat_autoload_failure_gate.py | 16 ++++++++-------- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index 6ef39af497..839d10e955 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -46,6 +46,9 @@ from hub.utils.hf_cache_state import ( # this TTL only bounds staleness from out-of-band edits while skipping re-walks # on rapid UI navigation. _HF_CACHE_SCANS_TTL_SECONDS = 15.0 +# How long a dangling ref must have sat before the sweep will unlink it, so a +# download that has written its ref but not yet its first blob keeps it. +_REF_SETTLE_SECONDS = 60.0 _GGUF_SPLIT_RE = re.compile(r"-(\d{3,})-of-(\d{3,})(?=\.gguf$)", re.IGNORECASE) _hf_cache_scans_lock = threading.Lock() @@ -164,6 +167,15 @@ def _prune_dangling_hf_cache_refs(cache_root: Path) -> int: continue for ref in ref_files: try: + # snapshot_download writes refs/ before it fetches the + # first file, so a starting download has a live ref with neither + # a snapshot nor an .incomplete blob yet. Leave anything recent + # alone: a ref stranded by a re-upload persists, so skipping it + # for one sweep only defers the repair to the next one, while + # unlinking mid-download would strand the finished snapshot with + # no branch mapping and break later loads by revision. + if time.time() - ref.stat().st_mtime < _REF_SETTLE_SECONDS: + continue commit = ref.read_text(encoding = "utf-8").strip() except (OSError, UnicodeDecodeError): continue diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 71fb0b92d1..9687003380 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -13,6 +13,7 @@ from __future__ import annotations import os import stat +import time from pathlib import Path import pytest @@ -30,6 +31,7 @@ def _build_repo( extra_refs: dict[str, str] | None = None, incomplete: bool = False, name: str = "models--Org--Model", + settled: bool = True, ) -> Path: """A cache repo holding one snapshot at ``SNAPSHOT``, shaped like HF's.""" repo_dir = cache_root / name @@ -46,6 +48,12 @@ def _build_repo( (refs / name).write_text(commit, encoding = "utf-8") if incomplete: (blobs / "d0d0d0d0.incomplete").write_bytes(b"partial") + if settled: + # The sweep leaves a just-written ref alone, so age these past that + # window to exercise the pruning rather than the settle guard. + old = time.time() - 3600 + for entry in refs.rglob("*"): + os.utime(entry, (old, old)) return repo_dir @@ -131,3 +139,14 @@ def test_unwritable_refs_dir_degrades_instead_of_raising(tmp_path, monkeypatch): assert _ref_names(repo_dir) == ["main"] finally: refs_dir.chmod(stat.S_IRWXU) + + +def test_a_just_written_ref_survives_the_sweep(tmp_path, monkeypatch): + """snapshot_download writes refs/ before fetching the first file, + so there is a window with a live ref, no snapshot and no .incomplete blob. + Unlinking there strands the finished snapshot with no branch mapping and + breaks later local_files_only loads by revision.""" + repo_dir = _build_repo(tmp_path, ref = UPSTREAM_HEAD, settled = False) + + assert inventory_scan._prune_dangling_hf_cache_refs(tmp_path) == 0 + assert _ref_names(repo_dir) == ["main"] diff --git a/tests/studio/test_chat_autoload_failure_gate.py b/tests/studio/test_chat_autoload_failure_gate.py index 7122e25aaa..21ba34c04c 100644 --- a/tests/studio/test_chat_autoload_failure_gate.py +++ b/tests/studio/test_chat_autoload_failure_gate.py @@ -212,7 +212,7 @@ def _require_node(): pytest.skip("node --experimental-strip-types not available") -def _build_harness(): +def _build_harness(run_dir: Path): """Slice autoLoadSmallestModel and its helpers verbatim out of the adapter.""" lines = ADAPTER.read_text(encoding = "utf-8").splitlines() start = next( @@ -232,8 +232,7 @@ def _build_harness(): ), "could not locate the auto-load region in chat-adapter.ts" body = "\n".join(lines[start:end]) assert "async function autoLoadSmallestModel" in body - TEMP.mkdir(parents = True, exist_ok = True) - (TEMP / "harness.ts").write_text( + (run_dir / "harness.ts").write_text( "// @ts-nocheck\n" + PREAMBLE + "\n" + body + "\nexport { autoLoadSmallestModel };\n", encoding = "utf-8", ) @@ -241,12 +240,16 @@ def _build_harness(): def _run(scenario_expr: str) -> dict: _require_node() - _build_harness() + # Its own directory per invocation, harness included: sharing either file + # lets a concurrent runner read one that another is mid-rewrite. + TEMP.mkdir(parents = True, exist_ok = True) + run_dir = Path(tempfile.mkdtemp(prefix = "run", dir = TEMP)) + _build_harness(run_dir) script = ( textwrap.dedent( """ // @ts-nocheck - import { autoLoadSmallestModel, setScenario, EVENTS } from "../harness.ts"; + import { autoLoadSmallestModel, setScenario, EVENTS } from "./harness.ts"; """ ) + SCENARIO_HELPERS @@ -258,9 +261,6 @@ def _run(scenario_expr: str) -> dict: """ ) ) - # Its own directory per invocation: a shared run.mts would let concurrent - # runners (pytest-xdist, or two suites at once) execute each other's script. - run_dir = Path(tempfile.mkdtemp(prefix = "run", dir = TEMP)) (run_dir / "run.mts").write_text(script, encoding = "utf-8") completed = subprocess.run( ["node", "--experimental-strip-types", "--no-warnings", "run.mts"], From a1fc09b924bca6866d6fec5b6ea71413edb9c12b Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 28 Jul 2026 13:25:58 +0000 Subject: [PATCH 06/29] Recover a ref-hidden HF cache repo without touching the cache Replace the dangling-ref prune with a read-only recovery. huggingface_hub writes refs/ with a bare unlocked in-place write_text (_cache_commit_hash_for_specific_revision), so there is no lock an external process can take and no compare-and-delete that a concurrent download cannot lose. Deleting refs from the sweep could not be made race-free, so it is gone. _scan_cached_repo already builds every revision successfully and only then raises CorruptedCacheException over leftover refs, so the repo is entirely scannable and is dropped purely by that consistency assertion. _recover_repo_hidden_by_dangling_refs rebuilds the entry from the same directories huggingface_hub reads and leaves the ref exactly as it is. It bails out whenever anything other than leftover refs would have failed the upstream scan, so a genuinely corrupt repo stays omitted, and a repo whose ref is written but whose snapshot has not landed yet is never reported as downloaded. Recovered entries mirror CachedRepoInfo / CachedRevisionInfo / CachedFileInfo field for field but are built here rather than through huggingface_hub's constructors, so a field added or removed upstream cannot break the call; a test asserts the surfaces stay in step. They are frozen because HFCacheInfo.delete_revisions keys a dict by repo and takes a set difference over revisions. Give a recovered row the snapshot path as its load identity. With no resolvable refs/main, from_pretrained(repo_id) fails offline and fetches the current upstream HEAD online instead of the snapshot already on disk, which is the download this whole fix exists to prevent. Read scan.warnings with getattr: an AttributeError there lands in the per-root except Exception and silently blanks a whole cache root. --- .../hub/services/models/cache_inventory.py | 10 + studio/backend/hub/utils/inventory_scan.py | 312 ++++++++++++++---- .../tests/test_hf_cache_dangling_refs.py | 312 ++++++++++++++---- 3 files changed, 513 insertions(+), 121 deletions(-) diff --git a/studio/backend/hub/services/models/cache_inventory.py b/studio/backend/hub/services/models/cache_inventory.py index 807ec70991..cdc43316e9 100644 --- a/studio/backend/hub/services/models/cache_inventory.py +++ b/studio/backend/hub/services/models/cache_inventory.py @@ -285,6 +285,16 @@ def _cache_inventory_fields( except (OSError, RuntimeError, ValueError): active_cache = False load_id = str(snapshot_path or repo_path) + if ( + load_id == repo_id + and snapshot_path is not None + and repo_path is not None + and not hf_cache_scan.default_ref_resolves_on_disk(repo_path) + ): + # No usable refs/main: from_pretrained(repo_id) would fail offline and + # would fetch the current upstream HEAD online, ignoring the snapshot + # already on disk. Point the load straight at that snapshot instead. + load_id = str(snapshot_path) return { "inventory_id": _local_inventory_id("cache", model_format, repo_id), "load_id": load_id, diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index 839d10e955..838e987976 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -17,7 +17,7 @@ import hashlib import re import threading import time -from dataclasses import dataclass +from dataclasses import dataclass, replace from pathlib import Path from typing import Callable, Optional @@ -46,9 +46,6 @@ from hub.utils.hf_cache_state import ( # this TTL only bounds staleness from out-of-band edits while skipping re-walks # on rapid UI navigation. _HF_CACHE_SCANS_TTL_SECONDS = 15.0 -# How long a dangling ref must have sat before the sweep will unlink it, so a -# download that has written its ref but not yet its first blob keeps it. -_REF_SETTLE_SECONDS = 60.0 _GGUF_SPLIT_RE = re.compile(r"-(\d{3,})-of-(\d{3,})(?=\.gguf$)", re.IGNORECASE) _hf_cache_scans_lock = threading.Lock() @@ -128,67 +125,242 @@ def all_hf_cache_scans() -> list: flight.event.set() -def _prune_dangling_hf_cache_refs(cache_root: Path) -> int: - """Drop ``refs/`` files naming a commit with no snapshot on disk. +# huggingface_hub skips these when walking refs/ and snapshots/; mirrored so a +# stray OS helper file is not mistaken for cache corruption. +_CACHE_ENTRIES_TO_IGNORE = frozenset({".DS_Store"}) +_HF_REPO_TYPES = frozenset({"model", "dataset", "space"}) - ``scan_cache_dir`` raises CorruptedCacheException for such a repo and omits - it from ``.repos``, so an otherwise intact download disappears from the Hub - inventory while the model-picker's directory walk still lists it. Studio - creates this state itself: a metadata probe that 404s writes ``.no_exist/`` - plus ``refs/main`` at the live upstream HEAD without materialising that - snapshot, so any repo re-uploaded since it was downloaded goes invisible. - Removing the ref is lossless -- it already resolves to nothing, and - huggingface_hub rewrites it on the next download. Returns the number of - refs removed. - """ +# Recovered entries deliberately mirror huggingface_hub's CachedFileInfo / +# CachedRevisionInfo / CachedRepoInfo field-for-field, but are constructed here +# rather than imported so a field added or removed upstream cannot break the +# call. test_hf_cache_dangling_refs asserts the surfaces stay in step. Frozen +# (so hashable) because HFCacheInfo.delete_revisions() keys a dict by repo and +# takes a set difference over ``revisions``. +@dataclass(frozen = True) +class _RecoveredFileInfo: + file_name: str + file_path: Path + size_on_disk: int + blob_path: Path + blob_last_accessed: float + blob_last_modified: float + + +@dataclass(frozen = True) +class _RecoveredRevisionInfo: + commit_hash: str + snapshot_path: Path + size_on_disk: int + files: frozenset + refs: frozenset + last_modified: float + + +@dataclass(frozen = True) +class _RecoveredRepoInfo: + repo_id: str + repo_type: str + repo_path: Path + size_on_disk: int + nb_files: int + revisions: frozenset + last_accessed: float + last_modified: float + + @property + def refs(self) -> dict: + return {ref: rev for rev in self.revisions for ref in rev.refs} + + +def _hf_repo_identity(repo_dir_name: str) -> Optional[tuple[str, str]]: + """``models--Org--Model`` -> ``("model", "Org/Model")``, as huggingface_hub parses it.""" + if "--" not in repo_dir_name: + return None + repo_type, _, repo_id = repo_dir_name.partition("--") + repo_type = repo_type[:-1] + if repo_type not in _HF_REPO_TYPES or not repo_id: + return None + return repo_type, repo_id.replace("--", "/") + + +def _read_refs_by_commit(refs_dir: Path) -> Optional[dict[str, set[str]]]: + """Map commit hash -> ref names under ``refs/``. None if unreadable.""" + refs_by_commit: dict[str, set[str]] = {} + if not refs_dir.exists(): + return refs_by_commit + if refs_dir.is_file(): + return None try: - repo_dirs = [ - entry for entry in cache_root.iterdir() if entry.is_dir() and "--" in entry.name - ] + entries = sorted(refs_dir.rglob("*")) except OSError: - return 0 - removed = 0 - for repo_dir in repo_dirs: - refs_dir = repo_dir / "refs" - snapshots_dir = repo_dir / "snapshots" - # is_dir() propagates a permission error rather than returning False, so - # it belongs inside the guard: one unreadable repo must not abort the - # sweep and drop the whole scan, which is the failure this repairs. + return None + for ref_path in entries: try: - if not refs_dir.is_dir(): + if ref_path.is_dir() or ref_path.name in _CACHE_ENTRIES_TO_IGNORE: continue - # A running download writes its ref before materialising the - # snapshot, so its ref is legitimately dangling while it transfers. - if repo_cache_dir_has_incomplete_blobs(repo_dir): - continue - ref_files = [entry for entry in refs_dir.rglob("*") if entry.is_file()] - except OSError: + commit = ref_path.read_text(encoding = "utf-8") + except (OSError, UnicodeDecodeError): + return None + # PurePath keeps the separator platform-native; huggingface_hub stores + # ref names the same way, so no manual posix normalisation here. + refs_by_commit.setdefault(commit, set()).add(str(ref_path.relative_to(refs_dir))) + return refs_by_commit + + +def _recover_repo_hidden_by_dangling_refs(repo_dir: Path) -> Optional[_RecoveredRepoInfo]: + """Rebuild the scan entry for a repo dropped *solely* over leftover refs. + + ``_scan_cached_repo`` assembles every revision successfully and only then + raises ``CorruptedCacheException`` because a ``refs/`` file names a + commit with no ``snapshots//`` dir, so ``scan_cache_dir`` omits an + entirely intact repo from ``.repos``. Studio creates that state itself: a + metadata probe that 404s writes ``refs/main`` at the live upstream HEAD + without materialising that snapshot, so any repo re-uploaded since it was + downloaded goes invisible to every inventory endpoint while the model + picker's plain directory walk still lists it. + + This reads the same directories huggingface_hub reads and writes nothing: + the ref file that upstream's assertion trips over is left exactly as it is. + Returns None whenever anything *other* than leftover refs would have failed + the upstream scan, so a genuinely corrupt repo stays omitted as before. + """ + identity = _hf_repo_identity(repo_dir.name) + if identity is None: + return None + repo_type, repo_id = identity + snapshots_dir = repo_dir / "snapshots" + refs_by_commit = _read_refs_by_commit(repo_dir / "refs") + if refs_by_commit is None: + return None + try: + if not snapshots_dir.is_dir(): + return None + snapshot_entries = sorted(snapshots_dir.iterdir()) + except OSError: + return None + + blob_stats: dict[Path, object] = {} + revisions: set[_RecoveredRevisionInfo] = set() + dangling = dict(refs_by_commit) + for snapshot in snapshot_entries: + if snapshot.name in _CACHE_ENTRIES_TO_IGNORE: continue - for ref in ref_files: + try: + if not snapshot.is_dir(): + # Upstream treats a file here as corruption; defer to it. + return None + entries = sorted(snapshot.rglob("*")) + except OSError: + return None + files: set[_RecoveredFileInfo] = set() + for entry in entries: try: - # snapshot_download writes refs/ before it fetches the - # first file, so a starting download has a live ref with neither - # a snapshot nor an .incomplete blob yet. Leave anything recent - # alone: a ref stranded by a re-upload persists, so skipping it - # for one sweep only defers the repair to the next one, while - # unlinking mid-download would strand the finished snapshot with - # no branch mapping and break later loads by revision. - if time.time() - ref.stat().st_mtime < _REF_SETTLE_SECONDS: + if entry.is_dir(): continue - commit = ref.read_text(encoding = "utf-8").strip() - except (OSError, UnicodeDecodeError): + blob_path = entry.resolve() + stat = blob_stats.get(blob_path) or blob_path.stat() + except OSError: + # Broken symlink / unreadable blob: upstream raises here too. + return None + blob_stats[blob_path] = stat + files.add( + _RecoveredFileInfo( + file_name = entry.name, + file_path = entry, + size_on_disk = stat.st_size, + blob_path = blob_path, + blob_last_accessed = stat.st_atime, + blob_last_modified = stat.st_mtime, + ) + ) + try: + last_modified = ( + max(f.blob_last_modified for f in files) if files else snapshot.stat().st_mtime + ) + except OSError: + return None + revisions.add( + _RecoveredRevisionInfo( + commit_hash = snapshot.name, + snapshot_path = snapshot, + size_on_disk = sum(blob_stats[blob].st_size for blob in {f.blob_path for f in files}), + files = frozenset(files), + refs = frozenset(dangling.pop(snapshot.name, set())), + last_modified = last_modified, + ) + ) + # A download writes refs/ before fetching its first file, so a + # repo with no snapshot yet is not downloaded and must not be reported as + # such -- that is the "already have it" lie this whole fix is about. + if not revisions: + return None + # Every ref resolved, so upstream did not drop this repo over leftover refs + # and either already returned it or failed for a reason we must not paper over. + if not dangling: + return None + try: + repo_stats = repo_dir.stat() + except OSError: + return None + return _RecoveredRepoInfo( + repo_id = repo_id, + repo_type = repo_type, + repo_path = repo_dir, + size_on_disk = sum(stat.st_size for stat in blob_stats.values()), + nb_files = len(blob_stats), + revisions = frozenset(revisions), + last_accessed = ( + max((stat.st_atime for stat in blob_stats.values()), default = repo_stats.st_atime) + ), + last_modified = ( + max((stat.st_mtime for stat in blob_stats.values()), default = repo_stats.st_mtime) + ), + ) + + +def _with_repos_hidden_by_dangling_refs(scan, cache_root: Path): + """Add back the repos ``scan_cache_dir`` dropped over a dangling ref.""" + try: + repo_dirs = sorted(entry for entry in cache_root.iterdir() if "--" in entry.name) + except OSError: + return scan + known = getattr(scan, "repos", ()) + scanned: set[str] = set() + for repo in known: + try: + scanned.add(str(Path(repo.repo_path).resolve(strict = False))) + except (AttributeError, OSError, RuntimeError, TypeError, ValueError): + continue + recovered: list[_RecoveredRepoInfo] = [] + for repo_dir in repo_dirs: + try: + if str(repo_dir.resolve(strict = False)) in scanned: continue - if not commit or (snapshots_dir / commit).is_dir(): - continue - try: - ref.unlink() - except OSError as exc: - logger.debug("Could not prune dangling HF ref %s: %s", ref, exc) - continue - logger.info("Pruned dangling HF cache ref %s -> %s (no snapshot on disk)", ref, commit) - removed += 1 - return removed + entry = _recover_repo_hidden_by_dangling_refs(repo_dir) + except (OSError, RuntimeError, ValueError): + continue + if entry is None: + continue + logger.info( + "Recovered HF cache repo %s hidden by a dangling ref (%d revision(s) on disk)", + entry.repo_id, + len(entry.revisions), + ) + recovered.append(entry) + if not recovered: + return scan + try: + return replace( + scan, + repos = frozenset(known) | frozenset(recovered), + size_on_disk = getattr(scan, "size_on_disk", 0) + + sum(entry.size_on_disk for entry in recovered), + ) + except (AttributeError, TypeError, ValueError) as exc: + # A scan shape we cannot rebuild is left untouched rather than dropped. + logger.debug("Could not attach recovered HF cache repos: %s", exc) + return scan def _compute_all_hf_cache_scans() -> list: @@ -199,15 +371,39 @@ def _compute_all_hf_cache_scans() -> list: try: scan = scan_cache_dir(cache_dir = str(cache_root)) # Only a warned-about scan can be hiding a repo, so a healthy cache - # is never walked twice and never written to. - if scan.warnings and _prune_dangling_hf_cache_refs(cache_root): - scan = scan_cache_dir(cache_dir = str(cache_root)) + # is never walked twice. getattr: a scan object without .warnings + # must not take the whole cache root down with it. + if getattr(scan, "warnings", None): + scan = _with_repos_hidden_by_dangling_refs(scan, cache_root) scans.append(scan) except Exception as exc: logger.warning("Could not scan HF cache %s: %s", cache_root, exc) return scans +def default_ref_resolves_on_disk(repo_dir: Path) -> bool: + """Whether ``refs/main`` names a snapshot that exists in *repo_dir*. + + When it does not, ``from_pretrained(repo_id)`` has nothing to resolve: an + offline load fails outright and an online one silently fetches the current + upstream HEAD instead of the snapshot already on disk. Callers use this to + hand out the snapshot path as the load identity instead of the repo id. + """ + ref_path = repo_dir / "refs" / "main" + try: + # No strip: huggingface_hub matches the raw ref contents against the + # snapshot dir name, so a ref with stray whitespace resolves nowhere. + commit = ref_path.read_text(encoding = "utf-8") + except (OSError, UnicodeDecodeError): + return False + if not commit: + return False + try: + return (repo_dir / "snapshots" / commit).is_dir() + except (OSError, ValueError): + return False + + def token_fingerprint(hf_token: Optional[str]) -> str: """16-char SHA256 prefix used as a cache-key qualifier for gated repos. diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 9687003380..b838fe06ac 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -7,13 +7,18 @@ commit with no ``snapshots//`` directory and omits it from ``.repos``, so the model stays visible in the model picker (a plain directory walk) while disappearing from every Hub inventory endpoint that feeds chat auto-load. + +The repair is read-only: the hidden repo is rebuilt from the same directories +huggingface_hub reads and the ref file is left exactly as it is, because +``_cache_commit_hash_for_specific_revision`` writes refs with an unlocked +in-place ``write_text``, so no external process can delete one race-free. """ from __future__ import annotations +import dataclasses import os import stat -import time from pathlib import Path import pytest @@ -27,33 +32,30 @@ UPSTREAM_HEAD = "b" * 40 def _build_repo( cache_root: Path, *, - ref: str, + ref: str | None = UPSTREAM_HEAD, extra_refs: dict[str, str] | None = None, - incomplete: bool = False, name: str = "models--Org--Model", - settled: bool = True, + payload: bytes = b"\0" * 11, + snapshots: tuple[str, ...] = (SNAPSHOT,), ) -> Path: - """A cache repo holding one snapshot at ``SNAPSHOT``, shaped like HF's.""" + """A cache repo shaped like HF's, using regular files rather than symlinks. + + ``_scan_cached_repo`` resolves each snapshot entry to its blob, and a + regular file resolves to itself, so this exercises the real scanner while + staying runnable on Windows without the symlink privilege. + """ repo_dir = cache_root / name - blobs = repo_dir / "blobs" - snapshot = repo_dir / "snapshots" / SNAPSHOT refs = repo_dir / "refs" - for directory in (blobs, snapshot, refs): - directory.mkdir(parents = True, exist_ok = True) - blob = blobs / ("c" * 40) - blob.write_bytes(b"\0") - os.symlink(os.path.relpath(blob, snapshot), snapshot / "Q4_K_M.gguf") - (refs / "main").write_text(ref, encoding = "utf-8") - for name, commit in (extra_refs or {}).items(): - (refs / name).write_text(commit, encoding = "utf-8") - if incomplete: - (blobs / "d0d0d0d0.incomplete").write_bytes(b"partial") - if settled: - # The sweep leaves a just-written ref alone, so age these past that - # window to exercise the pruning rather than the settle guard. - old = time.time() - 3600 - for entry in refs.rglob("*"): - os.utime(entry, (old, old)) + refs.mkdir(parents = True, exist_ok = True) + (repo_dir / "blobs").mkdir(parents = True, exist_ok = True) + for commit in snapshots: + snapshot = repo_dir / "snapshots" / commit + snapshot.mkdir(parents = True, exist_ok = True) + (snapshot / "model.safetensors").write_bytes(payload) + if ref is not None: + (refs / "main").write_text(ref, encoding = "utf-8") + for ref_name, commit in (extra_refs or {}).items(): + (refs / ref_name).write_text(commit, encoding = "utf-8") return repo_dir @@ -61,21 +63,90 @@ def _ref_names(repo_dir: Path) -> list[str]: return sorted(entry.name for entry in (repo_dir / "refs").rglob("*") if entry.is_file()) -def _scanned_repo_ids(cache_root: Path, monkeypatch) -> list[str]: +def _scan(cache_root: Path, monkeypatch) -> list: monkeypatch.setattr(inventory_scan, "hf_cache_roots", lambda: [cache_root]) - return [ - repo.repo_id for scan in inventory_scan._compute_all_hf_cache_scans() for repo in scan.repos - ] + return inventory_scan._compute_all_hf_cache_scans() + + +def _scanned_repo_ids(cache_root: Path, monkeypatch) -> list[str]: + return sorted(repo.repo_id for scan in _scan(cache_root, monkeypatch) for repo in scan.repos) + + +def _only_repo(cache_root: Path, monkeypatch): + repos = [repo for scan in _scan(cache_root, monkeypatch) for repo in scan.repos] + assert [repo.repo_id for repo in repos] == ["Org/Model"], "repo hidden from the scan" + return repos[0] + + +# --- the #7374 symptom ------------------------------------------------------- + + +def test_huggingface_hub_really_hides_a_repo_behind_a_dangling_ref(tmp_path): + """Baseline for the bug: the snapshot is intact, yet the repo is dropped.""" + from huggingface_hub import scan_cache_dir + + repo_dir = _build_repo(tmp_path) + + raw = scan_cache_dir(cache_dir = str(tmp_path)) + + assert [repo.repo_id for repo in raw.repos] == [] + assert raw.warnings + assert (repo_dir / "snapshots" / SNAPSHOT / "model.safetensors").is_file() def test_dangling_ref_no_longer_hides_an_intact_repo(tmp_path, monkeypatch): - repo_dir = _build_repo(tmp_path, ref = UPSTREAM_HEAD) + repo_dir = _build_repo(tmp_path) - assert _scanned_repo_ids(tmp_path, monkeypatch) == ["Org/Model"] - assert _ref_names(repo_dir) == [] + repo = _only_repo(tmp_path, monkeypatch) + + assert repo.repo_id == "Org/Model" + assert repo.repo_type == "model" + # routes/models.py does repo_path.parent, so this must stay a real Path. + assert isinstance(repo.repo_path, Path) and repo.repo_path == repo_dir + assert repo.size_on_disk == 11 + # The recovered revision keeps the identity the snapshot is loadable by. + revision = next(iter(repo.revisions)) + assert revision.commit_hash == SNAPSHOT + assert revision.snapshot_path == repo_dir / "snapshots" / SNAPSHOT + assert {f.file_name for f in revision.files} == {"model.safetensors"} + # The dangling ref resolves to nothing, so it maps to no revision... + assert revision.refs == frozenset() + # ...and, crucially, is still on disk: the repair never writes to the cache. + assert _ref_names(repo_dir) == ["main"] + assert (repo_dir / "refs" / "main").read_text(encoding = "utf-8") == UPSTREAM_HEAD -def test_healthy_cache_is_neither_pruned_nor_rescanned(tmp_path, monkeypatch): +def test_recovery_reads_a_multi_file_multi_revision_repo(tmp_path, monkeypatch): + other = "c" * 40 + repo_dir = _build_repo(tmp_path, snapshots = (SNAPSHOT, other)) + (repo_dir / "snapshots" / SNAPSHOT / "nested").mkdir() + (repo_dir / "snapshots" / SNAPSHOT / "nested" / "extra.json").write_bytes(b"{}") + + repo = _only_repo(tmp_path, monkeypatch) + + assert {rev.commit_hash for rev in repo.revisions} == {SNAPSHOT, other} + files = {f.file_path for rev in repo.revisions for f in rev.files} + assert repo_dir / "snapshots" / SNAPSHOT / "nested" / "extra.json" in files + # _resolve_cached_model_path relativises file_path against snapshot_path. + for rev in repo.revisions: + for f in rev.files: + assert f.file_path.relative_to(rev.snapshot_path) + + +def test_a_still_resolvable_ref_keeps_its_revision_mapping(tmp_path, monkeypatch): + """One good ref plus one stale ref: hub drops the repo, we keep the mapping.""" + repo_dir = _build_repo(tmp_path, ref = SNAPSHOT, extra_refs = {"stale": UPSTREAM_HEAD}) + + repo = _only_repo(tmp_path, monkeypatch) + + assert next(iter(repo.revisions)).refs == frozenset({"main"}) + assert _ref_names(repo_dir) == ["main", "stale"] + + +# --- the repair must not widen past the leftover-refs assertion -------------- + + +def test_a_healthy_cache_is_returned_untouched(tmp_path, monkeypatch): import huggingface_hub repo_dir = _build_repo(tmp_path, ref = SNAPSHOT, extra_refs = {"v1.0": SNAPSHOT}) @@ -93,60 +164,175 @@ def test_healthy_cache_is_neither_pruned_nor_rescanned(tmp_path, monkeypatch): assert _ref_names(repo_dir) == ["main", "v1.0"] -def test_in_flight_download_keeps_its_dangling_ref(tmp_path, monkeypatch): - """A download writes its ref before the snapshot lands, so it owns it.""" - repo_dir = _build_repo(tmp_path, ref = UPSTREAM_HEAD, incomplete = True) +def test_a_download_that_has_only_written_its_ref_is_not_invented(tmp_path, monkeypatch): + """snapshot_download writes refs/ before fetching the first file. + + There is no snapshot to recover yet, so nothing must be reported as + downloaded -- that would be the very "already have it" lie #7374 is about. + """ + repo_dir = tmp_path / "models--Org--Model" + (repo_dir / "snapshots").mkdir(parents = True) + (repo_dir / "refs").mkdir(parents = True) + (repo_dir / "refs" / "main").write_text(UPSTREAM_HEAD, encoding = "utf-8") assert _scanned_repo_ids(tmp_path, monkeypatch) == [] assert _ref_names(repo_dir) == ["main"] -def test_only_the_dangling_ref_of_a_mixed_repo_is_pruned(tmp_path, monkeypatch): - repo_dir = _build_repo(tmp_path, ref = SNAPSHOT, extra_refs = {"stale": UPSTREAM_HEAD}) +def test_a_repo_corrupted_beyond_a_dangling_ref_stays_omitted(tmp_path, monkeypatch): + """A broken snapshot symlink is corruption hub rejects for its own reasons.""" + repo_dir = _build_repo(tmp_path) + broken = repo_dir / "snapshots" / SNAPSHOT / "weights.bin" + try: + os.symlink(repo_dir / "blobs" / "missing", broken) + except (NotImplementedError, OSError): + pytest.skip("symlinks unavailable (Windows without developer mode)") - assert _scanned_repo_ids(tmp_path, monkeypatch) == ["Org/Model"] - assert _ref_names(repo_dir) == ["main"] + assert _scanned_repo_ids(tmp_path, monkeypatch) == [] -def test_an_unreadable_repo_does_not_abort_the_prune_sweep(tmp_path): - """is_dir() propagates a permission error instead of returning False, so an - unguarded probe would escape to the caller's whole-scan except and drop - every model in the cache. The sweep must skip that repo and carry on. +def test_an_unrelated_repo_is_never_disturbed(tmp_path, monkeypatch): + hidden = _build_repo(tmp_path) + healthy = _build_repo(tmp_path, ref = SNAPSHOT, name = "models--Org--Healthy") - Scoped to the sweep because scan_cache_dir itself already raises on an + assert _scanned_repo_ids(tmp_path, monkeypatch) == ["Org/Healthy", "Org/Model"] + assert _ref_names(hidden) == ["main"] + assert _ref_names(healthy) == ["main"] + + +def test_an_unreadable_repo_does_not_abort_the_recovery(tmp_path): + """One unreadable repo must not stop the others being recovered. + + Scoped to the recovery pass because scan_cache_dir itself raises on an unreadable repo dir, which is upstream of this code and unchanged here. """ - dangling = _build_repo(tmp_path, ref = UPSTREAM_HEAD) + from huggingface_hub import HFCacheInfo + + hidden = _build_repo(tmp_path) locked = _build_repo(tmp_path, ref = SNAPSHOT, name = "models--Org--Locked") locked.chmod(0) if os.access(locked / "refs", os.R_OK): pytest.skip("filesystem does not enforce directory permissions") try: - assert inventory_scan._prune_dangling_hf_cache_refs(tmp_path) == 1 - assert _ref_names(dangling) == [] + scan = HFCacheInfo(size_on_disk = 0, repos = frozenset(), warnings = []) + merged = inventory_scan._with_repos_hidden_by_dangling_refs(scan, tmp_path) + assert sorted(repo.repo_id for repo in merged.repos) == ["Org/Model"] + assert _ref_names(hidden) == ["main"] finally: locked.chmod(stat.S_IRWXU) -def test_unwritable_refs_dir_degrades_instead_of_raising(tmp_path, monkeypatch): - repo_dir = _build_repo(tmp_path, ref = UPSTREAM_HEAD) - refs_dir = repo_dir / "refs" - refs_dir.chmod(stat.S_IRUSR | stat.S_IXUSR) - if os.access(refs_dir, os.W_OK): - pytest.skip("filesystem does not enforce directory write permissions") +def test_a_non_repo_directory_is_ignored(tmp_path, monkeypatch): + _build_repo(tmp_path) + (tmp_path / ".locks").mkdir() + (tmp_path / "notarepo").mkdir() + (tmp_path / "spaces--Org--Thing").mkdir() + + assert _scanned_repo_ids(tmp_path, monkeypatch) == ["Org/Model"] + + +def test_a_scan_object_without_warnings_still_reaches_the_caller(tmp_path, monkeypatch): + """The gate must read .warnings defensively: an AttributeError here lands in + the per-root ``except Exception`` and silently blanks the whole cache.""" + from types import SimpleNamespace + + import huggingface_hub + + monkeypatch.setattr( + huggingface_hub, + "scan_cache_dir", + lambda cache_dir = None: SimpleNamespace(cache_dir = cache_dir), + ) + + assert len(_scan(tmp_path, monkeypatch)) == 1 + + +# --- version robustness ------------------------------------------------------ + + +def test_recovered_entries_match_the_huggingface_hub_field_surface(): + """The recovered entries are duck-typed rather than built with hub's own + constructors, so nothing breaks when a field is added or removed upstream. + This is the tripwire that says the surfaces have drifted.""" + from huggingface_hub import CachedFileInfo, CachedRepoInfo, CachedRevisionInfo + + pairs = ( + (inventory_scan._RecoveredFileInfo, CachedFileInfo), + (inventory_scan._RecoveredRevisionInfo, CachedRevisionInfo), + (inventory_scan._RecoveredRepoInfo, CachedRepoInfo), + ) + for ours, theirs in pairs: + missing = {f.name for f in dataclasses.fields(theirs)} - { + f.name for f in dataclasses.fields(ours) + } + assert not missing, f"{ours.__name__} is missing {sorted(missing)}" + + +def test_a_recovered_repo_survives_delete_revisions(tmp_path, monkeypatch): + """Deleting a recovered model routes through HFCacheInfo.delete_revisions, + which keys a dict by repo and takes a set difference over .revisions.""" + repo_dir = _build_repo(tmp_path) + scan = _scan(tmp_path, monkeypatch)[0] + + strategy = scan.delete_revisions(SNAPSHOT) + + assert strategy.repos == frozenset({repo_dir}) + assert strategy.expected_freed_size == 11 + + +# --- load identity for a recovered snapshot ---------------------------------- + + +def _autoload_rows(cache_root: Path, monkeypatch) -> list[dict]: + """What chat auto-load sees: GET /api/hub/cached-models.""" + from hub.services.models import cache_inventory + from types import SimpleNamespace + + monkeypatch.setattr(inventory_scan, "hf_cache_roots", lambda: [cache_root]) + monkeypatch.setattr( + "utils.hf_cache_settings.get_hf_cache_paths", + lambda: SimpleNamespace(hub_cache = cache_root), + ) + inventory_scan.invalidate_hf_cache_scans() try: - assert _scanned_repo_ids(tmp_path, monkeypatch) == [] - assert _ref_names(repo_dir) == ["main"] + return cache_inventory._scan_cached_models() finally: - refs_dir.chmod(stat.S_IRWXU) + inventory_scan.invalidate_hf_cache_scans() -def test_a_just_written_ref_survives_the_sweep(tmp_path, monkeypatch): - """snapshot_download writes refs/ before fetching the first file, - so there is a window with a live ref, no snapshot and no .incomplete blob. - Unlinking there strands the finished snapshot with no branch mapping and - breaks later local_files_only loads by revision.""" - repo_dir = _build_repo(tmp_path, ref = UPSTREAM_HEAD, settled = False) +def test_auto_load_sees_a_model_hidden_behind_a_dangling_ref(tmp_path, monkeypatch): + """The #7374 symptom end to end: the snapshot is on disk and the picker + lists it, but auto-load's inventory reported nothing downloaded and the app + fell through to a fresh download.""" + repo_dir = _build_repo(tmp_path) + snapshot = repo_dir / "snapshots" / SNAPSHOT + (snapshot / "config.json").write_text("{}", encoding = "utf-8") - assert inventory_scan._prune_dangling_hf_cache_refs(tmp_path) == 0 - assert _ref_names(repo_dir) == ["main"] + rows = _autoload_rows(tmp_path, monkeypatch) + + assert [row["repo_id"] for row in rows] == ["Org/Model"] + # Recovered rows must carry the snapshot as their load identity: refs/main + # dangles, so from_pretrained("Org/Model") would fail offline and download + # the current upstream HEAD online instead of using what is already here. + assert rows[0]["load_id"] == str(snapshot) + assert rows[0]["active_cache"] is True + + +def test_a_resolvable_repo_still_loads_by_repo_id(tmp_path, monkeypatch): + repo_dir = _build_repo(tmp_path, ref = SNAPSHOT) + (repo_dir / "snapshots" / SNAPSHOT / "config.json").write_text("{}", encoding = "utf-8") + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert [row["repo_id"] for row in rows] == ["Org/Model"] + assert rows[0]["load_id"] == "Org/Model" + + +def test_default_ref_resolves_only_when_main_names_a_snapshot(tmp_path): + dangling = _build_repo(tmp_path, name = "models--Org--Dangling") + resolved = _build_repo(tmp_path, ref = SNAPSHOT, name = "models--Org--Resolved") + detached = _build_repo(tmp_path, ref = None, name = "models--Org--Detached") + + assert inventory_scan.default_ref_resolves_on_disk(dangling) is False + assert inventory_scan.default_ref_resolves_on_disk(resolved) is True + assert inventory_scan.default_ref_resolves_on_disk(detached) is False From 0705a297bdb313fec3c680d17b2d98e6cd7b00d8 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 28 Jul 2026 13:29:48 +0000 Subject: [PATCH 07/29] Stop the structlog stub shadowing the real package for PR #7375 The bare setdefault parked an empty placeholder before anything imported the real structlog, so every later module calling structlog.get_logger at import time raised AttributeError. It only bit when this file was collected first, which is why the 8 hardware-dispatch cases passed alone and failed under pytest tests/studio. Only stub when the package is genuinely absent. --- .../load_freeze/test_load_orchestrator.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/studio/load_freeze/test_load_orchestrator.py b/tests/studio/load_freeze/test_load_orchestrator.py index a1f4caa309..0ff769fc78 100644 --- a/tests/studio/load_freeze/test_load_orchestrator.py +++ b/tests/studio/load_freeze/test_load_orchestrator.py @@ -49,7 +49,22 @@ import logging as _logging # noqa: E402 _loggers_stub = types.ModuleType("loggers") _loggers_stub.get_logger = lambda name: _logging.getLogger(name) sys.modules.setdefault("loggers", _loggers_stub) -sys.modules.setdefault("structlog", types.ModuleType("structlog")) +# structlog is a hard studio.txt requirement, but it is only imported lazily, so a +# bare setdefault here used to park an empty placeholder BEFORE anything imported +# the real package -- and it then shadowed it for the rest of the session. Every +# later file importing a studio module that calls structlog.get_logger at module +# scope (routes.inference -> core.inference.external_provider, utils.mlx_repair) +# blew up with AttributeError, but only when this file was collected first, so the +# same test passed alone and failed under `pytest tests/studio`. Only stub when the +# package is genuinely missing, and give the stub the attribute those callers use. +try: + import structlog # noqa: E402, F401 +except ImportError: + _structlog_stub = types.ModuleType("structlog") + _structlog_stub.get_logger = lambda *args, **kwargs: _logging.getLogger( + args[0] if args else "structlog" + ) + sys.modules["structlog"] = _structlog_stub import httpx # noqa: E402 From 691b9fd1d153d616c6840ed4c0d465d56eda7bf6 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 28 Jul 2026 14:17:27 +0000 Subject: [PATCH 08/29] Bind the cache load id to a snapshot that holds the payload The inventory row pools weights over every revision in a repo, so a repo with more than one snapshot can advertise a payload that lives in an older one: a config-only metadata probe against a moved commit leaves a newer, weightless snapshot beside the download. The load id was simply the newest snapshot dir, so from_pretrained() found no weights and GGUF variant resolution, which reads only that one directory, returned no quant. Auto-load then failed on a model that was fully cached, which is the failure this branch is meant to remove. Pick the newest snapshot that classifies, on its own, as the format the row advertises (or, for GGUF, that holds a primary GGUF), and fall back to today's newest-snapshot pick when none does. Discovery stays repo-wide so a payload sitting in an older revision is still reported as downloaded, matching how _repo_gguf_load_id already chooses a snapshot for the copied run command. Tests cover the safetensors and GGUF cases plus a repo whose newest snapshot is runnable, which must still win. --- .../hub/services/models/cache_inventory.py | 106 +++++++++++++---- .../tests/test_hf_cache_dangling_refs.py | 107 ++++++++++++++++++ 2 files changed, 189 insertions(+), 24 deletions(-) diff --git a/studio/backend/hub/services/models/cache_inventory.py b/studio/backend/hub/services/models/cache_inventory.py index cdc43316e9..fafef0e118 100644 --- a/studio/backend/hub/services/models/cache_inventory.py +++ b/studio/backend/hub/services/models/cache_inventory.py @@ -270,6 +270,9 @@ def _cache_inventory_fields( partial: bool = False, requires_variant: bool = False, ) -> dict: + # *snapshot_path* becomes the load identity whenever the repo id will not + # resolve, so callers must pass a snapshot that holds the payload this row + # advertises, not merely the newest one. load_id = repo_id active_cache = True if repo_path is not None: @@ -380,7 +383,7 @@ def _scan_cached_gguf() -> list[dict]: repo_id, "gguf", repo_path = repo_path, - snapshot_path = snapshot_path, + snapshot_path = _repo_gguf_payload_snapshot(repo_info) or snapshot_path, active_hub_cache = active_hub_cache, partial = bool(row["partial"]), requires_variant = True, @@ -428,6 +431,58 @@ class _CachedNonGgufPayload(NamedTuple): has_runnable_weights: bool model_format: ModelFormat last_modified: float + payload_snapshot: Optional[Path] + + +# Keys mirror _classify_non_gguf_model_format's keyword arguments so a revision's +# flags can be classified on their own, exactly as the whole repo's are. +_PAYLOAD_FLAGS = ( + "has_config", + "has_adapter_config", + "has_adapter_weights", + "has_safetensors", + "has_transformers_safetensors", + "has_checkpoint_weights", +) + + +def _newest_snapshot_dir(candidates) -> Optional[Path]: + """Newest of *candidates* by directory mtime, or None when there are none. + + Ordering and resolution match ``_cached_model_snapshot_path`` and + ``hub.utils.gguf.iter_hf_cache_snapshots`` so every consumer names the same + directory by the same string. + """ + best: Optional[tuple[float, Path]] = None + for candidate in candidates: + path = Path(candidate) + try: + mtime = path.stat().st_mtime + except OSError: + mtime = 0.0 + if best is None or mtime > best[0]: + best = (mtime, path) + if best is None: + return None + try: + return best[1].resolve() + except OSError: + return best[1] + + +def _repo_gguf_payload_snapshot(repo_info) -> Optional[Path]: + """Newest snapshot dir holding a primary GGUF, or None when none does. + + The row's size sums quants over every revision, while local variant + resolution reads only the directory handed out as ``load_id``, so the two + must agree or an advertised quant resolves to nothing. + """ + return _newest_snapshot_dir( + snapshot + for revision in repo_info.revisions + if (snapshot := getattr(revision, "snapshot_path", None)) is not None + and any(_is_main_gguf_filename(f.file_name) for f in revision.files) + ) def _repo_non_gguf_model_payload(repo_info) -> _CachedNonGgufPayload: @@ -435,12 +490,8 @@ def _repo_non_gguf_model_payload(repo_info) -> _CachedNonGgufPayload: adapter_blobs: dict[str, tuple[int, float]] = {} safetensors_blobs: dict[str, tuple[int, float]] = {} checkpoint_blobs: dict[str, tuple[int, float]] = {} - has_config = False - has_adapter_config = False - has_adapter_weights = False - has_safetensors = False - has_transformers_safetensors = False - has_checkpoint = False + repo_flags = dict.fromkeys(_PAYLOAD_FLAGS, False) + revision_flags: list[tuple[Path, dict[str, bool]]] = [] def _record_blob( target: dict[str, tuple[int, float]], file_obj, rev_id: str, file_name: str @@ -454,6 +505,7 @@ def _repo_non_gguf_model_payload(repo_info) -> _CachedNonGgufPayload: for revision in repo_info.revisions: rev_id = getattr(revision, "commit_hash", None) or str(id(revision)) + flags = dict.fromkeys(_PAYLOAD_FLAGS, False) for f in revision.files: file_name = str(f.file_name) lower = file_name.lower() @@ -461,37 +513,34 @@ def _repo_non_gguf_model_payload(repo_info) -> _CachedNonGgufPayload: if _is_gguf_filename(lower): continue if name == "config.json": - has_config = True + flags["has_config"] = True continue if name == "adapter_config.json": - has_adapter_config = True + flags["has_adapter_config"] = True continue is_adapter = _is_adapter_weight_name(name) is_safetensors = name.endswith(".safetensors") and not is_adapter is_checkpoint = _is_checkpoint_weight_name(name) if is_adapter: - has_adapter_weights = True + flags["has_adapter_weights"] = True _record_blob(adapter_blobs, f, rev_id, file_name) if is_safetensors: - has_safetensors = True + flags["has_safetensors"] = True if _is_transformers_safetensors_weight_name(name): - has_transformers_safetensors = True + flags["has_transformers_safetensors"] = True _record_blob(safetensors_blobs, f, rev_id, file_name) if is_checkpoint: - has_checkpoint = True + flags["has_checkpoint_weights"] = True _record_blob(checkpoint_blobs, f, rev_id, file_name) + snapshot = getattr(revision, "snapshot_path", None) + if snapshot is not None: + revision_flags.append((Path(snapshot), flags)) + for key, seen in flags.items(): + if seen: + repo_flags[key] = True model_format = ( - _classify_non_gguf_model_format( - has_config = has_config, - has_adapter_config = has_adapter_config, - has_adapter_weights = has_adapter_weights, - has_safetensors = has_safetensors, - has_transformers_safetensors = has_transformers_safetensors, - has_checkpoint_weights = has_checkpoint, - trusted_hf_cache_repo = True, - ) - or "unknown" + _classify_non_gguf_model_format(**repo_flags, trusted_hf_cache_repo = True) or "unknown" ) if model_format == "adapter": selected_blobs = adapter_blobs @@ -507,6 +556,15 @@ def _repo_non_gguf_model_payload(repo_info) -> _CachedNonGgufPayload: has_runnable_weights = model_format != "unknown", model_format = model_format, last_modified = max((mtime for _size, mtime in selected_blobs.values()), default = 0.0), + # Weights are pooled across revisions, so the newest snapshot need not + # hold any: the load id has to name one that classifies the same way on + # its own, otherwise the load fails on a fully cached model. + payload_snapshot = _newest_snapshot_dir( + snapshot + for snapshot, flags in revision_flags + if _classify_non_gguf_model_format(**flags, trusted_hf_cache_repo = True) + == model_format + ), ) @@ -670,7 +728,7 @@ def _scan_cached_models() -> list[dict]: repo_id, payload.model_format, repo_path = repo_path, - snapshot_path = snapshot_path, + snapshot_path = payload.payload_snapshot or snapshot_path, active_hub_cache = active_hub_cache, partial = bool(row["partial"]), ) diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index b838fe06ac..45c360fd80 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -336,3 +336,110 @@ def test_default_ref_resolves_only_when_main_names_a_snapshot(tmp_path): assert inventory_scan.default_ref_resolves_on_disk(dangling) is False assert inventory_scan.default_ref_resolves_on_disk(resolved) is True assert inventory_scan.default_ref_resolves_on_disk(detached) is False + + +# --- the load id must name a snapshot that holds the advertised payload ------ + +OLDER = "d" * 40 +NEWER = "e" * 40 + + +def _age(path: Path, seconds: float) -> None: + """Backdate a snapshot dir; snapshot selection orders by directory mtime.""" + stamp = os.stat(path).st_mtime - seconds + os.utime(path, (stamp, stamp)) + + +def _autoload_gguf_rows(cache_root: Path, monkeypatch) -> list[dict]: + """What chat auto-load sees for GGUF: GET /api/hub/cached-gguf.""" + from hub.services.models import cache_inventory + from types import SimpleNamespace + + monkeypatch.setattr(inventory_scan, "hf_cache_roots", lambda: [cache_root]) + monkeypatch.setattr( + "utils.hf_cache_settings.get_hf_cache_paths", + lambda: SimpleNamespace(hub_cache = cache_root), + ) + inventory_scan.invalidate_hf_cache_scans() + try: + return cache_inventory._scan_cached_gguf() + finally: + inventory_scan.invalidate_hf_cache_scans() + + +def _two_snapshot_repo(cache_root: Path, older_files: dict, newer_files: dict) -> Path: + """A repo whose payload sits in the older of two snapshots. + + Realistic because a metadata probe (config.json only) against a commit that + has moved on materialises a newer, weightless snapshot beside the download. + """ + repo_dir = cache_root / "models--Org--Model" + (repo_dir / "blobs").mkdir(parents = True, exist_ok = True) + (repo_dir / "refs").mkdir(parents = True, exist_ok = True) + (repo_dir / "refs" / "main").write_text(UPSTREAM_HEAD, encoding = "utf-8") + for commit, files in ((OLDER, older_files), (NEWER, newer_files)): + snapshot = repo_dir / "snapshots" / commit + snapshot.mkdir(parents = True, exist_ok = True) + for name, payload in files.items(): + (snapshot / name).write_bytes(payload) + _age(repo_dir / "snapshots" / OLDER, 600) + return repo_dir + + +def test_load_id_names_the_snapshot_holding_the_safetensors_payload(tmp_path, monkeypatch): + """The row aggregates weights over every revision, so the payload it + advertises can live in an older snapshot than the newest directory.""" + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}"}, + ) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert [row["repo_id"] for row in rows] == ["Org/Model"] + assert rows[0]["model_format"] == "safetensors" + load_dir = Path(rows[0]["load_id"]) + # Behavioural: from_pretrained(load_id) has to find the weights the row + # advertised, otherwise auto-load fails on a model that is fully cached. + assert any(entry.suffix == ".safetensors" for entry in load_dir.iterdir()), ( + f"load_id {load_dir.name} holds no weights; payload is in {OLDER[:8]}" + ) + assert load_dir == repo_dir / "snapshots" / OLDER + + +def test_load_id_names_the_snapshot_holding_the_advertised_gguf_quant(tmp_path, monkeypatch): + """Same for GGUF: the row's size sums quants across revisions, while local + variant resolution only ever reads the one directory in ``load_id``.""" + from hub.utils.gguf import list_local_gguf_variants + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}", "Model-Q4_K_M.gguf": b"\0" * 32}, + newer_files = {"config.json": b"{}"}, + ) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + assert [row["repo_id"] for row in rows] == ["Org/Model"] + assert rows[0]["size_bytes"] == 32 + load_dir = Path(rows[0]["load_id"]) + variants, _has_vision = list_local_gguf_variants(str(load_dir)) + assert [v.quant for v in variants] == ["Q4_K_M"], ( + f"no variant resolves under load_id {load_dir.name}; the quant is in {OLDER[:8]}" + ) + assert load_dir == repo_dir / "snapshots" / OLDER + + +def test_load_id_still_prefers_the_newest_snapshot_that_holds_the_payload(tmp_path, monkeypatch): + """The payload rule must not pin loads to stale revisions: when both + snapshots are runnable, the newest one still wins.""" + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}", "model.safetensors": b"\0" * 13}, + ) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER From 525906af89e0497201308c50e961dddd84bcaee4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:18:20 +0000 Subject: [PATCH 09/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../backend/hub/services/models/cache_inventory.py | 3 +-- studio/backend/tests/test_hf_cache_dangling_refs.py | 12 ++++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/studio/backend/hub/services/models/cache_inventory.py b/studio/backend/hub/services/models/cache_inventory.py index fafef0e118..d842ce0a4c 100644 --- a/studio/backend/hub/services/models/cache_inventory.py +++ b/studio/backend/hub/services/models/cache_inventory.py @@ -562,8 +562,7 @@ def _repo_non_gguf_model_payload(repo_info) -> _CachedNonGgufPayload: payload_snapshot = _newest_snapshot_dir( snapshot for snapshot, flags in revision_flags - if _classify_non_gguf_model_format(**flags, trusted_hf_cache_repo = True) - == model_format + if _classify_non_gguf_model_format(**flags, trusted_hf_cache_repo = True) == model_format ), ) diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 45c360fd80..bb76234f3d 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -402,9 +402,9 @@ def test_load_id_names_the_snapshot_holding_the_safetensors_payload(tmp_path, mo load_dir = Path(rows[0]["load_id"]) # Behavioural: from_pretrained(load_id) has to find the weights the row # advertised, otherwise auto-load fails on a model that is fully cached. - assert any(entry.suffix == ".safetensors" for entry in load_dir.iterdir()), ( - f"load_id {load_dir.name} holds no weights; payload is in {OLDER[:8]}" - ) + assert any( + entry.suffix == ".safetensors" for entry in load_dir.iterdir() + ), f"load_id {load_dir.name} holds no weights; payload is in {OLDER[:8]}" assert load_dir == repo_dir / "snapshots" / OLDER @@ -425,9 +425,9 @@ def test_load_id_names_the_snapshot_holding_the_advertised_gguf_quant(tmp_path, assert rows[0]["size_bytes"] == 32 load_dir = Path(rows[0]["load_id"]) variants, _has_vision = list_local_gguf_variants(str(load_dir)) - assert [v.quant for v in variants] == ["Q4_K_M"], ( - f"no variant resolves under load_id {load_dir.name}; the quant is in {OLDER[:8]}" - ) + assert [v.quant for v in variants] == [ + "Q4_K_M" + ], f"no variant resolves under load_id {load_dir.name}; the quant is in {OLDER[:8]}" assert load_dir == repo_dir / "snapshots" / OLDER From c0a4bb1bcbd6087f6a86d3c4542d9c498988507a Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 28 Jul 2026 21:53:54 +0000 Subject: [PATCH 10/29] Probe structlog with find_spec instead of a bare import for PR #7375 The availability check imported the module purely for its side effect, so the import-hoist verifier flagged it as an added-but-unused import and failed Source lint. find_spec answers the same question without binding a name. --- tests/studio/load_freeze/test_load_orchestrator.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/studio/load_freeze/test_load_orchestrator.py b/tests/studio/load_freeze/test_load_orchestrator.py index 0ff769fc78..2957150a2c 100644 --- a/tests/studio/load_freeze/test_load_orchestrator.py +++ b/tests/studio/load_freeze/test_load_orchestrator.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import importlib.util import os import re import socket @@ -57,9 +58,7 @@ sys.modules.setdefault("loggers", _loggers_stub) # blew up with AttributeError, but only when this file was collected first, so the # same test passed alone and failed under `pytest tests/studio`. Only stub when the # package is genuinely missing, and give the stub the attribute those callers use. -try: - import structlog # noqa: E402, F401 -except ImportError: +if importlib.util.find_spec("structlog") is None: _structlog_stub = types.ModuleType("structlog") _structlog_stub.get_logger = lambda *args, **kwargs: _logging.getLogger( args[0] if args else "structlog" From 44726e4bb119274e40ddf7398e6d9616f20e445c Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 28 Jul 2026 23:49:29 +0000 Subject: [PATCH 11/29] Judge the load id against where the repo id would actually resolve The payload binding only fired when refs/main dangled, but the scenario that strands weights in an older snapshot is a metadata probe against a moved commit, and that probe writes refs/main too. So refs/main resolved, load_id stayed the repo id, and from_pretrained() landed on the weightless snapshot the probe left behind: auto-load failed or re-downloaded a model that was cached. Resolve refs/main to a directory and keep the repo id only when that directory is one of the snapshots holding the advertised payload. A GGUF snapshot holding one shard of an interrupted split download is not a usable payload either, since the picker still offers that quant and the command asks llama-server for shards that are absent. Prefer a snapshot whose offered quants are all on disk, reusing snapshot_variants_all_complete the way _repo_gguf_load_id already does, and fall back to any primary GGUF when none is complete. The predicate moves next to the completed-variant walk it is built on; routes.models keeps the name for its existing callers. Finally, the row-level partial walk still used the newest snapshot while the load id now names an older complete one, so the same weightless probe snapshot flagged the row partial, flipped can_chat off and made auto-load skip a model that loads fine. Pin that walk to the snapshot the row advertises; the per-variant path already had an equivalent cross-snapshot fallback. Tests cover all three, plus a refs/main that does hold the payload staying on the repo id and a truly truncated download still reporting partial. --- .../hub/services/models/cache_inventory.py | 85 ++++++++--- .../backend/hub/tests/test_model_services.py | 10 +- studio/backend/hub/utils/inventory_scan.py | 71 +++++++-- studio/backend/routes/models.py | 22 +-- .../tests/test_hf_cache_dangling_refs.py | 138 +++++++++++++++++- 5 files changed, 263 insertions(+), 63 deletions(-) diff --git a/studio/backend/hub/services/models/cache_inventory.py b/studio/backend/hub/services/models/cache_inventory.py index d842ce0a4c..e3252eec85 100644 --- a/studio/backend/hub/services/models/cache_inventory.py +++ b/studio/backend/hub/services/models/cache_inventory.py @@ -269,10 +269,12 @@ def _cache_inventory_fields( active_hub_cache: Optional[Path] = None, partial: bool = False, requires_variant: bool = False, + payload_snapshots: frozenset[str] = frozenset(), ) -> dict: # *snapshot_path* becomes the load identity whenever the repo id will not # resolve, so callers must pass a snapshot that holds the payload this row - # advertises, not merely the newest one. + # advertises, not merely the newest one. *payload_snapshots* are every + # snapshot that does hold it, used to judge where the repo id would land. load_id = repo_id active_cache = True if repo_path is not None: @@ -288,16 +290,18 @@ def _cache_inventory_fields( except (OSError, RuntimeError, ValueError): active_cache = False load_id = str(snapshot_path or repo_path) - if ( - load_id == repo_id - and snapshot_path is not None - and repo_path is not None - and not hf_cache_scan.default_ref_resolves_on_disk(repo_path) - ): + if load_id == repo_id and snapshot_path is not None and repo_path is not None: + default_snapshot = hf_cache_scan.default_ref_snapshot(repo_path) # No usable refs/main: from_pretrained(repo_id) would fail offline and # would fetch the current upstream HEAD online, ignoring the snapshot - # already on disk. Point the load straight at that snapshot instead. - load_id = str(snapshot_path) + # already on disk. A refs/main that resolves is no better when it lands + # on a revision without the payload this row advertises: a metadata + # probe against a moved commit leaves exactly such a snapshot, and it + # is the newest, so it wins the ref. Point the load at the payload. + if default_snapshot is None or ( + payload_snapshots and str(default_snapshot) not in payload_snapshots + ): + load_id = str(snapshot_path) return { "inventory_id": _local_inventory_id("cache", model_format, repo_id), "load_id": load_id, @@ -378,15 +382,19 @@ def _scan_cached_gguf() -> list[dict]: last_modified = max(last_modified, (existing or {}).get("last_modified", 0.0)) if last_modified > 0: row["last_modified"] = last_modified + # Walks each snapshot's quants, so it runs only for repos that + # made it past the skips above. + gguf_snapshot, gguf_payload_snapshots = _repo_gguf_payload_snapshots(repo_info) row.update( _cache_inventory_fields( repo_id, "gguf", repo_path = repo_path, - snapshot_path = _repo_gguf_payload_snapshot(repo_info) or snapshot_path, + snapshot_path = gguf_snapshot or snapshot_path, active_hub_cache = active_hub_cache, partial = bool(row["partial"]), requires_variant = True, + payload_snapshots = gguf_payload_snapshots, ) ) if _repo_has_mmproj(repo_info): @@ -432,6 +440,7 @@ class _CachedNonGgufPayload(NamedTuple): model_format: ModelFormat last_modified: float payload_snapshot: Optional[Path] + payload_snapshots: frozenset[str] # Keys mirror _classify_non_gguf_model_format's keyword arguments so a revision's @@ -470,19 +479,42 @@ def _newest_snapshot_dir(candidates) -> Optional[Path]: return best[1] -def _repo_gguf_payload_snapshot(repo_info) -> Optional[Path]: - """Newest snapshot dir holding a primary GGUF, or None when none does. +def _resolved_snapshot_ids(candidates) -> frozenset[str]: + """The same strings ``_newest_snapshot_dir`` would return, for membership.""" + resolved: set[str] = set() + for candidate in candidates: + path = Path(candidate) + try: + resolved.add(str(path.resolve())) + except OSError: + resolved.add(str(path)) + return frozenset(resolved) + + +def _repo_gguf_payload_snapshots(repo_info) -> tuple[Optional[Path], frozenset[str]]: + """Snapshot dirs a GGUF load can actually use, plus the newest of them. The row's size sums quants over every revision, while local variant resolution reads only the directory handed out as ``load_id``, so the two - must agree or an advertised quant resolves to nothing. + must agree or an advertised quant resolves to nothing. A snapshot holding + only part of a split quant is not usable either: the picker still offers + that quant and the generated command asks for shards that are absent, so + prefer a complete one exactly as ``_repo_gguf_load_id`` does. Fall back to + any primary GGUF when nothing is complete, which is what shipped before. """ - return _newest_snapshot_dir( + with_gguf = [ snapshot for revision in repo_info.revisions if (snapshot := getattr(revision, "snapshot_path", None)) is not None and any(_is_main_gguf_filename(f.file_name) for f in revision.files) - ) + ] + complete = [ + snapshot + for snapshot in with_gguf + if hf_cache_scan.snapshot_variants_all_complete(str(snapshot)) + ] + usable = complete or with_gguf + return _newest_snapshot_dir(usable), _resolved_snapshot_ids(usable) def _repo_non_gguf_model_payload(repo_info) -> _CachedNonGgufPayload: @@ -551,19 +583,21 @@ def _repo_non_gguf_model_payload(repo_info) -> _CachedNonGgufPayload: else: selected_blobs = all_weight_blobs + # Weights are pooled across revisions, so the newest snapshot need not hold + # any: the load id has to name one that classifies the same way on its own, + # otherwise the load fails on a fully cached model. + payload_snapshots = [ + snapshot + for snapshot, flags in revision_flags + if _classify_non_gguf_model_format(**flags, trusted_hf_cache_repo = True) == model_format + ] return _CachedNonGgufPayload( size_bytes = sum(size for size, _mtime in selected_blobs.values()), has_runnable_weights = model_format != "unknown", model_format = model_format, last_modified = max((mtime for _size, mtime in selected_blobs.values()), default = 0.0), - # Weights are pooled across revisions, so the newest snapshot need not - # hold any: the load id has to name one that classifies the same way on - # its own, otherwise the load fails on a fully cached model. - payload_snapshot = _newest_snapshot_dir( - snapshot - for snapshot, flags in revision_flags - if _classify_non_gguf_model_format(**flags, trusted_hf_cache_repo = True) == model_format - ), + payload_snapshot = _newest_snapshot_dir(payload_snapshots), + payload_snapshots = _resolved_snapshot_ids(payload_snapshots), ) @@ -695,10 +729,14 @@ def _scan_cached_models() -> list[dict]: if local_metadata.pop("_hidden_stt", False): skipped_stt += 1 continue + # Scoped to the snapshot the row will advertise: an incomplete + # newer revision must not flip can_chat off for the complete + # one this row actually hands out as its load id. snapshot_partial = hf_cache_scan.is_snapshot_partial( "model", repo_id, repo_path, + snapshot_dir = payload.payload_snapshot, ) row = { "repo_id": repo_id, @@ -730,6 +768,7 @@ def _scan_cached_models() -> list[dict]: snapshot_path = payload.payload_snapshot or snapshot_path, active_hub_cache = active_hub_cache, partial = bool(row["partial"]), + payload_snapshots = payload.payload_snapshots, ) ) if _prefer_cache_row(row, existing): diff --git a/studio/backend/hub/tests/test_model_services.py b/studio/backend/hub/tests/test_model_services.py index fa5862a13c..005f29e00a 100644 --- a/studio/backend/hub/tests/test_model_services.py +++ b/studio/backend/hub/tests/test_model_services.py @@ -866,7 +866,7 @@ def test_cached_models_scan_hides_non_gguf_embedder(monkeypatch, tmp_path): monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_snapshot_partial", - lambda _kind, _repo_id, _path: False, + lambda _kind, _repo_id, _path, **_kw: False, ) result = {"cached": cache_inventory._scan_cached_models()} @@ -914,7 +914,7 @@ def test_cached_scans_hide_embedders_configured_by_cache_path(monkeypatch, tmp_p monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_snapshot_partial", - lambda _kind, _repo_id, _path: False, + lambda _kind, _repo_id, _path, **_kw: False, ) assert cache_inventory._scan_cached_gguf() == [] @@ -977,7 +977,7 @@ def test_cached_scans_hide_embedders_configured_by_snapshot_path(monkeypatch, tm monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_snapshot_partial", - lambda _kind, _repo_id, _path: False, + lambda _kind, _repo_id, _path, **_kw: False, ) assert cache_inventory._scan_cached_gguf() == [] @@ -1015,7 +1015,7 @@ def test_cached_models_scan_keeps_unrelated_repo_with_custom_generic_embedder( monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_snapshot_partial", - lambda _kind, _repo_id, _path: False, + lambda _kind, _repo_id, _path, **_kw: False, ) result = {"cached": cache_inventory._scan_cached_models()} @@ -1054,7 +1054,7 @@ def test_cached_scans_hide_stale_default_embedder_after_custom_setting(monkeypat monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_snapshot_partial", - lambda _kind, _repo_id, _path: False, + lambda _kind, _repo_id, _path, **_kw: False, ) assert cache_inventory._scan_cached_gguf() == [] diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index 838e987976..4fc32608af 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -381,6 +381,31 @@ def _compute_all_hf_cache_scans() -> list: return scans +def default_ref_snapshot(repo_dir: Path) -> Optional[Path]: + """Snapshot dir that ``refs/main`` names in *repo_dir*, or ``None``. + + This is the directory ``from_pretrained(repo_id)`` ends up in, so callers + compare it against the snapshot the inventory row advertises: naming the + repo id is only safe when the two hold the same payload. + """ + ref_path = repo_dir / "refs" / "main" + try: + # No strip: huggingface_hub matches the raw ref contents against the + # snapshot dir name, so a ref with stray whitespace resolves nowhere. + commit = ref_path.read_text(encoding = "utf-8") + except (OSError, UnicodeDecodeError): + return None + if not commit: + return None + snapshot = repo_dir / "snapshots" / commit + try: + if not snapshot.is_dir(): + return None + return snapshot.resolve() + except (OSError, ValueError): + return None + + def default_ref_resolves_on_disk(repo_dir: Path) -> bool: """Whether ``refs/main`` names a snapshot that exists in *repo_dir*. @@ -389,19 +414,7 @@ def default_ref_resolves_on_disk(repo_dir: Path) -> bool: upstream HEAD instead of the snapshot already on disk. Callers use this to hand out the snapshot path as the load identity instead of the repo id. """ - ref_path = repo_dir / "refs" / "main" - try: - # No strip: huggingface_hub matches the raw ref contents against the - # snapshot dir name, so a ref with stray whitespace resolves nowhere. - commit = ref_path.read_text(encoding = "utf-8") - except (OSError, UnicodeDecodeError): - return False - if not commit: - return False - try: - return (repo_dir / "snapshots" / commit).is_dir() - except (OSError, ValueError): - return False + return default_ref_snapshot(repo_dir) is not None def token_fingerprint(hf_token: Optional[str]) -> str: @@ -617,6 +630,28 @@ def _completed_gguf_variants(snapshot_dir: Optional[Path]) -> set[str]: return complete +def snapshot_variants_all_complete(snapshot: str) -> bool: + """True when every quant the variant lister would advertise from *snapshot* is + fully on disk. + + One complete quant is not enough: the picker enumerates the whole directory, so a + half-downloaded split quant sitting beside a good one still gets offered and the + generated command asks llama-server for shards that are absent. Both sides derive + their labels from ``extract_quant_label`` over paths relative to the snapshot, so + the sets are directly comparable. + """ + from hub.utils.gguf import list_local_gguf_variants + + try: + variants, _ = list_local_gguf_variants(snapshot) + offered = {v.quant for v in variants if getattr(v, "quant", None)} + if not offered: + return False + return offered <= _completed_gguf_variants(Path(snapshot)) + except Exception: + return False + + def _manifest_partial( repo_type: RepoType, repo_id: str, @@ -694,6 +729,7 @@ def is_snapshot_partial( repo_type: RepoType, repo_id: str, repo_cache_dir: Optional[Path] = None, + snapshot_dir: Optional[Path] = None, ) -> bool: """Repo-row partial flag for snapshot-style downloads (full-snapshot models — safetensors/adapter/checkpoint — and all datasets). @@ -704,7 +740,12 @@ def is_snapshot_partial( 3. Manifest walk (stat per expected file under the latest snapshot). A manifest without a resolvable snapshot is partial: the worker got - far enough to record expectations but did not leave a usable snapshot.""" + far enough to record expectations but did not leave a usable snapshot. + + *snapshot_dir* pins the manifest walk to the snapshot the row will hand out + as its load identity. Without it the walk uses the newest snapshot, so a + weightless metadata-only revision beside a complete download flags the row + partial and ``can_chat`` goes false for a model that loads fine.""" from hub.utils import download_manifest return _compose_partial( lambda: download_manifest.has_cancel_marker( @@ -718,7 +759,7 @@ def is_snapshot_partial( repo_type, repo_id, None, - None, + snapshot_dir, repo_cache_dir, ), ) diff --git a/studio/backend/routes/models.py b/studio/backend/routes/models.py index 96c5b96d73..0e996dfbde 100644 --- a/studio/backend/routes/models.py +++ b/studio/backend/routes/models.py @@ -3052,26 +3052,12 @@ def _repo_gguf_last_modified(repo_info) -> float: def snapshot_variants_all_complete(snapshot: str) -> bool: - """True when every quant the variant lister would advertise from *snapshot* is - fully on disk. - - One complete quant is not enough: the picker enumerates the whole directory, so a - half-downloaded split quant sitting beside a good one still gets offered and the - generated command asks llama-server for shards that are absent. Both sides derive - their labels from ``extract_quant_label`` over paths relative to the snapshot, so - the sets are directly comparable. - """ + """Re-exported for callers that already import it from here; the scan-side + cache inventory needs the same predicate, so it lives beside the completed + variant walk it is built on.""" from hub.utils import inventory_scan - from hub.utils.gguf import list_local_gguf_variants - try: - variants, _ = list_local_gguf_variants(snapshot) - offered = {v.quant for v in variants if getattr(v, "quant", None)} - if not offered: - return False - return offered <= inventory_scan._completed_gguf_variants(Path(snapshot)) - except Exception: - return False + return inventory_scan.snapshot_variants_all_complete(snapshot) def _repo_gguf_load_id(repo_info, active_root: Optional[Path]) -> Optional[str]: diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index bb76234f3d..e29c0d645b 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -367,7 +367,13 @@ def _autoload_gguf_rows(cache_root: Path, monkeypatch) -> list[dict]: inventory_scan.invalidate_hf_cache_scans() -def _two_snapshot_repo(cache_root: Path, older_files: dict, newer_files: dict) -> Path: +def _two_snapshot_repo( + cache_root: Path, + older_files: dict, + newer_files: dict, + *, + ref: str = UPSTREAM_HEAD, +) -> Path: """A repo whose payload sits in the older of two snapshots. Realistic because a metadata probe (config.json only) against a commit that @@ -376,7 +382,7 @@ def _two_snapshot_repo(cache_root: Path, older_files: dict, newer_files: dict) - repo_dir = cache_root / "models--Org--Model" (repo_dir / "blobs").mkdir(parents = True, exist_ok = True) (repo_dir / "refs").mkdir(parents = True, exist_ok = True) - (repo_dir / "refs" / "main").write_text(UPSTREAM_HEAD, encoding = "utf-8") + (repo_dir / "refs" / "main").write_text(ref, encoding = "utf-8") for commit, files in ((OLDER, older_files), (NEWER, newer_files)): snapshot = repo_dir / "snapshots" / commit snapshot.mkdir(parents = True, exist_ok = True) @@ -443,3 +449,131 @@ def test_load_id_still_prefers_the_newest_snapshot_that_holds_the_payload(tmp_pa rows = _autoload_rows(tmp_path, monkeypatch) assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER + + +def test_load_id_leaves_the_payload_snapshot_when_main_resolves_elsewhere(tmp_path, monkeypatch): + """A ``refs/main`` that resolves is not enough: the metadata probe that + strands the weights in an older snapshot also repoints ``refs/main`` at the + weightless one, so loading by repo id lands on a revision with no weights + and the app downloads the model again.""" + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}"}, + ref = NEWER, + ) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert [row["repo_id"] for row in rows] == ["Org/Model"] + load_id = rows[0]["load_id"] + # Behavioural: follow the load identity to a directory the way a load would + # (repo id resolves through refs/main) and require the weights to be there. + resolved = ( + repo_dir / "snapshots" / (repo_dir / "refs" / "main").read_text(encoding = "utf-8") + if load_id == "Org/Model" + else Path(load_id) + ) + assert any( + entry.suffix == ".safetensors" for entry in resolved.iterdir() + ), f"load_id {load_id} resolves to {resolved.name}, which holds no weights" + assert Path(load_id) == repo_dir / "snapshots" / OLDER + + +def test_load_id_stays_the_repo_id_when_main_resolves_onto_the_payload(tmp_path, monkeypatch): + """The rule must stay narrow: when ``refs/main`` names a snapshot that does + hold the payload, the pinned revision keeps winning and the row keeps the + repo id, even though a newer snapshot is runnable too.""" + _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}", "model.safetensors": b"\0" * 13}, + ref = OLDER, + ) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert rows[0]["load_id"] == "Org/Model" + + +def test_gguf_load_id_skips_a_snapshot_holding_half_a_split_quant(tmp_path, monkeypatch): + """The newest snapshot can hold shard 1 of an interrupted split download. + The picker still offers that quant, so the generated command asks + llama-server for a shard that is absent while a complete quant sits in an + older snapshot.""" + from hub.utils.gguf import list_local_gguf_variants + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"Model-Q4_K_M.gguf": b"\0" * 32}, + newer_files = {"Model-Q8_0-00001-of-00002.gguf": b"\0" * 16}, + ) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + assert [row["repo_id"] for row in rows] == ["Org/Model"] + load_dir = Path(rows[0]["load_id"]) + variants, _has_vision = list_local_gguf_variants(str(load_dir)) + offered = {v.quant for v in variants if getattr(v, "quant", None)} + complete = inventory_scan._completed_gguf_variants(load_dir) + assert offered and offered <= complete, ( + f"load_id {load_dir.name} offers {sorted(offered)} with only " + f"{sorted(complete)} complete; the usable quant is in {OLDER[:8]}" + ) + assert load_dir == repo_dir / "snapshots" / OLDER + + +def test_partial_is_judged_against_the_snapshot_the_row_advertises(tmp_path, monkeypatch): + """The manifest walk used the newest snapshot while the load id now names an + older complete one, so a weightless probe snapshot flagged the row partial; + that flips ``can_chat`` off and auto-load skips a model that loads fine.""" + from hub.utils import download_manifest + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}"}, + ) + download_manifest.write_manifest( + "model", + "Org/Model", + None, + [ + download_manifest.ExpectedFile(path = "config.json", size = 2), + download_manifest.ExpectedFile(path = "model.safetensors", size = 11), + ], + hub_cache = tmp_path, + ) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / OLDER + assert rows[0].get("partial") is False + assert rows[0]["capabilities"].get("can_chat") is True + + +def test_a_genuinely_incomplete_download_is_still_partial(tmp_path, monkeypatch): + """Negative side of the same rule: scoping the manifest walk to the + advertised snapshot must not stop reporting a truncated download.""" + from hub.utils import download_manifest + + _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}", "model.safetensors": b"\0" * 4}, + ) + download_manifest.write_manifest( + "model", + "Org/Model", + None, + [ + download_manifest.ExpectedFile(path = "config.json", size = 2), + download_manifest.ExpectedFile(path = "model.safetensors", size = 11), + ], + hub_cache = tmp_path, + ) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert rows[0].get("partial") is True + assert rows[0]["capabilities"].get("can_chat") is False From fd8967c16ae43869db94e9ea32e625d101d79d24 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 23:50:20 +0000 Subject: [PATCH 12/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- studio/backend/hub/utils/inventory_scan.py | 1 - studio/backend/routes/models.py | 1 - 2 files changed, 2 deletions(-) diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index 4fc32608af..d963279431 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -641,7 +641,6 @@ def snapshot_variants_all_complete(snapshot: str) -> bool: the sets are directly comparable. """ from hub.utils.gguf import list_local_gguf_variants - try: variants, _ = list_local_gguf_variants(snapshot) offered = {v.quant for v in variants if getattr(v, "quant", None)} diff --git a/studio/backend/routes/models.py b/studio/backend/routes/models.py index 0e996dfbde..f83c891193 100644 --- a/studio/backend/routes/models.py +++ b/studio/backend/routes/models.py @@ -3056,7 +3056,6 @@ def snapshot_variants_all_complete(snapshot: str) -> bool: cache inventory needs the same predicate, so it lives beside the completed variant walk it is built on.""" from hub.utils import inventory_scan - return inventory_scan.snapshot_variants_all_complete(snapshot) From 49e2440d1e784d7f3f4aaf0f8239857693070a85 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 29 Jul 2026 00:27:46 +0000 Subject: [PATCH 13/29] Scope the variant list and the partial walk to the snapshot the row hands out The load id now names a specific snapshot, but three of the signals paired with it were still repo-wide, so the row could advertise something that snapshot does not hold. The GGUF variant lookup walked snapshots newest-first and returned the first one with any quants. When the newest holds a single shard of an interrupted split download, that half-quant was reported as downloaded while the load id pointed at an older, complete snapshot that does not contain it, and the complete quant was never offered at all: auto-load failed on a cache that has a usable quant. Apply the same all-complete preference the load id uses, keeping the newest snapshot with anything as the fallback. The row-level partial walk was pinned to the advertised snapshot for the manifest signal only; the legacy signal still scanned the repo's blobs and the newest snapshot. An interrupted re-download, which is exactly what this branch exists to prevent, therefore left an .incomplete blob that flipped can_chat off for the complete snapshot the row hands out. Check broken symlinks in the advertised snapshot, and charge .incomplete blobs only to the revision a download is actually writing, which is the newest one. Finally, the per-revision payload check trusted transformer-named weights the way the repo-level classification does, so a snapshot holding weights but no config.json could become the load id even though from_pretrained on that directory cannot resolve an architecture. Require the snapshot to classify without that trust; the row then keeps the repo id, which can still fill the config in from the hub. Tests cover all three plus the fallbacks: a repo with nothing complete still lists its variants, an .incomplete blob against the advertised snapshot is still partial, and a self-contained snapshot is still pinned. --- .../hub/services/models/cache_inventory.py | 10 +- studio/backend/hub/utils/gguf.py | 22 ++- studio/backend/hub/utils/inventory_scan.py | 54 ++++++-- .../tests/test_hf_cache_dangling_refs.py | 128 ++++++++++++++++++ 4 files changed, 195 insertions(+), 19 deletions(-) diff --git a/studio/backend/hub/services/models/cache_inventory.py b/studio/backend/hub/services/models/cache_inventory.py index e3252eec85..844ffd9d19 100644 --- a/studio/backend/hub/services/models/cache_inventory.py +++ b/studio/backend/hub/services/models/cache_inventory.py @@ -585,11 +585,17 @@ def _repo_non_gguf_model_payload(repo_info) -> _CachedNonGgufPayload: # Weights are pooled across revisions, so the newest snapshot need not hold # any: the load id has to name one that classifies the same way on its own, - # otherwise the load fails on a fully cached model. + # otherwise the load fails on a fully cached model. Untrusted here on + # purpose: the repo-level classification may rest on transformer-named + # weights alone, but a pinned load id names ONE directory and + # from_pretrained needs config.json inside it. A snapshot that only + # qualifies through that trust is not self-contained, so it must not become + # the load id; the row then keeps the repo id, which can still fill the + # config in from the hub. payload_snapshots = [ snapshot for snapshot, flags in revision_flags - if _classify_non_gguf_model_format(**flags, trusted_hf_cache_repo = True) == model_format + if _classify_non_gguf_model_format(**flags, trusted_hf_cache_repo = False) == model_format ] return _CachedNonGgufPayload( size_bytes = sum(size for size, _mtime in selected_blobs.values()), diff --git a/studio/backend/hub/utils/gguf.py b/studio/backend/hub/utils/gguf.py index eb768db5d6..b5dd4e60d5 100644 --- a/studio/backend/hub/utils/gguf.py +++ b/studio/backend/hub/utils/gguf.py @@ -316,16 +316,32 @@ def list_empty_gguf_variant_dirs(repo_id: str, root: Optional[Path] = None) -> s def list_gguf_variants_from_hf_cache( repo_id: str, root: Optional[Path] = None ) -> Optional[tuple[list[GgufVariantInfo], bool]]: + # Imported here, not at module scope: inventory_scan imports this module. + from hub.utils.inventory_scan import snapshot_variants_all_complete + snapshots = ( iter_hf_cache_snapshots(repo_id, root = root) if root is not None else iter_hf_cache_snapshots(repo_id) ) + # The inventory row hands out the newest snapshot whose quants are all on + # disk as its load id, and a local load reads only that one directory. A + # plain newest-first walk reports a half-downloaded split quant from a newer + # snapshot as downloaded while /load points at an older snapshot that does + # not hold it, so the same preference is applied here, exactly as + # _repo_gguf_payload_snapshots does. The vision flag is OR-ed in because a + # skipped newer snapshot can be a projector fetched on its own; when nothing + # is complete the first snapshot with anything wins, as it did before. + any_vision = False + fallback: Optional[tuple[list[GgufVariantInfo], bool]] = None for snapshot in snapshots: variants, has_vision = list_local_gguf_variants(str(snapshot)) - if variants or has_vision: - return variants, has_vision - return None + any_vision = any_vision or has_vision + if variants and snapshot_variants_all_complete(str(snapshot)): + return variants, any_vision + if fallback is None and (variants or has_vision): + fallback = (variants, has_vision) + return fallback def list_partial_gguf_variants_from_state( diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index d963279431..2d465442d9 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -516,19 +516,21 @@ def _repo_cache_dir_incomplete_hashes(repo_cache_dir: Path) -> set[str]: return hashes -def _repo_cache_dir_has_non_gguf_broken_snapshot_symlinks(repo_cache_dir: Path) -> bool: - latest = latest_snapshot_dir(repo_cache_dir) - if latest is None: +def _repo_cache_dir_has_non_gguf_broken_snapshot_symlinks( + repo_cache_dir: Path, snapshot_dir: Optional[Path] = None +) -> bool: + target = snapshot_dir if snapshot_dir is not None else latest_snapshot_dir(repo_cache_dir) + if target is None: return False try: - entries = list(latest.rglob("*")) + entries = list(target.rglob("*")) except OSError: return False for entry in entries: try: if not entry.is_symlink() or entry.exists(): continue - rel = entry.relative_to(latest).as_posix() + rel = entry.relative_to(target).as_posix() if is_gguf_filename(rel): continue return True @@ -537,6 +539,16 @@ def _repo_cache_dir_has_non_gguf_broken_snapshot_symlinks(repo_cache_dir: Path) return False +def _is_latest_snapshot(repo_cache_dir: Path, snapshot_dir: Path) -> bool: + latest = latest_snapshot_dir(repo_cache_dir) + if latest is None: + return False + try: + return latest.resolve() == snapshot_dir.resolve() + except OSError: + return latest == snapshot_dir + + def _gguf_variant_manifest_blob_hashes( repo_id: str, repo_cache_dir: Optional[Path] = None ) -> frozenset[str]: @@ -564,18 +576,28 @@ def _gguf_variant_manifest_blob_hashes( def _repo_cache_dir_has_snapshot_legacy_partial( - repo_cache_dir: Path, *, ignored_blob_hashes: frozenset[str] + repo_cache_dir: Path, + *, + ignored_blob_hashes: frozenset[str], + snapshot_dir: Optional[Path] = None, ) -> bool: - incomplete_hashes = _repo_cache_dir_incomplete_hashes(repo_cache_dir) - if any(blob_hash not in ignored_blob_hashes for blob_hash in incomplete_hashes): + if _repo_cache_dir_has_non_gguf_broken_snapshot_symlinks(repo_cache_dir, snapshot_dir): return True - return _repo_cache_dir_has_non_gguf_broken_snapshot_symlinks(repo_cache_dir) + # ``.incomplete`` blobs sit in ``blobs/`` with no revision of their own, so + # they can only be charged to the revision a download is currently writing, + # which is the newest one. A row that advertises an older, already complete + # snapshot must not go partial (and lose ``can_chat``) over a separate fetch. + if snapshot_dir is not None and not _is_latest_snapshot(repo_cache_dir, snapshot_dir): + return False + incomplete_hashes = _repo_cache_dir_incomplete_hashes(repo_cache_dir) + return any(blob_hash not in ignored_blob_hashes for blob_hash in incomplete_hashes) def _snapshot_legacy_partial( repo_type: str, repo_id: str, repo_cache_dir: Optional[Path] = None, + snapshot_dir: Optional[Path] = None, ) -> bool: if repo_type != "model": return _legacy_partial(repo_type, repo_id, repo_cache_dir) @@ -584,7 +606,10 @@ def _snapshot_legacy_partial( return _repo_cache_dir_has_snapshot_legacy_partial( repo_cache_dir, ignored_blob_hashes = ignored_hashes, + snapshot_dir = snapshot_dir, ) + # Without a repo dir the snapshot cannot be attributed to one of the roots + # below, so the repo-wide signal is kept rather than applied to the wrong dir. return any( _repo_cache_dir_has_snapshot_legacy_partial( entry, @@ -741,10 +766,11 @@ def is_snapshot_partial( A manifest without a resolvable snapshot is partial: the worker got far enough to record expectations but did not leave a usable snapshot. - *snapshot_dir* pins the manifest walk to the snapshot the row will hand out - as its load identity. Without it the walk uses the newest snapshot, so a - weightless metadata-only revision beside a complete download flags the row - partial and ``can_chat`` goes false for a model that loads fine.""" + *snapshot_dir* pins both the legacy and the manifest walk to the snapshot the + row will hand out as its load identity. Without it they use the newest + snapshot, so a weightless metadata-only revision beside a complete download + flags the row partial and ``can_chat`` goes false for a model that loads + fine.""" from hub.utils import download_manifest return _compose_partial( lambda: download_manifest.has_cancel_marker( @@ -753,7 +779,7 @@ def is_snapshot_partial( None, hub_cache = _hub_cache_for_repo_dir(repo_cache_dir), ), - lambda: _snapshot_legacy_partial(repo_type, repo_id, repo_cache_dir), + lambda: _snapshot_legacy_partial(repo_type, repo_id, repo_cache_dir, snapshot_dir), lambda: _manifest_partial( repo_type, repo_id, diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index e29c0d645b..c9cfb9939e 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -577,3 +577,131 @@ def test_a_genuinely_incomplete_download_is_still_partial(tmp_path, monkeypatch) assert rows[0].get("partial") is True assert rows[0]["capabilities"].get("can_chat") is False + + +# --- everything the row advertises must resolve under its load id ------------ + + +def _local_gguf_variants_for_autoload(row: dict, cache_root: Path) -> list[str]: + """The quants chat auto-load is offered: GET /api/models/gguf-variants with + ``preferLocalCache`` and the row's ``cache_path``, exactly as chat-adapter + calls it before handing ``load_id`` to /load.""" + import asyncio + + from hub.services.models import gguf_variants + + response = asyncio.run( + gguf_variants.get_gguf_variants_response( + row["repo_id"], + prefer_local_cache = True, + local_path = row["cache_path"], + ) + ) + return [variant.quant for variant in response.variants if variant.downloaded] + + +def test_gguf_variants_are_scoped_to_the_snapshot_the_row_advertises(tmp_path, monkeypatch): + """The load id now names the newest snapshot whose quants are all on disk, + but the variant lookup still walked snapshots newest-first, so a half + downloaded split quant in a newer snapshot was offered as downloaded while + /load pointed at an older snapshot that does not hold it. Auto-load then + failed on a cache that has a perfectly usable quant.""" + from hub.utils.gguf import list_local_gguf_variants + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"Model-Q4_K_M.gguf": b"\0" * 32}, + newer_files = {"Model-Q8_0-00001-of-00002.gguf": b"\0" * 16}, + ) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + load_dir = Path(rows[0]["load_id"]) + assert load_dir == repo_dir / "snapshots" / OLDER + offered = _local_gguf_variants_for_autoload(rows[0], tmp_path) + resolvable = {v.quant for v in list_local_gguf_variants(str(load_dir))[0]} + # Behavioural: every quant offered as downloaded has to resolve under the + # load id, and the complete one must not be shadowed by the broken one. + assert set(offered) <= resolvable, ( + f"auto-load is offered {sorted(offered)} but load_id {load_dir.name[:8]} " + f"resolves only {sorted(resolvable)}" + ) + assert offered == ["Q4_K_M"] + + +def test_gguf_variants_still_list_when_no_snapshot_is_complete(tmp_path, monkeypatch): + """The completeness preference must not empty the list: with nothing + complete anywhere, the newest snapshot holding quants is still reported, + which is what shipped before, and it still agrees with the load id.""" + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"Model-Q4_K_M-00001-of-00002.gguf": b"\0" * 32}, + newer_files = {"Model-Q8_0-00001-of-00002.gguf": b"\0" * 16}, + ) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER + assert _local_gguf_variants_for_autoload(rows[0], tmp_path) == ["Q8_0"] + + +def test_partial_ignores_an_incomplete_blob_left_by_a_newer_revision(tmp_path, monkeypatch): + """The manifest walk was pinned to the advertised snapshot but the legacy + signal still scanned the whole repo, so the interrupted re-download this + branch is meant to prevent left an ``.incomplete`` blob that flipped + ``can_chat`` off for the complete snapshot the row hands out.""" + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}"}, + ) + (repo_dir / "blobs" / ("a" * 40 + ".incomplete")).write_bytes(b"\0" * 3) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / OLDER + assert rows[0].get("partial") is False + assert rows[0]["capabilities"].get("can_chat") is True + + +def test_an_incomplete_blob_against_the_advertised_snapshot_is_still_partial( + tmp_path, monkeypatch +): + """Negative side of the same rule: when the row advertises the newest + snapshot, the ``.incomplete`` blob does belong to it and must still count.""" + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}", "model.safetensors": b"\0" * 13}, + ) + (repo_dir / "blobs" / ("a" * 40 + ".incomplete")).write_bytes(b"\0" * 3) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER + assert rows[0].get("partial") is True + assert rows[0]["capabilities"].get("can_chat") is False + + +def test_load_id_is_not_pinned_to_a_snapshot_that_has_no_config(tmp_path, monkeypatch): + """The repo-level format is allowed to rest on transformer-named weights + alone, but a pinned load id names one directory and from_pretrained needs + ``config.json`` inside it. Pinning a weight-only snapshot advertised the row + as loadable while the load could only fail; keep the repo id, which can + still fill the config in from the hub.""" + _two_snapshot_repo( + tmp_path, + older_files = {"model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}"}, + ref = NEWER, + ) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert [row["repo_id"] for row in rows] == ["Org/Model"] + assert rows[0]["model_format"] == "safetensors" + load_id = rows[0]["load_id"] + assert load_id == "Org/Model", ( + f"load_id {Path(load_id).name[:8]} holds no config.json, so a local " + "from_pretrained on it cannot resolve the architecture" + ) From b31c3de89428e51a31c55b6a8c763ed8e7969510 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:37:51 +0000 Subject: [PATCH 14/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- studio/backend/tests/test_hf_cache_dangling_refs.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index c9cfb9939e..bcf3e8886c 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -664,9 +664,7 @@ def test_partial_ignores_an_incomplete_blob_left_by_a_newer_revision(tmp_path, m assert rows[0]["capabilities"].get("can_chat") is True -def test_an_incomplete_blob_against_the_advertised_snapshot_is_still_partial( - tmp_path, monkeypatch -): +def test_an_incomplete_blob_against_the_advertised_snapshot_is_still_partial(tmp_path, monkeypatch): """Negative side of the same rule: when the row advertises the newest snapshot, the ``.incomplete`` blob does belong to it and must still count.""" repo_dir = _two_snapshot_repo( From 8ef03c98fe930eae3848170871aca08129fae6b0 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 29 Jul 2026 01:21:36 +0000 Subject: [PATCH 15/29] Leave an existing structlog entry alone in the availability probe find_spec raises ValueError on a module already in sys.modules whose __spec__ is None, which is what a bare types.ModuleType stub is. Check sys.modules first so anything already present, real or stubbed, is left untouched and only a genuinely absent package gets stubbed. --- tests/studio/load_freeze/test_load_orchestrator.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/studio/load_freeze/test_load_orchestrator.py b/tests/studio/load_freeze/test_load_orchestrator.py index 2957150a2c..5cec40653b 100644 --- a/tests/studio/load_freeze/test_load_orchestrator.py +++ b/tests/studio/load_freeze/test_load_orchestrator.py @@ -58,7 +58,11 @@ sys.modules.setdefault("loggers", _loggers_stub) # blew up with AttributeError, but only when this file was collected first, so the # same test passed alone and failed under `pytest tests/studio`. Only stub when the # package is genuinely missing, and give the stub the attribute those callers use. -if importlib.util.find_spec("structlog") is None: +# Guard on sys.modules FIRST: another test module may have parked its own bare +# stub, and find_spec() raises ValueError on a module whose __spec__ is None. +# Anything already there (real or stub) is left alone; only a genuinely absent +# package gets stubbed. +if "structlog" not in sys.modules and importlib.util.find_spec("structlog") is None: _structlog_stub = types.ModuleType("structlog") _structlog_stub.get_logger = lambda *args, **kwargs: _logging.getLogger( args[0] if args else "structlog" From 488024c914c25814685d8e0ed491893de9ee4df7 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 29 Jul 2026 01:55:01 +0000 Subject: [PATCH 16/29] Pin the load id only to a snapshot that holds the payload, and read its metadata The per-revision payload check is deliberately untrusted, so a snapshot that holds transformer-named weights without a config.json never qualifies and the row is meant to keep the repo id, which can still complete the config from the hub. The dangling-ref arm overruled that: with no qualifying snapshot the caller falls back to the newest one, and refs/main not resolving pinned it regardless. Dangling refs are the case this branch exists for, so the check was inert exactly where it matters, and auto-load was handed a directory holding only a metadata probe's config.json. Pin only when the snapshot is one of the payload snapshots. The row's local metadata was still read from the newest snapshot while load_id names the payload one, so quant_method and the model card described a directory the row does not hand out; the probe snapshot carries neither. Read them from the advertised snapshot, falling back to the newest when no snapshot holds the payload. --- .../hub/services/models/cache_inventory.py | 31 +++++-- .../tests/test_hf_cache_dangling_refs.py | 80 +++++++++++++++++++ 2 files changed, 105 insertions(+), 6 deletions(-) diff --git a/studio/backend/hub/services/models/cache_inventory.py b/studio/backend/hub/services/models/cache_inventory.py index 844ffd9d19..3ac7960357 100644 --- a/studio/backend/hub/services/models/cache_inventory.py +++ b/studio/backend/hub/services/models/cache_inventory.py @@ -269,12 +269,14 @@ def _cache_inventory_fields( active_hub_cache: Optional[Path] = None, partial: bool = False, requires_variant: bool = False, - payload_snapshots: frozenset[str] = frozenset(), + payload_snapshots: Optional[frozenset[str]] = None, ) -> dict: # *snapshot_path* becomes the load identity whenever the repo id will not # resolve, so callers must pass a snapshot that holds the payload this row # advertises, not merely the newest one. *payload_snapshots* are every - # snapshot that does hold it, used to judge where the repo id would land. + # snapshot that does hold it, used to judge where the repo id would land; + # None means the caller does not track payloads and *snapshot_path* is + # taken on trust. load_id = repo_id active_cache = True if repo_path is not None: @@ -290,7 +292,16 @@ def _cache_inventory_fields( except (OSError, RuntimeError, ValueError): active_cache = False load_id = str(snapshot_path or repo_path) - if load_id == repo_id and snapshot_path is not None and repo_path is not None: + # Only pin a snapshot that is known to hold the payload. When none does the + # caller falls back to the newest snapshot, and handing that out names a + # directory the load cannot use; the repo id at least still completes the + # missing files from the hub. + if ( + load_id == repo_id + and snapshot_path is not None + and repo_path is not None + and (payload_snapshots is None or str(snapshot_path) in payload_snapshots) + ): default_snapshot = hf_cache_scan.default_ref_snapshot(repo_path) # No usable refs/main: from_pretrained(repo_id) would fail offline and # would fetch the current upstream HEAD online, ignoring the snapshot @@ -660,8 +671,13 @@ def _read_model_card_frontmatter(path: Path) -> dict: return {} -def _cached_model_local_metadata(repo_path: Path) -> dict: - snapshot = _cached_model_snapshot_path(repo_path) +def _cached_model_local_metadata(repo_path: Path, snapshot: Optional[Path] = None) -> dict: + # Describe the directory the row hands out, not merely the newest one: the + # metadata probe that strands the payload in an older snapshot carries + # neither the quantization config nor the model card. Fall back to the + # newest snapshot when no revision holds the payload on its own. + if snapshot is None: + snapshot = _cached_model_snapshot_path(repo_path) if snapshot is None: return {} @@ -731,7 +747,10 @@ def _scan_cached_models() -> list[dict]: continue key = repo_id.lower() existing = seen_lower.get(key) - local_metadata = _cached_model_local_metadata(repo_path) + local_metadata = _cached_model_local_metadata( + repo_path, + payload.payload_snapshot, + ) if local_metadata.pop("_hidden_stt", False): skipped_stt += 1 continue diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index bcf3e8886c..10191e0c7e 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -703,3 +703,83 @@ def test_load_id_is_not_pinned_to_a_snapshot_that_has_no_config(tmp_path, monkey f"load_id {Path(load_id).name[:8]} holds no config.json, so a local " "from_pretrained on it cannot resolve the architecture" ) + + +def test_no_snapshot_holds_the_payload_so_a_dangling_ref_pins_nothing(tmp_path, monkeypatch): + """Same repo, but with the dangling ``refs/main`` this branch exists for. + + The rule above is enforced by leaving the row on the repo id, and the + dangling-ref arm used to overrule it and pin the fallback newest snapshot + anyway, which is the one already known not to hold the payload. Pinning a + directory the load cannot use is worse than the repo id, which can still + complete the config from the hub.""" + _two_snapshot_repo( + tmp_path, + older_files = {"model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}"}, + ref = UPSTREAM_HEAD, + ) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert [row["repo_id"] for row in rows] == ["Org/Model"] + load_id = rows[0]["load_id"] + if load_id != "Org/Model": + # Behavioural: a pinned directory is only useful if from_pretrained can + # read an architecture and weights out of it. + pinned = Path(load_id) + held = sorted(entry.name for entry in pinned.iterdir()) + assert (pinned / "config.json").is_file() and any( + entry.endswith(".safetensors") for entry in held + ), f"load_id pins {pinned.name[:8]}, which holds only {held}" + assert load_id == "Org/Model" + + + + +# --- the metadata must describe the snapshot the row hands out --------------- + +QUANTIZED_CONFIG = b'{"quantization_config": {"quant_method": "bitsandbytes"}}' +MODEL_CARD = b"---\npipeline_tag: text-generation\nlibrary_name: transformers\n---\n" + + +def test_metadata_describes_the_snapshot_the_row_hands_out(tmp_path, monkeypatch): + """``quant_method`` and the model-card fields were read from the newest + snapshot while the load id names the payload one, so the row described a + directory it does not hand out. The metadata probe that strands the payload + carries neither the quantization config nor the model card, so the quant + chip and the On Device type filter judged the model on absent data.""" + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = { + "config.json": QUANTIZED_CONFIG, + "model.safetensors": b"\0" * 11, + "README.md": MODEL_CARD, + }, + newer_files = {"config.json": b"{}"}, + ref = NEWER, + ) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / OLDER + assert rows[0].get("quant_method") == "bitsandbytes" + assert rows[0].get("pipeline_tag") == "text-generation" + assert rows[0].get("library_name") == "transformers" + + +def test_metadata_still_falls_back_to_the_newest_snapshot(tmp_path, monkeypatch): + """The rule stays narrow: with no self-contained payload snapshot there is + nothing to scope to, so the newest snapshot still supplies the row.""" + _two_snapshot_repo( + tmp_path, + older_files = {"model.safetensors": b"\0" * 11}, + newer_files = {"config.json": QUANTIZED_CONFIG, "README.md": MODEL_CARD}, + ref = NEWER, + ) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert rows[0]["load_id"] == "Org/Model" + assert rows[0].get("quant_method") == "bitsandbytes" + assert rows[0].get("pipeline_tag") == "text-generation" From 4691b193473996bb26cf80180da0555934afad89 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:58:37 +0000 Subject: [PATCH 17/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- studio/backend/tests/test_hf_cache_dangling_refs.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 10191e0c7e..538c8a769d 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -735,8 +735,6 @@ def test_no_snapshot_holds_the_payload_so_a_dangling_ref_pins_nothing(tmp_path, assert load_id == "Org/Model" - - # --- the metadata must describe the snapshot the row hands out --------------- QUANTIZED_CONFIG = b'{"quantization_config": {"quant_method": "bitsandbytes"}}' From d500a771c1296e1c53ebea6a3015184a1fdb7638 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 29 Jul 2026 02:56:20 +0000 Subject: [PATCH 18/29] Charge the repo-wide partial signals to the snapshot the row hands out Three more signals paired with the pinned load id were still repo-wide or read from the wrong directory. GGUF payload selection matched companions by CachedFileInfo.file_name, which huggingface_hub sets to the bare name for a nested file (the recovered mirror copies it deliberately). The MTP/ drafters unsloth ships for Gemma-shaped repos are only recognisable as companions from their directory, so a snapshot holding nothing but a drafter counted as holding the payload and won the load id, while the variant lister, which does relativise, then offered no quant from it at all. Match the snapshot-relative path instead, via the helper the blob map already uses; this fixes normally scanned repos and recovered ones the same way rather than making the mirror diverge from hub. The cancel marker is repo-wide, and is_snapshot_partial checked it before its scoped legacy and manifest walks. A marker is cleared at every download start and again on success, so one that is present records the most recent attempt and belongs to the newest snapshot, exactly as an .incomplete blob does. A row advertising an older complete snapshot inherited it and lost can_chat. Give the marker the same attribution the legacy walk already had. The GGUF row computed partial before choosing its payload snapshot, and that walk took its completed quants from the newest snapshot while scanning the whole repo for .incomplete blobs. An interrupted re-download therefore flipped can_chat off for the older complete quant the row hands out as its load id, and with nothing complete in the newest snapshot the variant enumeration was empty so the repo-wide legacy flag was returned unfiltered. Pick the payload snapshot first and pin the walk to it. Tests cover all three plus the fallbacks: a drafter beside a real quant is still a payload snapshot, a marker against the advertised snapshot is still partial, and a GGUF download interrupted in the snapshot the row falls back to is still partial. The is_gguf_repo_partial stubs in the hub tests are widened to accept the new keyword so a signature mismatch cannot silently drop a repo through the per-repo except in the scan loop. --- .../hub/services/models/cache_inventory.py | 17 ++- .../backend/hub/tests/test_model_services.py | 16 +-- studio/backend/hub/utils/inventory_scan.py | 51 +++++-- .../tests/test_hf_cache_dangling_refs.py | 134 ++++++++++++++++++ 4 files changed, 197 insertions(+), 21 deletions(-) diff --git a/studio/backend/hub/services/models/cache_inventory.py b/studio/backend/hub/services/models/cache_inventory.py index 3ac7960357..996fe09b60 100644 --- a/studio/backend/hub/services/models/cache_inventory.py +++ b/studio/backend/hub/services/models/cache_inventory.py @@ -372,9 +372,15 @@ def _scan_cached_gguf() -> list[dict]: continue if total_size == 0 and not has_variant_state: continue + # Walks each snapshot's quants, so it runs only for repos that + # made it past the skips above. Computed before the partial + # walk, which has to be judged against the snapshot this row + # hands out rather than the newest one. + gguf_snapshot, gguf_payload_snapshots = _repo_gguf_payload_snapshots(repo_info) partial = hf_cache_scan.is_gguf_repo_partial( repo_id, repo_path, + snapshot_dir = gguf_snapshot, ) if total_size == 0 and not partial: continue @@ -393,9 +399,6 @@ def _scan_cached_gguf() -> list[dict]: last_modified = max(last_modified, (existing or {}).get("last_modified", 0.0)) if last_modified > 0: row["last_modified"] = last_modified - # Walks each snapshot's quants, so it runs only for repos that - # made it past the skips above. - gguf_snapshot, gguf_payload_snapshots = _repo_gguf_payload_snapshots(repo_info) row.update( _cache_inventory_fields( repo_id, @@ -513,11 +516,17 @@ def _repo_gguf_payload_snapshots(repo_info) -> tuple[Optional[Path], frozenset[s prefer a complete one exactly as ``_repo_gguf_load_id`` does. Fall back to any primary GGUF when nothing is complete, which is what shipped before. """ + # Matched on the snapshot-relative path, not ``file_name``: huggingface_hub + # sets that to the bare name for a nested file (the recovered mirror copies + # it), and the ``MTP/`` drafters unsloth ships are only recognisable as + # companions from their directory. Matching the bare name lets a snapshot + # holding nothing but a drafter win the load id, where the variant lister + # -- which does relativise -- then offers no quant at all. with_gguf = [ snapshot for revision in repo_info.revisions if (snapshot := getattr(revision, "snapshot_path", None)) is not None - and any(_is_main_gguf_filename(f.file_name) for f in revision.files) + and any(_is_main_gguf_filename(_cached_repo_file_name(f)) for f in revision.files) ] complete = [ snapshot diff --git a/studio/backend/hub/tests/test_model_services.py b/studio/backend/hub/tests/test_model_services.py index 005f29e00a..e0e955be23 100644 --- a/studio/backend/hub/tests/test_model_services.py +++ b/studio/backend/hub/tests/test_model_services.py @@ -702,7 +702,7 @@ def test_cached_gguf_scan_dedupes_and_excludes_mmproj_only(monkeypatch, tmp_path monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_gguf_repo_partial", - lambda _repo_id, _path: False, + lambda _repo_id, _path, **_kw: False, ) result = {"cached": cache_inventory._scan_cached_gguf()} @@ -723,7 +723,7 @@ def test_cached_gguf_scan_preserves_partial_flag(monkeypatch, tmp_path): monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_gguf_repo_partial", - lambda _repo_id, _path: True, + lambda _repo_id, _path, **_kw: True, ) result = {"cached": cache_inventory._scan_cached_gguf()} @@ -766,7 +766,7 @@ def test_cached_gguf_scan_includes_variant_state_without_completed_gguf(monkeypa monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_gguf_repo_partial", - lambda _repo_id, _path: True, + lambda _repo_id, _path, **_kw: True, ) result = {"cached": cache_inventory._scan_cached_gguf()} @@ -799,7 +799,7 @@ def test_cached_gguf_scan_hides_infra_repos_without_user_downloads(monkeypatch, monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_gguf_repo_partial", - lambda _repo_id, _path: False, + lambda _repo_id, _path, **_kw: False, ) result = {"cached": cache_inventory._scan_cached_gguf()} @@ -834,7 +834,7 @@ def test_cached_gguf_scan_keeps_infra_repo_with_user_downloaded_variant(monkeypa monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_gguf_repo_partial", - lambda _repo_id, _path: False, + lambda _repo_id, _path, **_kw: False, ) result = {"cached": cache_inventory._scan_cached_gguf()} @@ -909,7 +909,7 @@ def test_cached_scans_hide_embedders_configured_by_cache_path(monkeypatch, tmp_p monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_gguf_repo_partial", - lambda _repo_id, _path: False, + lambda _repo_id, _path, **_kw: False, ) monkeypatch.setattr( cache_inventory.hf_cache_scan, @@ -972,7 +972,7 @@ def test_cached_scans_hide_embedders_configured_by_snapshot_path(monkeypatch, tm monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_gguf_repo_partial", - lambda _repo_id, _path: False, + lambda _repo_id, _path, **_kw: False, ) monkeypatch.setattr( cache_inventory.hf_cache_scan, @@ -1049,7 +1049,7 @@ def test_cached_scans_hide_stale_default_embedder_after_custom_setting(monkeypat monkeypatch.setattr( cache_inventory.hf_cache_scan, "is_gguf_repo_partial", - lambda _repo_id, _path: False, + lambda _repo_id, _path, **_kw: False, ) monkeypatch.setattr( cache_inventory.hf_cache_scan, diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index 2d465442d9..8d8b06a755 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -549,6 +549,23 @@ def _is_latest_snapshot(repo_cache_dir: Path, snapshot_dir: Path) -> bool: return latest == snapshot_dir +def _repo_signal_applies_to_snapshot( + repo_cache_dir: Optional[Path], snapshot_dir: Optional[Path] +) -> bool: + """Whether a repo-wide partial signal describes *snapshot_dir*. + + A cancel marker is cleared at every download start and again on success, so + one that is present records the most recent attempt; an ``.incomplete`` blob + likewise belongs to the revision a download is writing. Both attach to the + newest snapshot, so a row advertising an older, already complete one must + not inherit them and lose ``can_chat``. With nothing to attribute against, + the signal is kept rather than dropped. + """ + if repo_cache_dir is None or snapshot_dir is None: + return True + return _is_latest_snapshot(repo_cache_dir, snapshot_dir) + + def _gguf_variant_manifest_blob_hashes( repo_id: str, repo_cache_dir: Optional[Path] = None ) -> frozenset[str]: @@ -759,7 +776,7 @@ def is_snapshot_partial( models — safetensors/adapter/checkpoint — and all datasets). Composes three signals, cheapest first: - 1. Cancel marker (single stat). + 1. Cancel marker (single stat), charged to the newest snapshot. 2. Snapshot-attributed legacy .incomplete blob / broken-symlink check. 3. Manifest walk (stat per expected file under the latest snapshot). @@ -773,7 +790,8 @@ def is_snapshot_partial( fine.""" from hub.utils import download_manifest return _compose_partial( - lambda: download_manifest.has_cancel_marker( + lambda: _repo_signal_applies_to_snapshot(repo_cache_dir, snapshot_dir) + and download_manifest.has_cancel_marker( repo_type, repo_id, None, @@ -829,7 +847,12 @@ def is_variant_partial( ) -def is_gguf_repo_partial(repo_id: str, repo_cache_dir: Optional[Path] = None) -> bool: +def is_gguf_repo_partial( + repo_id: str, + repo_cache_dir: Optional[Path] = None, + *, + snapshot_dir: Optional[Path] = None, +) -> bool: """Repo-row partial flag for a GGUF repo. The inventory shows ONE row per GGUF repo (requires_variant=True); per-variant detail lives in GET /api/models/gguf-variants and uses is_variant_partial. @@ -848,15 +871,25 @@ def is_gguf_repo_partial(repo_id: str, repo_cache_dir: Optional[Path] = None) -> Composes signals: 1. Cheap legacy fast-path (.incomplete blobs / broken symlinks). 2. Per-variant manifest + marker enumeration, gated on "all broken". + + *snapshot_dir* pins both to the snapshot the row hands out as its load id. + Without it the newest snapshot supplies the completed quants while the + legacy walk stays repo-wide, so an interrupted re-download flips can_chat + off for the older complete quant the row actually advertises. """ from hub.utils import download_manifest - has_legacy_partial = _legacy_partial("model", repo_id, repo_cache_dir) - snapshot_dir = resolve_snapshot_dir_for_scan( - "model", - repo_id, - repo_cache_dir, - ) + if snapshot_dir is None: + snapshot_dir = resolve_snapshot_dir_for_scan( + "model", + repo_id, + repo_cache_dir, + ) + # Same attribution as is_snapshot_partial: an .incomplete blob or a broken + # symlink belongs to the revision a download is writing, which is the newest. + has_legacy_partial = _repo_signal_applies_to_snapshot( + repo_cache_dir, snapshot_dir + ) and _legacy_partial("model", repo_id, repo_cache_dir) variants: set[str] = set(_completed_gguf_variants(snapshot_dir)) hub_cache = _hub_cache_for_repo_dir(repo_cache_dir) for variant, _path in download_manifest.iter_variant_manifests( diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 538c8a769d..7a1bf93e4c 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -387,6 +387,9 @@ def _two_snapshot_repo( snapshot = repo_dir / "snapshots" / commit snapshot.mkdir(parents = True, exist_ok = True) for name, payload in files.items(): + # Keys may name a subdir ("MTP/...") the way real GGUF repos ship + # their companions; forward slashes work on Windows too. + (snapshot / name).parent.mkdir(parents = True, exist_ok = True) (snapshot / name).write_bytes(payload) _age(repo_dir / "snapshots" / OLDER, 600) return repo_dir @@ -781,3 +784,134 @@ def test_metadata_still_falls_back_to_the_newest_snapshot(tmp_path, monkeypatch) assert rows[0]["load_id"] == "Org/Model" assert rows[0].get("quant_method") == "bitsandbytes" assert rows[0].get("pipeline_tag") == "text-generation" + + +# --- the signals paired with the pinned snapshot ------------------------------ + + +def test_a_companion_only_snapshot_is_not_a_gguf_payload(tmp_path, monkeypatch): + """Payload selection matched GGUF companions by bare file name, but the + ``MTP/`` drafters unsloth ships are only recognisable from the path + relative to the snapshot (``huggingface_hub`` sets ``file_name`` to the + bare name for nested files, and the recovered mirror matches it). A + snapshot holding nothing but a drafter therefore counted as holding the + payload and won the load id, while the variant lister -- which does look at + the relative path -- offers nothing from it.""" + from hub.utils.gguf import list_local_gguf_variants + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"Model-Q4_K_M-00001-of-00002.gguf": b"\0" * 32}, + newer_files = {"MTP/Model-Q8_0-MTP.gguf": b"\0" * 64}, + ) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + assert [row["repo_id"] for row in rows] == ["Org/Model"] + load_dir = Path(rows[0]["load_id"]) + variants, _has_vision = list_local_gguf_variants(str(load_dir)) + assert [v.quant for v in variants], ( + f"load_id {load_dir.name[:8]} offers no quant at all; it holds only a " + "companion drafter" + ) + assert load_dir == repo_dir / "snapshots" / OLDER + + +def test_a_repo_root_drafter_still_leaves_a_real_quant_selectable(tmp_path, monkeypatch): + """The rule must stay narrow: a snapshot that holds a drafter *and* a real + quant is still a payload snapshot.""" + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"Model-Q4_K_M.gguf": b"\0" * 32}, + newer_files = {"mtp-Model-Q8_0.gguf": b"\0" * 64, "Model-Q8_0.gguf": b"\0" * 128}, + ) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER + + +def test_a_cancel_marker_from_a_newer_attempt_does_not_disable_the_pinned_snapshot( + tmp_path, monkeypatch +): + """A cancel marker is repo-wide and records only the *last* attempt (it is + cleared at every download start and on success), so it belongs to the + newest snapshot. The row advertises an older, complete one, so inheriting + the marker turned ``can_chat`` off for a model that loads fine.""" + from hub.utils import download_manifest + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}"}, + ref = NEWER, + ) + download_manifest.write_cancel_marker("model", "Org/Model", None, "http", hub_cache = tmp_path) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / OLDER + assert rows[0].get("partial") is False + assert rows[0]["capabilities"].get("can_chat") is True + + +def test_a_cancel_marker_against_the_advertised_snapshot_is_still_partial(tmp_path, monkeypatch): + """Negative side of the same rule: when the row advertises the newest + snapshot the marker does describe it, and a cancelled download with no + ``.incomplete`` blob left behind has nothing else to give it away.""" + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, + newer_files = {"config.json": b"{}", "model.safetensors": b"\0" * 13}, + ) + from hub.utils import download_manifest + + download_manifest.write_cancel_marker("model", "Org/Model", None, "http", hub_cache = tmp_path) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER + assert rows[0].get("partial") is True + assert rows[0]["capabilities"].get("can_chat") is False + + +def test_gguf_partial_is_judged_against_the_snapshot_the_row_advertises(tmp_path, monkeypatch): + """The GGUF row picked its payload snapshot after computing ``partial``, + and that walk used the repo's blobs plus the newest snapshot. An + interrupted re-download therefore flipped ``can_chat`` off for the older, + complete quant the row hands out as its load id.""" + from hub.utils.gguf import list_local_gguf_variants + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"Model-Q4_K_M.gguf": b"\0" * 32}, + newer_files = {"Model-Q8_0-00001-of-00002.gguf": b"\0" * 16}, + ) + (repo_dir / "blobs" / ("a" * 40 + ".incomplete")).write_bytes(b"\0" * 3) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + load_dir = Path(rows[0]["load_id"]) + assert load_dir == repo_dir / "snapshots" / OLDER + variants, _has_vision = list_local_gguf_variants(str(load_dir)) + assert [v.quant for v in variants] == ["Q4_K_M"] + assert rows[0].get("partial") is False + assert rows[0]["capabilities"].get("can_chat") is True + + +def test_a_gguf_download_interrupted_in_its_own_snapshot_is_still_partial(tmp_path, monkeypatch): + """Negative side of the same rule: with no complete quant anywhere the row + falls back to the newest snapshot, the ``.incomplete`` blob does belong to + it, and the row must still be partial.""" + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"Model-Q4_K_M-00001-of-00002.gguf": b"\0" * 32}, + newer_files = {"Model-Q8_0-00001-of-00002.gguf": b"\0" * 16}, + ) + (repo_dir / "blobs" / ("a" * 40 + ".incomplete")).write_bytes(b"\0" * 3) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER + assert rows[0].get("partial") is True + assert rows[0]["capabilities"].get("can_chat") is False From ac47c10c9ee47bae9fd2c3e5209dab9592b900f7 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 02:57:31 +0000 Subject: [PATCH 19/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- studio/backend/tests/test_hf_cache_dangling_refs.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 7a1bf93e4c..6bb157c7c3 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -811,8 +811,7 @@ def test_a_companion_only_snapshot_is_not_a_gguf_payload(tmp_path, monkeypatch): load_dir = Path(rows[0]["load_id"]) variants, _has_vision = list_local_gguf_variants(str(load_dir)) assert [v.quant for v in variants], ( - f"load_id {load_dir.name[:8]} offers no quant at all; it holds only a " - "companion drafter" + f"load_id {load_dir.name[:8]} offers no quant at all; it holds only a " "companion drafter" ) assert load_dir == repo_dir / "snapshots" / OLDER From b82d1d692ed8c32b9be0e1ad769748e68c06f43c Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 29 Jul 2026 03:36:45 +0000 Subject: [PATCH 20/29] Scope GGUF variant cancel markers to the pinned snapshot and skip resume-only rows in auto-load A cancel marker is keyed (repo, variant) with no revision, so re-downloading the quant a row already holds and cancelling it marked the complete copy in the older, advertised snapshot broken and On Device hid a loadable GGUF. is_variant_partial now takes the same snapshot attribution the legacy walk got. The cache endpoints also return rows that exist only for the resume and delete affordances. Auto-load attempted them, and now that a load rejection suppresses the default download, a half-finished cache left chat with no model at all. --- studio/backend/hub/utils/inventory_scan.py | 31 +++++--- .../tests/test_hf_cache_dangling_refs.py | 79 +++++++++++++++++++ .../src/features/chat/api/chat-adapter.ts | 22 +++++- .../src/features/chat/api/chat-api.ts | 11 +++ .../studio/test_chat_autoload_failure_gate.py | 60 ++++++++++++++ 5 files changed, 193 insertions(+), 10 deletions(-) diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index 8d8b06a755..4ef5802eb2 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -816,6 +816,7 @@ def is_variant_partial( incomplete_blob_hashes: Optional[set[str]] = None, variant_blob_hashes: Optional[frozenset[str]] = None, repo_cache_dir: Optional[Path] = None, + cancel_marker_applies: bool = True, ) -> bool: """Per-variant partial detection. Owns its manifest, owns its marker. Used by the GGUF variants endpoint to flag a specific quant as broken @@ -823,10 +824,18 @@ def is_variant_partial( snapshot_dir is an optional hint to avoid re-walking the cache when a caller is checking many variants of the same repo (see - is_gguf_repo_partial for that usage).""" + is_gguf_repo_partial for that usage). + + ``cancel_marker_applies`` is the same attribution the repo-wide signals get: + the marker is keyed by (repo, variant) with no revision, so a caller that + pinned *snapshot_dir* to an older revision than the cancelled attempt was + writing passes False rather than call the quant it verifies there broken. + Defaults True so the per-variant endpoint keeps reporting a cancelled quant + as cancelled.""" from hub.utils import download_manifest return _compose_partial( - lambda: download_manifest.has_cancel_marker( + lambda: cancel_marker_applies + and download_manifest.has_cancel_marker( "model", repo_id, variant, @@ -872,10 +881,11 @@ def is_gguf_repo_partial( 1. Cheap legacy fast-path (.incomplete blobs / broken symlinks). 2. Per-variant manifest + marker enumeration, gated on "all broken". - *snapshot_dir* pins both to the snapshot the row hands out as its load id. - Without it the newest snapshot supplies the completed quants while the - legacy walk stays repo-wide, so an interrupted re-download flips can_chat - off for the older complete quant the row actually advertises. + *snapshot_dir* pins all three to the snapshot the row hands out as its load + id. Without it the newest snapshot supplies the completed quants while the + legacy walk and the per-variant cancel markers stay repo-wide, so an + interrupted re-download flips can_chat off for the older complete quant the + row actually advertises. """ from hub.utils import download_manifest @@ -887,9 +897,11 @@ def is_gguf_repo_partial( ) # Same attribution as is_snapshot_partial: an .incomplete blob or a broken # symlink belongs to the revision a download is writing, which is the newest. - has_legacy_partial = _repo_signal_applies_to_snapshot( - repo_cache_dir, snapshot_dir - ) and _legacy_partial("model", repo_id, repo_cache_dir) + # Variant cancel markers carry no revision either, so they get it too. + repo_signal_applies = _repo_signal_applies_to_snapshot(repo_cache_dir, snapshot_dir) + has_legacy_partial = repo_signal_applies and _legacy_partial( + "model", repo_id, repo_cache_dir + ) variants: set[str] = set(_completed_gguf_variants(snapshot_dir)) hub_cache = _hub_cache_for_repo_dir(repo_cache_dir) for variant, _path in download_manifest.iter_variant_manifests( @@ -929,6 +941,7 @@ def is_gguf_repo_partial( variant, snapshot_dir, repo_cache_dir = repo_cache_dir, + cancel_marker_applies = repo_signal_applies, ): has_broken = True else: diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 6bb157c7c3..e6e8861cbf 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -914,3 +914,82 @@ def test_a_gguf_download_interrupted_in_its_own_snapshot_is_still_partial(tmp_pa assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER assert rows[0].get("partial") is True assert rows[0]["capabilities"].get("can_chat") is False + + +def test_a_gguf_variant_marker_from_a_newer_attempt_does_not_disable_the_pinned_quant( + tmp_path, monkeypatch +): + """The GGUF twin of the repo-wide marker rule. A cancel marker is keyed by + (repo, variant) with no revision, so re-downloading the quant the row + already holds and cancelling it marked the complete copy in the older, + advertised snapshot broken and On Device hid a loadable GGUF.""" + from hub.utils import download_manifest + from hub.utils.gguf import list_local_gguf_variants + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"Model-Q4_K_M.gguf": b"\0" * 32}, + newer_files = {"config.json": b"{}"}, + ref = NEWER, + ) + download_manifest.write_cancel_marker( + "model", "Org/Model", "Q4_K_M", "http", hub_cache = tmp_path + ) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + load_dir = Path(rows[0]["load_id"]) + assert load_dir == repo_dir / "snapshots" / OLDER + # The quant the marker names does resolve under the load id, so the row has + # something to chat with. + assert [v.quant for v in list_local_gguf_variants(str(load_dir))[0]] == ["Q4_K_M"] + assert rows[0].get("partial") is False + assert rows[0]["capabilities"].get("can_chat") is True + + +def test_a_gguf_variant_marker_against_the_advertised_snapshot_is_still_partial( + tmp_path, monkeypatch +): + """Negative side of the same rule: when the row advertises the newest + snapshot the marker does describe the attempt that wrote it, so the only + quant stays broken and the row keeps its resume affordance.""" + from hub.utils import download_manifest + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"config.json": b"{}"}, + newer_files = {"Model-Q4_K_M.gguf": b"\0" * 32}, + ) + download_manifest.write_cancel_marker( + "model", "Org/Model", "Q4_K_M", "http", hub_cache = tmp_path + ) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER + assert rows[0].get("partial") is True + assert rows[0]["capabilities"].get("can_chat") is False + + +def test_a_marker_for_another_quant_still_leaves_the_pinned_one_chattable( + tmp_path, monkeypatch +): + """The Q8+Q4 mixed-state rule still holds across snapshots: a cancelled + quant that is not the one the load id resolves must not veto the clean one.""" + from hub.utils import download_manifest + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"Model-Q4_K_M.gguf": b"\0" * 32}, + newer_files = {"config.json": b"{}"}, + ref = NEWER, + ) + download_manifest.write_cancel_marker( + "model", "Org/Model", "Q8_0", "http", hub_cache = tmp_path + ) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / OLDER + assert rows[0].get("partial") is False + assert rows[0]["capabilities"].get("can_chat") is True diff --git a/studio/frontend/src/features/chat/api/chat-adapter.ts b/studio/frontend/src/features/chat/api/chat-adapter.ts index 1954dfdd59..646ec1366c 100644 --- a/studio/frontend/src/features/chat/api/chat-adapter.ts +++ b/studio/frontend/src/features/chat/api/chat-adapter.ts @@ -1499,6 +1499,22 @@ function isAutoLoadableGgufVariant(variant: GgufVariantDetail | null): boolean { return !hasBigEndianGgufMarker(filename, variant.quant); } +/** Whether a cache row is a model the user can actually chat with. + * + * The cache endpoints also return rows that exist only for the resume/delete + * affordances (an interrupted or cancelled download), and those carry + * partial: true with can_chat: false. Auto-load used to attempt them anyway + * and fall through on the rejection; now that a rejection suppresses the + * default download, attempting one would leave chat with no model at all. + * Both fields are optional so an older backend that omits them keeps its + * current behaviour of trying the row. */ +function isChattableCachedRepo(repo: { + partial?: boolean; + capabilities?: { can_chat?: boolean } | null; +}): boolean { + return repo.partial !== true && repo.capabilities?.can_chat !== false; +} + async function autoLoadSmallestModel(): Promise<{ loaded: boolean; blockedByTrustRemoteCode: boolean; @@ -1836,10 +1852,14 @@ async function autoLoadSmallestModel(): Promise<{ return true; } try { - const [ggufRepos, modelRepos] = await Promise.all([ + const [allGgufRepos, allModelRepos] = await Promise.all([ listCachedGguf().catch(() => []), listCachedModels().catch(() => []), ]); + // Filtered once, so the last-used lookup below sees the same set as the + // sweeps and neither can spend a load attempt on a resume-only row. + const ggufRepos = allGgufRepos.filter(isChattableCachedRepo); + const modelRepos = allModelRepos.filter(isChattableCachedRepo); if (lastLoaded) { if (lastLoaded.kind === "gguf") { diff --git a/studio/frontend/src/features/chat/api/chat-api.ts b/studio/frontend/src/features/chat/api/chat-api.ts index 4f558545ca..30e161d998 100644 --- a/studio/frontend/src/features/chat/api/chat-api.ts +++ b/studio/frontend/src/features/chat/api/chat-api.ts @@ -267,6 +267,15 @@ export interface CachedGgufRepo { /** True when the repo ships an mmproj adapter (image inputs). Optional for * older-backend compatibility. */ has_vision?: boolean; + partial?: boolean; + capabilities?: CachedRepoCapabilities | null; +} + +/** The subset of the row's capabilities auto-load acts on. The backend sends + * the whole block on both cache endpoints; the rest is only read by the Hub + * view models, which have their own wider type. */ +export interface CachedRepoCapabilities { + can_chat?: boolean; } export async function getGgufDownloadProgress( @@ -386,6 +395,8 @@ export interface CachedModelRepo { /** Owning cache dir; sent so a delete targets this copy, not the active * cache. Optional for older-backend compatibility. */ cache_path?: string | null; + partial?: boolean; + capabilities?: CachedRepoCapabilities | null; } export async function listCachedModels( diff --git a/tests/studio/test_chat_autoload_failure_gate.py b/tests/studio/test_chat_autoload_failure_gate.py index 21ba34c04c..1b9070cca5 100644 --- a/tests/studio/test_chat_autoload_failure_gate.py +++ b/tests/studio/test_chat_autoload_failure_gate.py @@ -34,6 +34,7 @@ def _source_path(relative_path: str) -> Path: ADAPTER = _source_path("studio/frontend/src/features/chat/api/chat-adapter.ts") TEMP = WORKDIR / "temp" / "chat_autoload_failure_gate" DEFAULT_MODEL = "unsloth/Qwen3.5-4B-MTP-GGUF" +GEMMA_REPO = "unsloth/gemma-4-26B-A4B-it-qat-GGUF" # Stubs for everything autoLoadSmallestModel imports. Each scenario supplies the # cache inventory and how /validate and /load answer for a given model_path. @@ -408,3 +409,62 @@ def test_empty_device_does_not_flag_a_reported_failure(): out = _run("scenario({ ggufRepos: [], models: [] })") assert out["result"].get("loadFailureReported") is not True + + +def test_a_resume_only_cached_row_is_skipped_so_the_default_still_downloads(): + """An interrupted download leaves a row the backend already marks + partial/can_chat=false, kept for the resume and delete affordances. The + sweep attempted it anyway, and with the failure gate above that rejection + suppressed the default download, so a half-finished cache left chat with no + model at all.""" + out = _run( + "scenario({ modelRepos: [{ repo_id: 'org/half', load_id: 'org/half'," + " size_bytes: 1, partial: true, capabilities: { can_chat: false } }]," + " load: (p) => p.model_path === 'org/half'" + " ? new Error('config.json not found') : LOADED(p) })" + ) + + assert _loaded_paths(out) == [DEFAULT_MODEL] + assert out["result"]["loaded"] is True + assert _toasts(out, "toast.error") == [] + + +def test_a_can_chat_false_cached_row_is_skipped_on_its_own(): + """The two fields are set independently on the row, so can_chat alone (a + weightless config-only repo) has to be enough to skip it.""" + out = _run( + "scenario({ ggufRepos: [{ ...GEMMA, capabilities: { can_chat: false } }]," + " variants: { [GEMMA.repo_id]: GEMMA_VARIANTS }," + " load: (p) => p.model_path === GEMMA.repo_id ? new Error(OOM) : LOADED(p) })" + ) + + assert _loaded_paths(out) == [DEFAULT_MODEL] + assert out["result"]["loaded"] is True + + +def test_the_last_used_model_is_skipped_when_its_row_went_partial(): + """The last-used shortcut reads the same rows, so an update that was + cancelled over the model the user last chatted with must not spend the + attempt either.""" + out = _run( + "scenario({ ggufRepos: [{ ...GEMMA, partial: true }]," + " variants: { [GEMMA.repo_id]: GEMMA_VARIANTS }," + " lastLoaded: { id: GEMMA.repo_id, kind: 'gguf', ggufVariant: 'UD-Q4_K_XL' }," + " load: (p) => p.model_path === GEMMA.repo_id ? new Error(OOM) : LOADED(p) })" + ) + + assert _loaded_paths(out) == [DEFAULT_MODEL] + assert out["result"]["loaded"] is True + + +def test_a_complete_cached_row_is_still_attempted(): + """Guard on the filter itself: a row with the fields present and healthy + must still be swept, and a backend that omits them entirely (older Studio) + keeps its current behaviour.""" + out = _run( + "scenario({ ggufRepos: [{ ...GEMMA, partial: false, capabilities: { can_chat: true } }]," + " variants: { [GEMMA.repo_id]: GEMMA_VARIANTS } })" + ) + + assert _loaded_paths(out) == [GEMMA_REPO] + assert out["result"]["loaded"] is True From 5669015f6329753da72a33b3254dc377c4b57fb4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 03:57:04 +0000 Subject: [PATCH 21/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- studio/backend/hub/utils/inventory_scan.py | 4 +--- studio/backend/tests/test_hf_cache_dangling_refs.py | 8 ++------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index 4ef5802eb2..c39c1779a1 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -899,9 +899,7 @@ def is_gguf_repo_partial( # symlink belongs to the revision a download is writing, which is the newest. # Variant cancel markers carry no revision either, so they get it too. repo_signal_applies = _repo_signal_applies_to_snapshot(repo_cache_dir, snapshot_dir) - has_legacy_partial = repo_signal_applies and _legacy_partial( - "model", repo_id, repo_cache_dir - ) + has_legacy_partial = repo_signal_applies and _legacy_partial("model", repo_id, repo_cache_dir) variants: set[str] = set(_completed_gguf_variants(snapshot_dir)) hub_cache = _hub_cache_for_repo_dir(repo_cache_dir) for variant, _path in download_manifest.iter_variant_manifests( diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index e6e8861cbf..3576d8838c 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -971,9 +971,7 @@ def test_a_gguf_variant_marker_against_the_advertised_snapshot_is_still_partial( assert rows[0]["capabilities"].get("can_chat") is False -def test_a_marker_for_another_quant_still_leaves_the_pinned_one_chattable( - tmp_path, monkeypatch -): +def test_a_marker_for_another_quant_still_leaves_the_pinned_one_chattable(tmp_path, monkeypatch): """The Q8+Q4 mixed-state rule still holds across snapshots: a cancelled quant that is not the one the load id resolves must not veto the clean one.""" from hub.utils import download_manifest @@ -984,9 +982,7 @@ def test_a_marker_for_another_quant_still_leaves_the_pinned_one_chattable( newer_files = {"config.json": b"{}"}, ref = NEWER, ) - download_manifest.write_cancel_marker( - "model", "Org/Model", "Q8_0", "http", hub_cache = tmp_path - ) + download_manifest.write_cancel_marker("model", "Org/Model", "Q8_0", "http", hub_cache = tmp_path) rows = _autoload_gguf_rows(tmp_path, monkeypatch) From f6ff1995c14807cccf85a516cf15558a14ebceb8 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 29 Jul 2026 04:13:20 +0000 Subject: [PATCH 22/29] Consolidate the dangling-ref tests without changing behaviour for PR #7375 Seven review rounds each added a two-snapshot fixture that differed only in which signal it asserted, so the near-identical cases now share the existing _two_snapshot_repo helper and collapse into parametrized rows. No behaviour changes. Each surviving case was re-checked against the source revert that motivated it: reverting rounds 4, 5, 6 and 7 still reddens on values (load_id pinning a snapshot with only config.json, 'offers no quant at all', None == 'bitsandbytes', partial True is False), and the client filter still reddens at ['org/half']. hub/tests stays at 157, so no repo is being silently dropped by a stub signature. --- .../tests/test_hf_cache_dangling_refs.py | 232 +++++------------- 1 file changed, 62 insertions(+), 170 deletions(-) diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 3576d8838c..7415b3b87f 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -283,8 +283,9 @@ def test_a_recovered_repo_survives_delete_revisions(tmp_path, monkeypatch): # --- load identity for a recovered snapshot ---------------------------------- -def _autoload_rows(cache_root: Path, monkeypatch) -> list[dict]: - """What chat auto-load sees: GET /api/hub/cached-models.""" +def _autoload_rows(cache_root: Path, monkeypatch, *, gguf: bool = False) -> list[dict]: + """What chat auto-load sees: GET /api/hub/cached-models, or with *gguf* set + GET /api/hub/cached-gguf.""" from hub.services.models import cache_inventory from types import SimpleNamespace @@ -295,6 +296,8 @@ def _autoload_rows(cache_root: Path, monkeypatch) -> list[dict]: ) inventory_scan.invalidate_hf_cache_scans() try: + if gguf: + return cache_inventory._scan_cached_gguf() return cache_inventory._scan_cached_models() finally: inventory_scan.invalidate_hf_cache_scans() @@ -352,19 +355,7 @@ def _age(path: Path, seconds: float) -> None: def _autoload_gguf_rows(cache_root: Path, monkeypatch) -> list[dict]: """What chat auto-load sees for GGUF: GET /api/hub/cached-gguf.""" - from hub.services.models import cache_inventory - from types import SimpleNamespace - - monkeypatch.setattr(inventory_scan, "hf_cache_roots", lambda: [cache_root]) - monkeypatch.setattr( - "utils.hf_cache_settings.get_hf_cache_paths", - lambda: SimpleNamespace(hub_cache = cache_root), - ) - inventory_scan.invalidate_hf_cache_scans() - try: - return cache_inventory._scan_cached_gguf() - finally: - inventory_scan.invalidate_hf_cache_scans() + return _autoload_rows(cache_root, monkeypatch, gguf = True) def _two_snapshot_repo( @@ -499,89 +490,6 @@ def test_load_id_stays_the_repo_id_when_main_resolves_onto_the_payload(tmp_path, assert rows[0]["load_id"] == "Org/Model" -def test_gguf_load_id_skips_a_snapshot_holding_half_a_split_quant(tmp_path, monkeypatch): - """The newest snapshot can hold shard 1 of an interrupted split download. - The picker still offers that quant, so the generated command asks - llama-server for a shard that is absent while a complete quant sits in an - older snapshot.""" - from hub.utils.gguf import list_local_gguf_variants - - repo_dir = _two_snapshot_repo( - tmp_path, - older_files = {"Model-Q4_K_M.gguf": b"\0" * 32}, - newer_files = {"Model-Q8_0-00001-of-00002.gguf": b"\0" * 16}, - ) - - rows = _autoload_gguf_rows(tmp_path, monkeypatch) - - assert [row["repo_id"] for row in rows] == ["Org/Model"] - load_dir = Path(rows[0]["load_id"]) - variants, _has_vision = list_local_gguf_variants(str(load_dir)) - offered = {v.quant for v in variants if getattr(v, "quant", None)} - complete = inventory_scan._completed_gguf_variants(load_dir) - assert offered and offered <= complete, ( - f"load_id {load_dir.name} offers {sorted(offered)} with only " - f"{sorted(complete)} complete; the usable quant is in {OLDER[:8]}" - ) - assert load_dir == repo_dir / "snapshots" / OLDER - - -def test_partial_is_judged_against_the_snapshot_the_row_advertises(tmp_path, monkeypatch): - """The manifest walk used the newest snapshot while the load id now names an - older complete one, so a weightless probe snapshot flagged the row partial; - that flips ``can_chat`` off and auto-load skips a model that loads fine.""" - from hub.utils import download_manifest - - repo_dir = _two_snapshot_repo( - tmp_path, - older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, - newer_files = {"config.json": b"{}"}, - ) - download_manifest.write_manifest( - "model", - "Org/Model", - None, - [ - download_manifest.ExpectedFile(path = "config.json", size = 2), - download_manifest.ExpectedFile(path = "model.safetensors", size = 11), - ], - hub_cache = tmp_path, - ) - - rows = _autoload_rows(tmp_path, monkeypatch) - - assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / OLDER - assert rows[0].get("partial") is False - assert rows[0]["capabilities"].get("can_chat") is True - - -def test_a_genuinely_incomplete_download_is_still_partial(tmp_path, monkeypatch): - """Negative side of the same rule: scoping the manifest walk to the - advertised snapshot must not stop reporting a truncated download.""" - from hub.utils import download_manifest - - _two_snapshot_repo( - tmp_path, - older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, - newer_files = {"config.json": b"{}", "model.safetensors": b"\0" * 4}, - ) - download_manifest.write_manifest( - "model", - "Org/Model", - None, - [ - download_manifest.ExpectedFile(path = "config.json", size = 2), - download_manifest.ExpectedFile(path = "model.safetensors", size = 11), - ], - hub_cache = tmp_path, - ) - - rows = _autoload_rows(tmp_path, monkeypatch) - - assert rows[0].get("partial") is True - assert rows[0]["capabilities"].get("can_chat") is False - - # --- everything the row advertises must resolve under its load id ------------ @@ -603,12 +511,19 @@ def _local_gguf_variants_for_autoload(row: dict, cache_root: Path) -> list[str]: return [variant.quant for variant in response.variants if variant.downloaded] -def test_gguf_variants_are_scoped_to_the_snapshot_the_row_advertises(tmp_path, monkeypatch): - """The load id now names the newest snapshot whose quants are all on disk, - but the variant lookup still walked snapshots newest-first, so a half - downloaded split quant in a newer snapshot was offered as downloaded while - /load pointed at an older snapshot that does not hold it. Auto-load then - failed on a cache that has a perfectly usable quant.""" +def test_a_half_split_quant_shadows_neither_the_load_id_nor_the_variants(tmp_path, monkeypatch): + """The newest snapshot can hold shard 1 of an interrupted split download. + The picker still offers that quant, so the generated command asks + llama-server for a shard that is absent while a complete quant sits in an + older snapshot. + + The load id therefore names the newest snapshot whose quants are all on + disk, but the variant lookup still walked snapshots newest-first, so the + half downloaded split quant was offered as downloaded while /load pointed at + an older snapshot that does not hold it. Auto-load then failed on a cache + that has a perfectly usable quant. Both ends are asserted here because they + are only correct together: the load id and the quants offered under it have + to agree on one directory.""" from hub.utils.gguf import list_local_gguf_variants repo_dir = _two_snapshot_repo( @@ -619,7 +534,14 @@ def test_gguf_variants_are_scoped_to_the_snapshot_the_row_advertises(tmp_path, m rows = _autoload_gguf_rows(tmp_path, monkeypatch) + assert [row["repo_id"] for row in rows] == ["Org/Model"] load_dir = Path(rows[0]["load_id"]) + held = {v.quant for v in list_local_gguf_variants(str(load_dir))[0] if v.quant} + complete = inventory_scan._completed_gguf_variants(load_dir) + assert held and held <= complete, ( + f"load_id {load_dir.name} offers {sorted(held)} with only " + f"{sorted(complete)} complete; the usable quant is in {OLDER[:8]}" + ) assert load_dir == repo_dir / "snapshots" / OLDER offered = _local_gguf_variants_for_autoload(rows[0], tmp_path) resolvable = {v.quant for v in list_local_gguf_variants(str(load_dir))[0]} @@ -648,42 +570,6 @@ def test_gguf_variants_still_list_when_no_snapshot_is_complete(tmp_path, monkeyp assert _local_gguf_variants_for_autoload(rows[0], tmp_path) == ["Q8_0"] -def test_partial_ignores_an_incomplete_blob_left_by_a_newer_revision(tmp_path, monkeypatch): - """The manifest walk was pinned to the advertised snapshot but the legacy - signal still scanned the whole repo, so the interrupted re-download this - branch is meant to prevent left an ``.incomplete`` blob that flipped - ``can_chat`` off for the complete snapshot the row hands out.""" - repo_dir = _two_snapshot_repo( - tmp_path, - older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, - newer_files = {"config.json": b"{}"}, - ) - (repo_dir / "blobs" / ("a" * 40 + ".incomplete")).write_bytes(b"\0" * 3) - - rows = _autoload_rows(tmp_path, monkeypatch) - - assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / OLDER - assert rows[0].get("partial") is False - assert rows[0]["capabilities"].get("can_chat") is True - - -def test_an_incomplete_blob_against_the_advertised_snapshot_is_still_partial(tmp_path, monkeypatch): - """Negative side of the same rule: when the row advertises the newest - snapshot, the ``.incomplete`` blob does belong to it and must still count.""" - repo_dir = _two_snapshot_repo( - tmp_path, - older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, - newer_files = {"config.json": b"{}", "model.safetensors": b"\0" * 13}, - ) - (repo_dir / "blobs" / ("a" * 40 + ".incomplete")).write_bytes(b"\0" * 3) - - rows = _autoload_rows(tmp_path, monkeypatch) - - assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER - assert rows[0].get("partial") is True - assert rows[0]["capabilities"].get("can_chat") is False - - def test_load_id_is_not_pinned_to_a_snapshot_that_has_no_config(tmp_path, monkeypatch): """The repo-level format is allowed to rest on transformer-named weights alone, but a pinned load id names one directory and from_pretrained needs @@ -744,48 +630,54 @@ QUANTIZED_CONFIG = b'{"quantization_config": {"quant_method": "bitsandbytes"}}' MODEL_CARD = b"---\npipeline_tag: text-generation\nlibrary_name: transformers\n---\n" -def test_metadata_describes_the_snapshot_the_row_hands_out(tmp_path, monkeypatch): - """``quant_method`` and the model-card fields were read from the newest - snapshot while the load id names the payload one, so the row described a - directory it does not hand out. The metadata probe that strands the payload - carries neither the quantization config nor the model card, so the quant - chip and the On Device type filter judged the model on absent data.""" +@pytest.mark.parametrize( + "older_files, newer_files, pinned", + [ + # ``quant_method`` and the model-card fields were read from the newest + # snapshot while the load id names the payload one, so the row described + # a directory it does not hand out. The metadata probe that strands the + # payload carries neither the quantization config nor the model card, so + # the quant chip and the On Device type filter judged the model on + # absent data. + pytest.param( + { + "config.json": QUANTIZED_CONFIG, + "model.safetensors": b"\0" * 11, + "README.md": MODEL_CARD, + }, + {"config.json": b"{}"}, + True, + id = "payload-snapshot-supplies-the-row", + ), + # The rule stays narrow: with no self-contained payload snapshot there + # is nothing to scope to, so the newest snapshot still supplies the row. + pytest.param( + {"model.safetensors": b"\0" * 11}, + {"config.json": QUANTIZED_CONFIG, "README.md": MODEL_CARD}, + False, + id = "newest-snapshot-fallback", + ), + ], +) +def test_metadata_describes_the_snapshot_the_row_hands_out( + older_files, newer_files, pinned, tmp_path, monkeypatch +): repo_dir = _two_snapshot_repo( tmp_path, - older_files = { - "config.json": QUANTIZED_CONFIG, - "model.safetensors": b"\0" * 11, - "README.md": MODEL_CARD, - }, - newer_files = {"config.json": b"{}"}, + older_files = older_files, + newer_files = newer_files, ref = NEWER, ) rows = _autoload_rows(tmp_path, monkeypatch) - assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / OLDER + expected_load_id = str(repo_dir / "snapshots" / OLDER) if pinned else "Org/Model" + assert rows[0]["load_id"] == expected_load_id assert rows[0].get("quant_method") == "bitsandbytes" assert rows[0].get("pipeline_tag") == "text-generation" assert rows[0].get("library_name") == "transformers" -def test_metadata_still_falls_back_to_the_newest_snapshot(tmp_path, monkeypatch): - """The rule stays narrow: with no self-contained payload snapshot there is - nothing to scope to, so the newest snapshot still supplies the row.""" - _two_snapshot_repo( - tmp_path, - older_files = {"model.safetensors": b"\0" * 11}, - newer_files = {"config.json": QUANTIZED_CONFIG, "README.md": MODEL_CARD}, - ref = NEWER, - ) - - rows = _autoload_rows(tmp_path, monkeypatch) - - assert rows[0]["load_id"] == "Org/Model" - assert rows[0].get("quant_method") == "bitsandbytes" - assert rows[0].get("pipeline_tag") == "text-generation" - - # --- the signals paired with the pinned snapshot ------------------------------ From f7a80782bd68dfd3732da8fae55054d63c5dba38 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 04:14:07 +0000 Subject: [PATCH 23/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- studio/backend/tests/test_hf_cache_dangling_refs.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 7415b3b87f..3d6fb228bf 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -283,7 +283,12 @@ def test_a_recovered_repo_survives_delete_revisions(tmp_path, monkeypatch): # --- load identity for a recovered snapshot ---------------------------------- -def _autoload_rows(cache_root: Path, monkeypatch, *, gguf: bool = False) -> list[dict]: +def _autoload_rows( + cache_root: Path, + monkeypatch, + *, + gguf: bool = False, +) -> list[dict]: """What chat auto-load sees: GET /api/hub/cached-models, or with *gguf* set GET /api/hub/cached-gguf.""" from hub.services.models import cache_inventory From 3331202667d17c4179e2183044ba09736032a39c Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 29 Jul 2026 05:07:55 +0000 Subject: [PATCH 24/29] Charge the repo-wide manifest to the pinned snapshot and stop offering half quants Two more signals paired with the pinned load id. is_snapshot_partial already attributes the cancel marker and the .incomplete blobs to the newest snapshot, because neither records a revision, but it still verified the repo-wide manifest against whatever snapshot the row hands out. A metadata probe that strands the weights in an older revision, followed by an interrupted download of the newer one, leaves a manifest describing the newer file list; verifying it against the older payload fails on any rename or size change, so the row went partial and can_chat off for a model that loads fine. Give the manifest the same attribution its two siblings already have. The GGUF cache lister falls back to the first snapshot holding anything when no snapshot has every advertised quant complete, and reported all of them as downloaded. Auto-load tries only the smallest, and now that a rejected load suppresses the default download, a half-downloaded split quant beside a whole one left chat with no model at all. Keep the whole quants when the fallback snapshot has any, and leave the list untouched when it has none. Tests parametrise both sides of each rule: the marker and the manifest now share one case matrix over pinned-older and advertised-newest, and the fallback lister covers nothing-complete-anywhere alongside a whole quant beside a half one. --- studio/backend/hub/utils/gguf.py | 16 ++- studio/backend/hub/utils/inventory_scan.py | 25 +++- .../tests/test_hf_cache_dangling_refs.py | 131 ++++++++++++------ 3 files changed, 126 insertions(+), 46 deletions(-) diff --git a/studio/backend/hub/utils/gguf.py b/studio/backend/hub/utils/gguf.py index b5dd4e60d5..3317040904 100644 --- a/studio/backend/hub/utils/gguf.py +++ b/studio/backend/hub/utils/gguf.py @@ -317,7 +317,10 @@ def list_gguf_variants_from_hf_cache( repo_id: str, root: Optional[Path] = None ) -> Optional[tuple[list[GgufVariantInfo], bool]]: # Imported here, not at module scope: inventory_scan imports this module. - from hub.utils.inventory_scan import snapshot_variants_all_complete + from hub.utils.inventory_scan import ( + complete_snapshot_variants, + snapshot_variants_all_complete, + ) snapshots = ( iter_hf_cache_snapshots(repo_id, root = root) @@ -332,6 +335,11 @@ def list_gguf_variants_from_hf_cache( # _repo_gguf_payload_snapshots does. The vision flag is OR-ed in because a # skipped newer snapshot can be a projector fetched on its own; when nothing # is complete the first snapshot with anything wins, as it did before. + # + # That fallback still reports every quant it finds as downloaded. Auto-load + # takes only the smallest one and a rejected load now suppresses the default + # download, so a half-downloaded split quant beside a whole one left chat + # with no model at all; keep the whole ones when the snapshot has any. any_vision = False fallback: Optional[tuple[list[GgufVariantInfo], bool]] = None for snapshot in snapshots: @@ -340,7 +348,11 @@ def list_gguf_variants_from_hf_cache( if variants and snapshot_variants_all_complete(str(snapshot)): return variants, any_vision if fallback is None and (variants or has_vision): - fallback = (variants, has_vision) + complete = complete_snapshot_variants(str(snapshot)) + # A quant with no label cannot be judged, so it is kept rather than + # dropped; with nothing complete the list stays as it was. + usable = [v for v in variants if not v.quant or v.quant in complete] + fallback = (usable or variants, has_vision) return fallback diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index c39c1779a1..bdd6d410d7 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -693,6 +693,16 @@ def snapshot_variants_all_complete(snapshot: str) -> bool: return False +def complete_snapshot_variants(snapshot: str) -> set[str]: + """Quant labels in *snapshot* whose files are all on disk. Same labels as + ``snapshot_variants_all_complete`` compares, for callers that need the subset + rather than the all-or-nothing answer.""" + try: + return _completed_gguf_variants(Path(snapshot)) + except (OSError, RuntimeError, ValueError): + return set() + + def _manifest_partial( repo_type: RepoType, repo_id: str, @@ -787,10 +797,18 @@ def is_snapshot_partial( row will hand out as its load identity. Without it they use the newest snapshot, so a weightless metadata-only revision beside a complete download flags the row partial and ``can_chat`` goes false for a model that loads - fine.""" + fine. + + The repo-wide manifest carries no revision either, so it gets the same + attribution as the marker and the ``.incomplete`` blobs: it describes the + revision the last attempt was writing, which is the newest. Verifying it + against an older pinned snapshot compares one revision's file list with + another's payload, and any rename or size change between the two flagged a + complete, loadable row partial.""" from hub.utils import download_manifest + repo_signal_applies = _repo_signal_applies_to_snapshot(repo_cache_dir, snapshot_dir) return _compose_partial( - lambda: _repo_signal_applies_to_snapshot(repo_cache_dir, snapshot_dir) + lambda: repo_signal_applies and download_manifest.has_cancel_marker( repo_type, repo_id, @@ -798,7 +816,8 @@ def is_snapshot_partial( hub_cache = _hub_cache_for_repo_dir(repo_cache_dir), ), lambda: _snapshot_legacy_partial(repo_type, repo_id, repo_cache_dir, snapshot_dir), - lambda: _manifest_partial( + lambda: repo_signal_applies + and _manifest_partial( repo_type, repo_id, None, diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 3d6fb228bf..7d8d7dd3f3 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -559,20 +559,47 @@ def test_a_half_split_quant_shadows_neither_the_load_id_nor_the_variants(tmp_pat assert offered == ["Q4_K_M"] -def test_gguf_variants_still_list_when_no_snapshot_is_complete(tmp_path, monkeypatch): - """The completeness preference must not empty the list: with nothing - complete anywhere, the newest snapshot holding quants is still reported, - which is what shipped before, and it still agrees with the load id.""" +@pytest.mark.parametrize( + "newer_files, offered", + [ + # With nothing complete anywhere the newest snapshot holding quants is + # still reported, which is what shipped before, and it still agrees with + # the load id. + pytest.param( + {"Model-Q8_0-00001-of-00002.gguf": b"\0" * 16}, + ["Q8_0"], + id = "nothing-complete-anywhere", + ), + # When that snapshot holds a whole quant beside the half-downloaded one, + # offering both as downloaded shadowed the usable one: auto-load takes + # only the smallest and a rejected load now suppresses the default + # download, so chat was left with no model at all. + pytest.param( + { + "Model-Q8_0.gguf": b"\0" * 64, + "Model-Q4_K_M-00001-of-00002.gguf": b"\0" * 16, + }, + ["Q8_0"], + id = "one-whole-quant-beside-a-half-one", + ), + ], +) +def test_gguf_variants_still_list_when_no_snapshot_is_complete( + newer_files, offered, tmp_path, monkeypatch +): + """The completeness preference must not empty the list, and must not offer a + quant whose shards are missing while a whole one sits beside it.""" repo_dir = _two_snapshot_repo( tmp_path, older_files = {"Model-Q4_K_M-00001-of-00002.gguf": b"\0" * 32}, - newer_files = {"Model-Q8_0-00001-of-00002.gguf": b"\0" * 16}, + newer_files = newer_files, ) rows = _autoload_gguf_rows(tmp_path, monkeypatch) - assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER - assert _local_gguf_variants_for_autoload(rows[0], tmp_path) == ["Q8_0"] + load_dir = Path(rows[0]["load_id"]) + assert load_dir == repo_dir / "snapshots" / NEWER + assert _local_gguf_variants_for_autoload(rows[0], tmp_path) == offered def test_load_id_is_not_pinned_to_a_snapshot_that_has_no_config(tmp_path, monkeypatch): @@ -727,48 +754,70 @@ def test_a_repo_root_drafter_still_leaves_a_real_quant_selectable(tmp_path, monk assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER -def test_a_cancel_marker_from_a_newer_attempt_does_not_disable_the_pinned_snapshot( - tmp_path, monkeypatch +def _write_repo_wide_signal(kind: str, hub_cache: Path) -> None: + """Either repo-wide partial signal, neither of which records a revision. + + The manifest names a file the pinned older snapshot holds at a different + size, which is what a revision that renamed or resized its weights leaves + behind for the previous one. + """ + from hub.utils import download_manifest + + if kind == "marker": + download_manifest.write_cancel_marker( + "model", "Org/Model", None, "http", hub_cache = hub_cache + ) + return + download_manifest.write_manifest( + "model", + "Org/Model", + None, + [download_manifest.ExpectedFile("model.safetensors", 99)], + "http", + hub_cache = hub_cache, + ) + + +@pytest.mark.parametrize("signal", ["marker", "manifest"]) +@pytest.mark.parametrize( + "newer_files, ref, advertised, partial", + [ + # The signal belongs to the newest snapshot while the row advertises an + # older, complete one, so inheriting it turned ``can_chat`` off for a + # model that loads fine. + pytest.param({"config.json": b"{}"}, NEWER, OLDER, False, id = "pinned-older-snapshot"), + # Negative side: when the row advertises the newest snapshot the signal + # does describe it, and a cancelled download with no ``.incomplete`` + # blob left behind has nothing else to give it away. + pytest.param( + {"config.json": b"{}", "model.safetensors": b"\0" * 13}, + UPSTREAM_HEAD, + NEWER, + True, + id = "advertised-snapshot", + ), + ], +) +def test_repo_wide_partial_signals_are_charged_to_the_newest_snapshot( + signal, newer_files, ref, advertised, partial, tmp_path, monkeypatch ): - """A cancel marker is repo-wide and records only the *last* attempt (it is - cleared at every download start and on success), so it belongs to the - newest snapshot. The row advertises an older, complete one, so inheriting - the marker turned ``can_chat`` off for a model that loads fine.""" - from hub.utils import download_manifest - + """A cancel marker and a repo-wide manifest both record only the *last* + attempt (the marker is cleared at every download start and on success; the + manifest is overwritten), so both belong to the newest snapshot and neither + may be verified against an older revision's payload.""" repo_dir = _two_snapshot_repo( tmp_path, older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, - newer_files = {"config.json": b"{}"}, - ref = NEWER, + newer_files = newer_files, + ref = ref, ) - download_manifest.write_cancel_marker("model", "Org/Model", None, "http", hub_cache = tmp_path) + _write_repo_wide_signal(signal, tmp_path) rows = _autoload_rows(tmp_path, monkeypatch) - assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / OLDER - assert rows[0].get("partial") is False - assert rows[0]["capabilities"].get("can_chat") is True - - -def test_a_cancel_marker_against_the_advertised_snapshot_is_still_partial(tmp_path, monkeypatch): - """Negative side of the same rule: when the row advertises the newest - snapshot the marker does describe it, and a cancelled download with no - ``.incomplete`` blob left behind has nothing else to give it away.""" - repo_dir = _two_snapshot_repo( - tmp_path, - older_files = {"config.json": b"{}", "model.safetensors": b"\0" * 11}, - newer_files = {"config.json": b"{}", "model.safetensors": b"\0" * 13}, - ) - from hub.utils import download_manifest - - download_manifest.write_cancel_marker("model", "Org/Model", None, "http", hub_cache = tmp_path) - - rows = _autoload_rows(tmp_path, monkeypatch) - - assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / NEWER - assert rows[0].get("partial") is True - assert rows[0]["capabilities"].get("can_chat") is False + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / advertised + assert rows[0].get("partial") is partial + assert rows[0]["capabilities"].get("can_chat") is not partial def test_gguf_partial_is_judged_against_the_snapshot_the_row_advertises(tmp_path, monkeypatch): From 5d7f07d11caf39c371661eff6eb913c660c9bec5 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 05:32:24 +0000 Subject: [PATCH 25/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- studio/backend/hub/utils/inventory_scan.py | 1 + 1 file changed, 1 insertion(+) diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index bdd6d410d7..d029bbd9f4 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -806,6 +806,7 @@ def is_snapshot_partial( another's payload, and any rename or size change between the two flagged a complete, loadable row partial.""" from hub.utils import download_manifest + repo_signal_applies = _repo_signal_applies_to_snapshot(repo_cache_dir, snapshot_dir) return _compose_partial( lambda: repo_signal_applies From f0e8162b3b606d1e00208a4f85c4500ef5dc0a51 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 29 Jul 2026 06:21:34 +0000 Subject: [PATCH 26/29] Attribute a revision-less signal to the ref when it names no snapshot Both signals paired with the pinned snapshot again, one repo-wide and one per-variant. snapshot_download rewrites refs/ with the resolved commit before it fetches a single file, and the snapshot directory only appears once the first file lands; the downloader writes the repo manifest earlier still and never passes a revision, so every attempt it starts goes through that ref. An update interrupted in that window therefore leaves refs/main naming a commit with no directory, the previous complete snapshot as the only payload on disk, and a manifest plus marker describing a revision that was never materialised. The newest-snapshot attribution then charged all of it to the cached payload, so the row this branch recovers arrived partial with can_chat off and chat auto-load skipped a model that loads fine. A present but unresolvable refs/main now pins those signals to a revision that is not on disk, so no snapshot inherits them. An absent ref is left alone: a commit-pinned fetch never writes one, so it carries no evidence either way. is_gguf_repo_partial passed that attribution to the per-variant cancel marker but not to the per-variant manifest, which kept verifying a newer attempt's file list against the older snapshot the row pins. A same-variant re-download that renames or resizes its shards made the one complete quant on disk look broken. The manifest now takes the same gate as the marker, per variant: a quant whose files are all in the pinned snapshot loads from it whatever a later attempt recorded. Tests parametrise both sides of each rule. The existing negative controls move to a commit-pinned repo with no refs/main, which is the state that actually leaves a signal owned by the newest snapshot. --- studio/backend/hub/utils/inventory_scan.py | 64 ++++++++-- .../tests/test_hf_cache_dangling_refs.py | 109 +++++++++++++++++- 2 files changed, 156 insertions(+), 17 deletions(-) diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index d029bbd9f4..df58c116ce 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -549,6 +549,28 @@ def _is_latest_snapshot(repo_cache_dir: Path, snapshot_dir: Path) -> bool: return latest == snapshot_dir +def _default_ref_names_an_absent_snapshot(repo_cache_dir: Path) -> bool: + """Whether ``refs/main`` is present and names a commit with no snapshot dir. + + ``snapshot_download`` rewrites ``refs/`` with the resolved commit + *before* it fetches a single file, and the snapshot directory is only + created once the first file lands, so this state is the window between the + two. A missing ``refs/main`` is not the same thing: a repo fetched by + commit hash never gets one, so it carries no evidence either way. + """ + ref_path = repo_cache_dir / "refs" / "main" + try: + commit = ref_path.read_text(encoding = "utf-8") + except (OSError, UnicodeDecodeError): + return False + if not commit: + return False + try: + return not (repo_cache_dir / "snapshots" / commit).is_dir() + except (OSError, ValueError): + return False + + def _repo_signal_applies_to_snapshot( repo_cache_dir: Optional[Path], snapshot_dir: Optional[Path] ) -> bool: @@ -560,9 +582,19 @@ def _repo_signal_applies_to_snapshot( newest snapshot, so a row advertising an older, already complete one must not inherit them and lose ``can_chat``. With nothing to attribute against, the signal is kept rather than dropped. + + A ``refs/main`` naming a commit with no directory pins that attempt to a + revision that is not on disk at all, so no snapshot here may inherit it. + The downloader never passes a revision, so every attempt it starts rewrites + that ref first; leaving the signal on the newest snapshot instead charged an + interrupted update to the previous, complete payload and hid a model that + still loads. This is the very state the dangling-ref recovery restores rows + from, so the recovered row would arrive unusable. """ if repo_cache_dir is None or snapshot_dir is None: return True + if _default_ref_names_an_absent_snapshot(repo_cache_dir): + return False return _is_latest_snapshot(repo_cache_dir, snapshot_dir) @@ -604,7 +636,9 @@ def _repo_cache_dir_has_snapshot_legacy_partial( # they can only be charged to the revision a download is currently writing, # which is the newest one. A row that advertises an older, already complete # snapshot must not go partial (and lose ``can_chat``) over a separate fetch. - if snapshot_dir is not None and not _is_latest_snapshot(repo_cache_dir, snapshot_dir): + if snapshot_dir is not None and not _repo_signal_applies_to_snapshot( + repo_cache_dir, snapshot_dir + ): return False incomplete_hashes = _repo_cache_dir_incomplete_hashes(repo_cache_dir) return any(blob_hash not in ignored_blob_hashes for blob_hash in incomplete_hashes) @@ -836,7 +870,7 @@ def is_variant_partial( incomplete_blob_hashes: Optional[set[str]] = None, variant_blob_hashes: Optional[frozenset[str]] = None, repo_cache_dir: Optional[Path] = None, - cancel_marker_applies: bool = True, + repo_signal_applies: bool = True, ) -> bool: """Per-variant partial detection. Owns its manifest, owns its marker. Used by the GGUF variants endpoint to flag a specific quant as broken @@ -846,15 +880,16 @@ def is_variant_partial( caller is checking many variants of the same repo (see is_gguf_repo_partial for that usage). - ``cancel_marker_applies`` is the same attribution the repo-wide signals get: - the marker is keyed by (repo, variant) with no revision, so a caller that - pinned *snapshot_dir* to an older revision than the cancelled attempt was - writing passes False rather than call the quant it verifies there broken. - Defaults True so the per-variant endpoint keeps reporting a cancelled quant - as cancelled.""" + ``repo_signal_applies`` is the same attribution the repo-wide signals get. + The marker and the manifest are both keyed by (repo, variant) with no + revision and are both overwritten by the next attempt, so a caller that + pinned *snapshot_dir* to an older revision than that attempt was writing + passes False rather than judge the quant it verifies there by another + revision's file list. Defaults True so the per-variant endpoint keeps + reporting a cancelled or unfinished quant as broken.""" from hub.utils import download_manifest return _compose_partial( - lambda: cancel_marker_applies + lambda: repo_signal_applies and download_manifest.has_cancel_marker( "model", repo_id, @@ -866,7 +901,8 @@ def is_variant_partial( and variant_blob_hashes and incomplete_blob_hashes.intersection(variant_blob_hashes) ), - lambda: _manifest_partial( + lambda: repo_signal_applies + and _manifest_partial( "model", repo_id, variant, @@ -920,7 +956,8 @@ def is_gguf_repo_partial( # Variant cancel markers carry no revision either, so they get it too. repo_signal_applies = _repo_signal_applies_to_snapshot(repo_cache_dir, snapshot_dir) has_legacy_partial = repo_signal_applies and _legacy_partial("model", repo_id, repo_cache_dir) - variants: set[str] = set(_completed_gguf_variants(snapshot_dir)) + complete_here = _completed_gguf_variants(snapshot_dir) + variants: set[str] = set(complete_here) hub_cache = _hub_cache_for_repo_dir(repo_cache_dir) for variant, _path in download_manifest.iter_variant_manifests( "model", @@ -959,7 +996,10 @@ def is_gguf_repo_partial( variant, snapshot_dir, repo_cache_dir = repo_cache_dir, - cancel_marker_applies = repo_signal_applies, + # A quant whose files are all in the pinned snapshot is loadable + # from it whatever a newer attempt's marker or manifest says, and + # that attempt's manifest lists another revision's files. + repo_signal_applies = repo_signal_applies or variant not in complete_here, ): has_broken = True else: diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 7d8d7dd3f3..48a69c7a32 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -368,17 +368,22 @@ def _two_snapshot_repo( older_files: dict, newer_files: dict, *, - ref: str = UPSTREAM_HEAD, + ref: str | None = UPSTREAM_HEAD, ) -> Path: """A repo whose payload sits in the older of two snapshots. Realistic because a metadata probe (config.json only) against a commit that has moved on materialises a newer, weightless snapshot beside the download. + + ``ref = None`` leaves no ``refs/main`` at all, which is what a fetch pinned + to a commit hash leaves behind: ``snapshot_download`` only writes a ref for + a branch or tag. """ repo_dir = cache_root / "models--Org--Model" (repo_dir / "blobs").mkdir(parents = True, exist_ok = True) (repo_dir / "refs").mkdir(parents = True, exist_ok = True) - (repo_dir / "refs" / "main").write_text(ref, encoding = "utf-8") + if ref is not None: + (repo_dir / "refs" / "main").write_text(ref, encoding = "utf-8") for commit, files in ((OLDER, older_files), (NEWER, newer_files)): snapshot = repo_dir / "snapshots" / commit snapshot.mkdir(parents = True, exist_ok = True) @@ -788,13 +793,26 @@ def _write_repo_wide_signal(kind: str, hub_cache: Path) -> None: pytest.param({"config.json": b"{}"}, NEWER, OLDER, False, id = "pinned-older-snapshot"), # Negative side: when the row advertises the newest snapshot the signal # does describe it, and a cancelled download with no ``.incomplete`` - # blob left behind has nothing else to give it away. + # blob left behind has nothing else to give it away. No ``refs/main`` + # at all, which is what a commit-pinned fetch leaves: the ref carries no + # evidence about the attempt, so the newest snapshot still owns it. + pytest.param( + {"config.json": b"{}", "model.safetensors": b"\0" * 13}, + None, + NEWER, + True, + id = "advertised-snapshot", + ), + # A ``refs/main`` naming a commit with no directory does carry evidence: + # the ref is rewritten before the first file lands, so the attempt that + # left the signal never materialised a snapshot and the complete payload + # already on disk must not inherit it. pytest.param( {"config.json": b"{}", "model.safetensors": b"\0" * 13}, UPSTREAM_HEAD, NEWER, - True, - id = "advertised-snapshot", + False, + id = "unmaterialised-attempt", ), ], ) @@ -852,6 +870,7 @@ def test_a_gguf_download_interrupted_in_its_own_snapshot_is_still_partial(tmp_pa tmp_path, older_files = {"Model-Q4_K_M-00001-of-00002.gguf": b"\0" * 32}, newer_files = {"Model-Q8_0-00001-of-00002.gguf": b"\0" * 16}, + ref = None, ) (repo_dir / "blobs" / ("a" * 40 + ".incomplete")).write_bytes(b"\0" * 3) @@ -862,6 +881,85 @@ def test_a_gguf_download_interrupted_in_its_own_snapshot_is_still_partial(tmp_pa assert rows[0]["capabilities"].get("can_chat") is False +@pytest.mark.parametrize("signal", ["marker", "manifest"]) +def test_an_update_that_never_materialised_leaves_the_cached_payload_chattable( + signal, tmp_path, monkeypatch +): + """The recovered row's own scenario, and the reason it must not arrive + partial. ``snapshot_download`` rewrites ``refs/main`` with the new commit + before it fetches a byte, and the downloader writes the manifest earlier + still, so an update interrupted before the first file leaves the previous + complete snapshot as the only payload on disk under a ref that resolves + nowhere. That is exactly the state this branch recovers rows from, and + charging the interrupted attempt's signals to the cached payload handed + chat auto-load a model it had to skip.""" + repo_dir = _build_repo(tmp_path, ref = UPSTREAM_HEAD) + snapshot = repo_dir / "snapshots" / SNAPSHOT + (snapshot / "config.json").write_text("{}", encoding = "utf-8") + _write_repo_wide_signal(signal, tmp_path) + + rows = _autoload_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == snapshot + assert rows[0].get("partial") is False + assert rows[0]["capabilities"].get("can_chat") is True + + +@pytest.mark.parametrize( + "older_files, newer_files, ref, advertised, partial", + [ + # A same-variant re-download that stops before materialising its + # snapshot leaves a manifest listing the new revision's files. Verifying + # it against the older snapshot the row pins fails on any rename or size + # change, so the one complete quant on disk looked broken. + pytest.param( + {"Model-Q4_K_M.gguf": b"\0" * 32}, + {"config.json": b"{}"}, + NEWER, + OLDER, + False, + id = "pinned-older-snapshot", + ), + # Negative side: the quant the manifest names is not complete under the + # pinned snapshot, so the manifest is the only thing that can judge it + # and the row stays partial. + pytest.param( + {"config.json": b"{}"}, + {"Model-Q4_K_M.gguf": b"\0" * 32}, + None, + NEWER, + True, + id = "advertised-snapshot", + ), + ], +) +def test_a_gguf_variant_manifest_is_scoped_to_the_snapshot_the_row_pins( + older_files, newer_files, ref, advertised, partial, tmp_path, monkeypatch +): + from hub.utils import download_manifest + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = older_files, + newer_files = newer_files, + ref = ref, + ) + download_manifest.write_manifest( + "model", + "Org/Model", + "Q4_K_M", + [download_manifest.ExpectedFile("Model-Q4_K_M.gguf", 999)], + "http", + hub_cache = tmp_path, + ) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + assert Path(rows[0]["load_id"]) == repo_dir / "snapshots" / advertised + assert rows[0].get("partial") is partial + assert rows[0]["capabilities"].get("can_chat") is not partial + + def test_a_gguf_variant_marker_from_a_newer_attempt_does_not_disable_the_pinned_quant( tmp_path, monkeypatch ): @@ -905,6 +1003,7 @@ def test_a_gguf_variant_marker_against_the_advertised_snapshot_is_still_partial( tmp_path, older_files = {"config.json": b"{}"}, newer_files = {"Model-Q4_K_M.gguf": b"\0" * 32}, + ref = None, ) download_manifest.write_cancel_marker( "model", "Org/Model", "Q4_K_M", "http", hub_cache = tmp_path From 2e0f1b1068cbefd7dea7d3768d9cecf552d4368a Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 29 Jul 2026 07:41:20 +0000 Subject: [PATCH 27/29] Correct the dangling-ref docstring to name snapshot_download for PR #7375 --- studio/backend/hub/utils/inventory_scan.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index df58c116ce..7c23352f83 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -214,11 +214,14 @@ def _recover_repo_hidden_by_dangling_refs(repo_dir: Path) -> Optional[_Recovered ``_scan_cached_repo`` assembles every revision successfully and only then raises ``CorruptedCacheException`` because a ``refs/`` file names a commit with no ``snapshots//`` dir, so ``scan_cache_dir`` omits an - entirely intact repo from ``.repos``. Studio creates that state itself: a - metadata probe that 404s writes ``refs/main`` at the live upstream HEAD - without materialising that snapshot, so any repo re-uploaded since it was - downloaded goes invisible to every inventory endpoint while the model - picker's plain directory walk still lists it. + entirely intact repo from ``.repos``. Studio creates that state itself: + ``snapshot_download`` writes ``refs/main`` at the live upstream sha *before* + fetching the first file and never creates ``snapshots//`` itself, and no + Studio caller pins ``revision``. So any repo re-uploaded since it was + downloaded goes invisible to every inventory endpoint the moment a refresh + starts, while the model picker's plain directory walk still lists it. No race + is needed: when the allow/ignore patterns match nothing the download returns + normally having written only the ref. This reads the same directories huggingface_hub reads and writes nothing: the ref file that upstream's assertion trips over is left exactly as it is. From 4d349f29e6931d0b79530cb9bff89f42311ee611 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 29 Jul 2026 07:57:38 +0000 Subject: [PATCH 28/29] Offer the whole quant from a mixed newest snapshot instead of an older larger one --- .../hub/services/models/cache_inventory.py | 10 ++-- studio/backend/hub/utils/gguf.py | 39 ++++++++-------- studio/backend/hub/utils/inventory_scan.py | 25 ++++++++++ .../tests/test_hf_cache_dangling_refs.py | 46 +++++++++++++++++++ 4 files changed, 97 insertions(+), 23 deletions(-) diff --git a/studio/backend/hub/services/models/cache_inventory.py b/studio/backend/hub/services/models/cache_inventory.py index 996fe09b60..14f03d9da4 100644 --- a/studio/backend/hub/services/models/cache_inventory.py +++ b/studio/backend/hub/services/models/cache_inventory.py @@ -513,8 +513,12 @@ def _repo_gguf_payload_snapshots(repo_info) -> tuple[Optional[Path], frozenset[s must agree or an advertised quant resolves to nothing. A snapshot holding only part of a split quant is not usable either: the picker still offers that quant and the generated command asks for shards that are absent, so - prefer a complete one exactly as ``_repo_gguf_load_id`` does. Fall back to - any primary GGUF when nothing is complete, which is what shipped before. + prefer one holding a whole quant exactly as ``_repo_gguf_load_id`` does. A + snapshot that mixes a whole quant with an interrupted split one still counts, + because the lister trims its offer to the completed subset; demanding the + whole directory be complete would hide that finished quant behind an older + revision's larger one. Fall back to any primary GGUF when nothing is + complete, which is what shipped before. """ # Matched on the snapshot-relative path, not ``file_name``: huggingface_hub # sets that to the bare name for a nested file (the recovered mirror copies @@ -531,7 +535,7 @@ def _repo_gguf_payload_snapshots(repo_info) -> tuple[Optional[Path], frozenset[s complete = [ snapshot for snapshot in with_gguf - if hf_cache_scan.snapshot_variants_all_complete(str(snapshot)) + if hf_cache_scan.snapshot_has_complete_variants(str(snapshot)) ] usable = complete or with_gguf return _newest_snapshot_dir(usable), _resolved_snapshot_ids(usable) diff --git a/studio/backend/hub/utils/gguf.py b/studio/backend/hub/utils/gguf.py index 3317040904..121e7816a7 100644 --- a/studio/backend/hub/utils/gguf.py +++ b/studio/backend/hub/utils/gguf.py @@ -317,42 +317,41 @@ def list_gguf_variants_from_hf_cache( repo_id: str, root: Optional[Path] = None ) -> Optional[tuple[list[GgufVariantInfo], bool]]: # Imported here, not at module scope: inventory_scan imports this module. - from hub.utils.inventory_scan import ( - complete_snapshot_variants, - snapshot_variants_all_complete, - ) + from hub.utils.inventory_scan import complete_snapshot_variants snapshots = ( iter_hf_cache_snapshots(repo_id, root = root) if root is not None else iter_hf_cache_snapshots(repo_id) ) - # The inventory row hands out the newest snapshot whose quants are all on - # disk as its load id, and a local load reads only that one directory. A - # plain newest-first walk reports a half-downloaded split quant from a newer - # snapshot as downloaded while /load points at an older snapshot that does - # not hold it, so the same preference is applied here, exactly as - # _repo_gguf_payload_snapshots does. The vision flag is OR-ed in because a - # skipped newer snapshot can be a projector fetched on its own; when nothing - # is complete the first snapshot with anything wins, as it did before. + # The inventory row hands out one snapshot as its load id and a local load + # reads only that one directory, so this walk has to land on the same one. + # A plain newest-first walk reports a half-downloaded split quant from a + # newer snapshot as downloaded while /load points elsewhere, so the newest + # snapshot holding at least one whole quant wins, exactly as + # _repo_gguf_payload_snapshots does, and only that snapshot's completed + # subset is offered. Requiring the whole directory to be complete instead + # would skip a newer snapshot that mixes a finished quant with an + # interrupted one and hide the finished quant behind an older revision's + # larger one, which auto-load may not have the memory for. # - # That fallback still reports every quant it finds as downloaded. Auto-load - # takes only the smallest one and a rejected load now suppresses the default - # download, so a half-downloaded split quant beside a whole one left chat - # with no model at all; keep the whole ones when the snapshot has any. + # The vision flag is OR-ed in because a skipped newer snapshot can be a + # projector fetched on its own. When no snapshot has a whole quant the first + # one with anything wins, as it did before. any_vision = False fallback: Optional[tuple[list[GgufVariantInfo], bool]] = None for snapshot in snapshots: variants, has_vision = list_local_gguf_variants(str(snapshot)) any_vision = any_vision or has_vision - if variants and snapshot_variants_all_complete(str(snapshot)): - return variants, any_vision - if fallback is None and (variants or has_vision): + if variants: complete = complete_snapshot_variants(str(snapshot)) # A quant with no label cannot be judged, so it is kept rather than # dropped; with nothing complete the list stays as it was. usable = [v for v in variants if not v.quant or v.quant in complete] - fallback = (usable or variants, has_vision) + if usable: + return usable, any_vision + if fallback is None and (variants or has_vision): + fallback = (variants, has_vision) return fallback diff --git a/studio/backend/hub/utils/inventory_scan.py b/studio/backend/hub/utils/inventory_scan.py index 7c23352f83..250d95d1f8 100644 --- a/studio/backend/hub/utils/inventory_scan.py +++ b/studio/backend/hub/utils/inventory_scan.py @@ -730,6 +730,31 @@ def snapshot_variants_all_complete(snapshot: str) -> bool: return False +def snapshot_has_complete_variants(snapshot: str) -> bool: + """True when at least one quant the variant lister would advertise from + *snapshot* is fully on disk. + + Deliberately weaker than ``snapshot_variants_all_complete``: a snapshot that + mixes a whole quant with an interrupted split one is still loadable for the + whole quant, and the lister trims the offer to that completed subset. Skipping + such a snapshot outright hides a fully downloaded quant behind an older + revision's larger one, which auto-load may not have the memory for. + + Snapshot selection and the offered variants have to agree on one directory, so + every caller that pins a load id uses this predicate and the lister uses the + matching subset. + """ + from hub.utils.gguf import list_local_gguf_variants + try: + variants, _ = list_local_gguf_variants(snapshot) + offered = {v.quant for v in variants if getattr(v, "quant", None)} + if not offered: + return False + return bool(offered & _completed_gguf_variants(Path(snapshot))) + except Exception: + return False + + def complete_snapshot_variants(snapshot: str) -> set[str]: """Quant labels in *snapshot* whose files are all on disk. Same labels as ``snapshot_variants_all_complete`` compares, for callers that need the subset diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 48a69c7a32..49d38c55aa 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -607,6 +607,52 @@ def test_gguf_variants_still_list_when_no_snapshot_is_complete( assert _local_gguf_variants_for_autoload(rows[0], tmp_path) == offered +def test_a_whole_quant_in_a_mixed_newest_snapshot_beats_an_older_larger_one( + tmp_path, monkeypatch +): + """A whole small quant can sit in the newest snapshot beside an interrupted + split one while an older snapshot holds nothing but a whole larger quant. + + Requiring the whole directory to be complete skipped that newest snapshot + outright, so both the load id and the offered quants fell back to the older + revision and the fully downloaded small quant disappeared. Auto-load takes + only the smallest quant offered, so it would then spend its one attempt on + the larger one and, if that does not fit in memory, leave chat with no model + while a usable quant sat on disk. The snapshot counts as usable and its + completed subset is what gets offered, so both ends still name one directory + and the interrupted quant is still withheld.""" + from hub.utils.gguf import list_local_gguf_variants + + repo_dir = _two_snapshot_repo( + tmp_path, + older_files = {"Model-Q6_K.gguf": b"\0" * 96}, + newer_files = { + "Model-Q4_K_M.gguf": b"\0" * 16, + "Model-Q8_0-00001-of-00002.gguf": b"\0" * 8, + }, + ) + + rows = _autoload_gguf_rows(tmp_path, monkeypatch) + + assert [row["repo_id"] for row in rows] == ["Org/Model"] + load_dir = Path(rows[0]["load_id"]) + assert load_dir == repo_dir / "snapshots" / NEWER + offered = _local_gguf_variants_for_autoload(rows[0], tmp_path) + assert offered == ["Q4_K_M"] + # The pair still has to agree on one directory, and the interrupted split + # quant must not be advertised from it. + resolvable = {v.quant for v in list_local_gguf_variants(str(load_dir))[0]} + assert set(offered) <= resolvable, ( + f"auto-load is offered {sorted(offered)} but load_id {load_dir.name[:8]} " + f"resolves only {sorted(resolvable)}" + ) + assert "Q8_0" not in offered + # Pinning the snapshot that holds the interrupted download must not flip the + # row partial: a whole quant is loadable from it. + assert rows[0].get("partial") is False + assert rows[0].get("capabilities", {}).get("can_chat") is True + + def test_load_id_is_not_pinned_to_a_snapshot_that_has_no_config(tmp_path, monkeypatch): """The repo-level format is allowed to rest on transformer-named weights alone, but a pinned load id names one directory and from_pretrained needs From 9abe3d7823d34a7fef54f20ae52ad5d98facd8ac Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:04:03 +0000 Subject: [PATCH 29/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- studio/backend/tests/test_hf_cache_dangling_refs.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/studio/backend/tests/test_hf_cache_dangling_refs.py b/studio/backend/tests/test_hf_cache_dangling_refs.py index 49d38c55aa..d311b00d4a 100644 --- a/studio/backend/tests/test_hf_cache_dangling_refs.py +++ b/studio/backend/tests/test_hf_cache_dangling_refs.py @@ -607,9 +607,7 @@ def test_gguf_variants_still_list_when_no_snapshot_is_complete( assert _local_gguf_variants_for_autoload(rows[0], tmp_path) == offered -def test_a_whole_quant_in_a_mixed_newest_snapshot_beats_an_older_larger_one( - tmp_path, monkeypatch -): +def test_a_whole_quant_in_a_mixed_newest_snapshot_beats_an_older_larger_one(tmp_path, monkeypatch): """A whole small quant can sit in the newest snapshot beside an interrupted split one while an older snapshot holds nothing but a whole larger quant.