From 140b3fbe057398fa22b075b20366c62d10e936e5 Mon Sep 17 00:00:00 2001 From: Michael Han <107991372+shimmyshimmer@users.noreply.github.com> Date: Fri, 24 Jul 2026 00:48:54 -0700 Subject: [PATCH 01/15] Studio: register text-ui tokens with tailwind-merge so cn() keeps them (#7396) * Studio: register text-ui tokens with tailwind-merge so cn keeps them Stock tailwind-merge classifies text-ui-* as a text color, so cn() dropped the size class whenever a color utility followed it in the same call. The element then fell back to the unscaled 16px root font, which made hub tabs and capability pills look oversized at small UI font sizes. Extend the merge config so text-ui-* and leading-ui-* resolve as font-size and line-height groups, and cover the failure in the contract and Playwright regression tests. * Studio: rename the Models page to Model hub Page heading, sidebar navigation label in all locales, and the chat download toasts that point at the tab. --- .../frontend/src/features/chat/chat-page.tsx | 4 ++-- .../features/hub/catalog/models-header.tsx | 2 +- studio/frontend/src/i18n/locales/ar.ts | 2 +- studio/frontend/src/i18n/locales/de.ts | 2 +- studio/frontend/src/i18n/locales/en.ts | 2 +- studio/frontend/src/i18n/locales/es.ts | 2 +- studio/frontend/src/i18n/locales/fr.ts | 2 +- studio/frontend/src/i18n/locales/hi.ts | 2 +- studio/frontend/src/i18n/locales/ja.ts | 2 +- studio/frontend/src/i18n/locales/ko.ts | 2 +- studio/frontend/src/i18n/locales/pt-br.ts | 2 +- studio/frontend/src/i18n/locales/ru.ts | 2 +- studio/frontend/src/i18n/locales/zh-CN.ts | 2 +- studio/frontend/src/lib/utils.ts | 16 +++++++++++++++- tests/studio/playwright_ui_font_scale.py | 19 +++++++++++++++++++ tests/studio/test_ui_font_scale_contract.py | 11 +++++++++++ 16 files changed, 59 insertions(+), 15 deletions(-) diff --git a/studio/frontend/src/features/chat/chat-page.tsx b/studio/frontend/src/features/chat/chat-page.tsx index c241607e28..7452cf3447 100644 --- a/studio/frontend/src/features/chat/chat-page.tsx +++ b/studio/frontend/src/features/chat/chat-page.tsx @@ -2289,7 +2289,7 @@ export function ChatPage({ } else if (outcome === "conflict") { toast.info("Resume this download from Models", { description: - "An earlier partial download used a different transport. Open the Models tab to resume or restart it.", + "An earlier partial download used a different transport. Open the Model hub tab to resume or restart it.", }); } else if (outcome === "busy") { toast.info("Download already in progress", { @@ -2410,7 +2410,7 @@ export function ChatPage({ // surface's onComplete auto-loads, mirroring the "started" branch. toast.info("Resume this download from Models", { description: - "An earlier partial download used a different transport. Open the Models tab to resume or restart it.", + "An earlier partial download used a different transport. Open the Model hub tab to resume or restart it.", }); return; } diff --git a/studio/frontend/src/features/hub/catalog/models-header.tsx b/studio/frontend/src/features/hub/catalog/models-header.tsx index f0e0950871..f844336d4a 100644 --- a/studio/frontend/src/features/hub/catalog/models-header.tsx +++ b/studio/frontend/src/features/hub/catalog/models-header.tsx @@ -64,7 +64,7 @@ export function ModelsHeader({ return (
/^ui-\d+(p5)?$/.test(value); + +const twMerge = extendTailwindMerge({ + extend: { + classGroups: { + "font-size": [{ text: [isUiToken] }], + leading: [{ leading: [isUiToken] }], + }, + }, +}); export function cn(...inputs: ClassValue[]): string { return twMerge(clsx(inputs)); diff --git a/tests/studio/playwright_ui_font_scale.py b/tests/studio/playwright_ui_font_scale.py index 89c7894929..0f14c42422 100644 --- a/tests/studio/playwright_ui_font_scale.py +++ b/tests/studio/playwright_ui_font_scale.py @@ -193,6 +193,25 @@ def main(): page.set_viewport_size({"width": 1440, "height": 900}) page.wait_for_timeout(400) + step("cn keeps text-ui-* next to color classes (hub tabs)") + page.keyboard.press("Escape") + page.wait_for_timeout(400) + page.goto(f"{BASE}/hub", wait_until = "domcontentloaded") + page.wait_for_timeout(2000) + open_appearance(page) + set_input(page, "UI font size", 12) + page.keyboard.press("Escape") + page.wait_for_timeout(400) + tab = page.get_by_role("radio").filter(has_text = "Discover").first + tab.wait_for(state = "visible", timeout = 15000) + tab_font = tab.evaluate("el => parseFloat(getComputedStyle(el).fontSize)") + # text-ui-12p5 at scale 0.75; 16px means twMerge dropped the token. + if not near(tab_font, 12.5 * 12 / 16): + fail(f"hub tab font did not scale (twMerge drop?): {tab_font}") + page.goto(BASE, wait_until = "domcontentloaded") + page.wait_for_timeout(1500) + open_appearance(page) + step("default restores exactly") page.get_by_role("dialog").get_by_role("button").filter(has_text = "Appearance").first.click() page.wait_for_timeout(500) diff --git a/tests/studio/test_ui_font_scale_contract.py b/tests/studio/test_ui_font_scale_contract.py index 393c60dd6b..1153eea643 100644 --- a/tests/studio/test_ui_font_scale_contract.py +++ b/tests/studio/test_ui_font_scale_contract.py @@ -17,6 +17,7 @@ SRC = REPO / "studio/frontend/src" INDEX_CSS = (SRC / "index.css").read_text(encoding = "utf-8") STORE = (SRC / "features/settings/stores/appearance-custom-store.ts").read_text(encoding = "utf-8") SELECT = (SRC / "components/ui/select.tsx").read_text(encoding = "utf-8") +UTILS = (SRC / "lib/utils.ts").read_text(encoding = "utf-8") # Raw numeric fontSize props are only allowed where a scaled stylesheet rule # (.recharts-text) overrides the presentation attribute at render time. @@ -87,6 +88,16 @@ def test_radix_select_viewport_owns_the_scroll_state(): assert "overflow-y-auto" not in content_cls.group(1) +def test_cn_knows_the_ui_typography_tokens(): + """Stock tailwind-merge classifies text-ui-* as a text color and deletes + it whenever a real color class follows in the same cn() call, so the + element falls back to the unscaled inherited font size.""" + assert "extendTailwindMerge" in UTILS + assert '"font-size": [{ text: [isUiToken] }]' in UTILS + assert "leading: [{ leading: [isUiToken] }]" in UTILS + assert "/^ui-\\d+(p5)?$/.test(value)" in UTILS + + def test_no_raw_pixel_text_utilities(): offenders = [] for path in _frontend_sources(): From 6e91d1dff8c8a8720e2be5afb8797f7819b63a55 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Fri, 24 Jul 2026 02:12:00 -0700 Subject: [PATCH 02/15] Studio: scan HF cache snapshot loads by their repo id (#7398) * Studio: scan HF cache snapshot loads by their repo id Inactive Hugging Face caches (legacy, default, and previously selected download locations) are loaded by their resolved snapshot path so they keep using the selected cache instead of re-downloading. That path is a local filesystem path, so evaluate_file_security exempted it with "local path; no Hub scan" and skipped Hugging Face's pickle/malware scan. Active caches load by repo id and are still scanned, so the same model could dodge the gate simply by being in an inactive cache. An HF cache snapshot keeps the canonical models--org--repo/snapshots/ layout, so recover the repo id from that path and scan it instead of exempting it. Non-cache local paths (models directory, custom folders) still skip the scan, and a remote ref is still scanned by repo id. Adds a regression test that a flagged pickle in an inactive-cache snapshot path blocks the load. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: scan the exact cached commit for inactive HF caches An HF cache snapshot path encodes the commit, not just the repo id (models--org--repo/snapshots/). Recover the revision alongside the repo id and pass it to model_info and the shard-index lookup so the scan covers the exact files that will be deserialized, rather than the repo's default branch. Without this, a pickle in an older cached commit that was later removed from the branch would scan clean and still load. Extends the regression test to assert the recovered revision is forwarded to the Hub scan. --------- Co-authored-by: danielhanchen Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- studio/backend/tests/test_file_security.py | 17 +++++++ .../backend/utils/security/file_security.py | 51 ++++++++++++++++--- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/studio/backend/tests/test_file_security.py b/studio/backend/tests/test_file_security.py index b4c8f5d242..e02c33a0f1 100644 --- a/studio/backend/tests/test_file_security.py +++ b/studio/backend/tests/test_file_security.py @@ -165,6 +165,23 @@ def test_skips_local_path(): assert "local" in d.reason +def test_scans_inactive_hf_cache_snapshot_path(tmp_path): + # An inactive HF cache loads by snapshot path; the gate must recover the repo id + + # commit from models--org--repo/snapshots/ and scan that exact commit, not exempt + # it and not fall back to the default branch (an older commit may hold a dropped pickle). + snapshot = tmp_path / "models--evil--repo" / "snapshots" / "deadbeef" + snapshot.mkdir(parents = True) + status = { + "scansDone": True, + "filesWithIssues": [{"path": "pytorch_model.bin", "level": "unsafe"}], + } + with _patch_status(status) as model_info: + d = evaluate_file_security(str(snapshot)) + assert d.blocked is True + assert model_info.call_args.args[0] == "evil/repo" + assert model_info.call_args.kwargs["revision"] == "deadbeef" + + def test_remote_gguf_named_repo_is_still_scanned(): # Only LOCAL paths skip the Hub scan, so a remote .gguf repo is still scanned and a # poisoned pickle smuggled into it is blocked. diff --git a/studio/backend/utils/security/file_security.py b/studio/backend/utils/security/file_security.py index 892f7862a9..3e12c15096 100644 --- a/studio/backend/utils/security/file_security.py +++ b/studio/backend/utils/security/file_security.py @@ -114,6 +114,28 @@ def _file_suffix(path: str) -> str: return "." + base.rsplit(".", 1)[1].lower() if "." in base else "" +def _hf_cache_snapshot_ref(local_path: str) -> Optional[tuple]: + """``(repo_id, revision)`` for an HF-cache snapshot path, else None. An inactive Studio + cache loads by its snapshot path but keeps the ``models--org--repo/snapshots/`` + layout, so the gate recovers its provenance and scans that exact commit instead of + exempting it (an older cached commit can hold a pickle since dropped from the branch).""" + try: + path = Path(local_path).resolve(strict = False) + except (OSError, ValueError): + return None + for parent in path.parents: + if parent.name != "snapshots": + continue + encoded = parent.parent.name + if not encoded.startswith("models--"): + return None + repo_id = encoded.removeprefix("models--").replace("--", "/") + if not repo_id: + return None + return repo_id, path.relative_to(parent).parts[0] # dir under snapshots/ + return None + + def _load_relative_path(norm: str, load_subdirs) -> str: """``norm`` relative to a ``from_pretrained`` load root. Some loads read from a snapshot SUBDIRECTORY (Spark-TTS / BiCodec load ``/LLM``), where a file @@ -141,13 +163,14 @@ def _indexed_shard_paths( model_name: str, hf_token: Optional[str], load_subdirs = (), + revision: Optional[str] = None, ): """Repo-relative weight paths a load could fetch via weight-index files. Returns a set (empty when the repo ships no index files -- a definitive "nothing sharded"), or None when the lookup was inconclusive (transient error) so the caller treats a flagged subdir pickle conservatively. Reads only small JSON indexes, never weights. Indexes are looked up at the root and each ``load_subdirs`` root, with ``weight_map`` - entries re-prefixed to repo-relative paths. + entries re-prefixed to repo-relative paths. ``revision`` scopes to a cached commit. """ import json @@ -166,6 +189,7 @@ def _indexed_shard_paths( index_path = hf_hub_download( model_name, prefix + filename, + revision = revision, token = hf_token or None, cache_dir = active_hf_hub_cache(), ) @@ -260,9 +284,14 @@ def _load_scan_target(model_name: str, load_subdirs: tuple) -> tuple: return model_name, load_subdirs -def _fetch_security_status(model_name: str, hf_token: Optional[str]): +def _fetch_security_status( + model_name: str, + hf_token: Optional[str], + revision: Optional[str] = None, +): """``security_repo_status`` (a dict) or None if unavailable. Hub metadata only; retries once on a transient error, then returns None so the caller fails open. + ``revision`` scopes the scan to a specific cached commit (else the default branch). """ from huggingface_hub import model_info as hf_model_info @@ -272,6 +301,7 @@ def _fetch_security_status(model_name: str, hf_token: Optional[str]): try: info = hf_model_info( model_name, + revision = revision, token = token_arg, securityStatus = True, timeout = timeout, @@ -485,12 +515,17 @@ def evaluate_file_security( # fails open): the Spark-TTS "/LLM" alias is really unsloth/ from LLM/. model_name, load_subdirs = _load_scan_target(model_name, tuple(load_subdirs)) - # Local paths (including a local .gguf) have no Hub scan. A remote ref is scanned - # even if named "*.gguf", so a repo cannot dodge the scan via its name. + # Local paths have no Hub scan, EXCEPT an HF-cache snapshot whose canonical path + # encodes a repo id + commit: scan that exact commit so an inactive-cache load can't + # dodge the gate. A remote ref is scanned even if named "*.gguf" (name can't dodge it). + snapshot_revision = None try: from utils.paths import is_local_path if is_local_path(model_name): - return FileSecurityDecision(model_name, False, reason = "local path; no Hub scan") + cache_ref = _hf_cache_snapshot_ref(model_name) + if cache_ref is None: + return FileSecurityDecision(model_name, False, reason = "local path; no Hub scan") + model_name, snapshot_revision = cache_ref except Exception: # Cannot classify the path -> do not block on that account. return FileSecurityDecision(model_name, False, reason = "path check failed; not blocked") @@ -499,7 +534,7 @@ def evaluate_file_security( if local_only_load: return _evaluate_local_only(model_name) - status = _fetch_security_status(model_name, hf_token) + status = _fetch_security_status(model_name, hf_token, revision = snapshot_revision) if not isinstance(status, dict): return FileSecurityDecision( model_name, False, reason = "scan unavailable; allowed (fail-open)" @@ -536,7 +571,9 @@ def evaluate_file_security( maybe_shard.append({"path": path, "level": level, "norm": norm}) if maybe_shard: - indexed = _indexed_shard_paths(model_name, hf_token, load_subdirs) + indexed = _indexed_shard_paths( + model_name, hf_token, load_subdirs, revision = snapshot_revision + ) for m in maybe_shard: # Block if a root index lists this shard, or if the lookup was inconclusive # (transient error -> stay conservative). A definitive "no index / not listed" From 418ae14388318dd9f9d152afb3d36b2afc086f2f Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Fri, 24 Jul 2026 02:12:16 -0700 Subject: [PATCH 03/15] Fix ROCm wheel-index unit test: extract the gfx-arch probe helpers get_torch_index_url now calls (#7399) * Fix ROCm wheel-index test: extract the gfx-arch probe helpers get_torch_index_url now calls get_torch_index_url gained a gfx-arch probe on the ROCm path (Strix reroute work) and now calls _ensure_rocm_probe_env, _probe_amd_gfx_arch, _infer_linux_amd_gfx_arch and friends. The unit test in tests/sh/test_get_torch_index_url.sh sources a curated subset of install.sh functions, and that list was never updated, so those helpers were undefined in the harness. On the ROCm path the gfx probe hit an undefined function, the branch silently fell through to the CPU wheel index, and every ROCm assertion failed (9 failures: all ROCm versions resolved to /whl/cpu). Extract the six missing helpers so the ROCm branch runs end to end. All 49 assertions pass. Adds a comment noting these must stay in sync with install.sh. * Keep the ROCm wheel-index test hermetic: redirect the /opt/rocm prefix Extracting _ensure_rocm_probe_env pulled its absolute-path host probe into the harness: it appends /opt/rocm/bin to PATH and runs the real host rocminfo, and version detection reads /opt/rocm/.info/version. On a host with ROCm installed that leaks the host GPU into the minimal-PATH test, so the no-GPU and CUDA-visible-device assertions could select a host ROCm wheel index instead of their expected CPU result, making the test host-dependent. Redirect the whole /opt/rocm prefix to an empty temp dir in the same sed pass that stubs /usr/bin/nvidia-smi, so the probes stay hermetic. All 49 assertions pass and the generated harness contains no real /opt/rocm path. --------- Co-authored-by: danielhanchen --- tests/sh/test_get_torch_index_url.sh | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/sh/test_get_torch_index_url.sh b/tests/sh/test_get_torch_index_url.sh index 23902097ef..633d8ca17b 100755 --- a/tests/sh/test_get_torch_index_url.sh +++ b/tests/sh/test_get_torch_index_url.sh @@ -14,6 +14,13 @@ FAIL=0 # controllable path so we can test the "no GPU" scenario on GPU machines. _FUNC_FILE=$(mktemp) _FAKE_SMI_DIR=$(mktemp -d) +# The ROCm probe helpers read the real /opt/rocm prefix by ABSOLUTE path +# (_ensure_rocm_probe_env appends /opt/rocm/bin to PATH and runs the host +# rocminfo; version detection reads /opt/rocm/.info/version). On a real ROCm +# host that leaks the host GPU into the minimal-PATH harness and makes the +# no-GPU / CPU assertions host-dependent. Redirect the whole prefix to an empty +# temp dir so the probes stay hermetic (same idea as the nvidia-smi rewrite). +_FAKE_ROCM_DIR=$(mktemp -d) { sed -n '/^_run_bounded()/,/^}/p' "$INSTALL_SH" echo "" @@ -23,10 +30,28 @@ _FAKE_SMI_DIR=$(mktemp -d) echo "" sed -n '/^_has_usable_nvidia_gpu()/,/^}/p' "$INSTALL_SH" echo "" + # ROCm gfx-arch probe helpers that get_torch_index_url / _has_amd_rocm_gpu + # now call. These MUST stay in sync with install.sh: if get_torch_index_url + # references a helper that is not extracted here, the ROCm branch hits an + # undefined function, silently falls through to the CPU wheel index, and the + # ROCm assertions below fail. + sed -n '/^_ensure_rocm_probe_env()/,/^}/p' "$INSTALL_SH" + echo "" + sed -n '/^_probe_amd_gfx_arch()/,/^}/p' "$INSTALL_SH" + echo "" + sed -n '/^_amd_gpu_present_via_pci()/,/^}/p' "$INSTALL_SH" + echo "" + sed -n '/^_infer_amd_gfx_arch_from_gpu_name()/,/^}/p' "$INSTALL_SH" + echo "" + sed -n '/^_infer_linux_amd_gfx_arch()/,/^}/p' "$INSTALL_SH" + echo "" + sed -n '/^_amd_arch_index_family_for_gfx()/,/^}/p' "$INSTALL_SH" + echo "" sed -n '/^_trim_index_path_slashes()/,/^}/p' "$INSTALL_SH" echo "" sed -n '/^get_torch_index_url()/,/^}/p' "$INSTALL_SH" -} | sed "s|/usr/bin/nvidia-smi|$_FAKE_SMI_DIR/nvidia-smi-absent|g" \ +} | sed -e "s|/usr/bin/nvidia-smi|$_FAKE_SMI_DIR/nvidia-smi-absent|g" \ + -e "s|/opt/rocm|$_FAKE_ROCM_DIR|g" \ > "$_FUNC_FILE" # Save system PATH so we always have basic tools (uname, grep, head, etc.) @@ -438,6 +463,7 @@ assert_eq "url override preserves fragment slash" "https://mirror.example.com/wh rm -f "$_FUNC_FILE" rm -rf "$_FAKE_SMI_DIR" +rm -rf "$_FAKE_ROCM_DIR" rm -rf "$_TOOLS_DIR" echo "" From 0e800d213aacd48ae8c6dcf6e0745182fb8991af Mon Sep 17 00:00:00 2001 From: Souravrajvi0 <144546710+Souravrajvi0@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:43:52 +0530 Subject: [PATCH 04/15] fix(studio): stop false MTP/vision capability reports (#7332) * fix(studio): stop false MTP/vision capability reports (#7302) MTP probing only inspected the first physical --spec-type help line and treated empty/crash --help output as "lacks MTP", which false-warned on otherwise capable builds. Parse the full --spec-type help block, fail open when the probe is inconclusive, and stop blaming bare mmproj crashes on a projector-format mismatch when the text-only retry also fails. Fixes #7302 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix(studio): tighten MTP probe semantics per Codex review (#7302) Treat nonempty --help without --spec-type as definitive no-MTP, keep only empty/crash probes inconclusive, skip binary_no_mtp UI hint on inconclusive loads, and stop reporting supports_mtp=True in /status for unknown probes. * Treat failed llama-server --help probes as inconclusive (#7302) Gate definitive no-MTP results on a zero exit code so crash diagnostics with nonempty stderr do not re-enable the false lacks-MTP warning path. * Add returncode to probe test mock so probe_ok gating passes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fail open in /status when the MTP probe is inconclusive * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Report missing llama-server as lacking MTP in /status * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten comments in MTP/mmproj probe changes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han --- studio/backend/core/inference/llama_cpp.py | 155 +++++++++++++----- studio/backend/main.py | 6 +- studio/backend/routes/inference.py | 9 +- .../tests/test_llama_cpp_mmproj_fallback.py | 21 +++ .../tests/test_llama_cpp_mtp_detection.py | 114 ++++++++++++- 5 files changed, 262 insertions(+), 43 deletions(-) diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index 7d67339c1e..1fe134c3f9 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -2781,6 +2781,7 @@ class LlamaCppBackend: "found": False, "mtp_token": None, "supports_mtp": False, + "mtp_probe_inconclusive": True, "ngram_mod_flavor": None, "supports_ngram_mod": False, "spec_draft_n_max_flag": None, @@ -2813,6 +2814,9 @@ class LlamaCppBackend: supports_no_cache_prompt = False supports_metrics = False supports_slot_save = False + saw_spec_type = False + probe_ok = False + help_text = "" try: probe_env = cls._llama_server_env_for_binary(bin_path) result = subprocess.run( @@ -2824,6 +2828,7 @@ class LlamaCppBackend: check = False, env = probe_env, ) + probe_ok = result.returncode == 0 help_text = (result.stdout or "") + "\n" + (result.stderr or "") # Split into per-flag blocks (each --flag line + its indented # continuation), so the "argument has been removed" description @@ -2868,17 +2873,19 @@ class LlamaCppBackend: return False return "argument has been removed" not in desc - # MTP token from the --spec-type line. - spec_line = "" - for line in help_text.splitlines(): - if "--spec-type" in line: - spec_line = line - break - # PR #22673 used draft-mtp; later renamed to mtp. - if "draft-mtp" in spec_line: - mtp_token = "draft-mtp" - elif re.search(r"[|,\[]mtp[|,\]]", spec_line): - mtp_token = "mtp" + # MTP token from the full --spec-type help block (decl + indented + # continuation). First-line-only probing missed builds putting the + # enum on the next line (#7302). Prefer draft-mtp (PR #22673) over mtp. + spec_help = blocks.get("--spec-type") or "" + if not spec_help: + # Fallback: join --spec-type lines, avoiding incidental "mtp" in --help. + spec_help = "\n".join( + line for line in help_text.splitlines() if "--spec-type" in line + ) + mtp_token = cls._mtp_token_from_spec_help(spec_help) + # Only a resolved --spec-type block confirms missing MTP; empty/crash + # leaves saw_spec_type False so supports_mtp fails open. + saw_spec_type = bool(spec_help.strip()) and "--spec-type" in spec_help # ngram-mod flag flavor. Post-rename builds advertise both new # args (real) and legacy ones (stubs); pre-rename builds only @@ -2914,11 +2921,29 @@ class LlamaCppBackend: supports_slot_save = _is_real("--slot-save-path") except (OSError, subprocess.SubprocessError) as exc: logger.debug(f"llama-server --help probe failed: {exc}") + saw_spec_type = False + probe_ok = False + help_text = "" + + help_nonempty = bool(help_text.strip()) + # Confirmed only when a successful --help lists a --spec-type block with + # mtp/draft-mtp; nonempty --help without it is a definitive pre-spec + # binary; failed/empty probes stay inconclusive (#7302). + if saw_spec_type and probe_ok: + supports_mtp = mtp_token is not None + mtp_probe_inconclusive = False + elif help_nonempty and probe_ok: + supports_mtp = False + mtp_probe_inconclusive = False + else: + supports_mtp = False + mtp_probe_inconclusive = True info = { "found": True, "mtp_token": mtp_token, - "supports_mtp": mtp_token is not None, + "supports_mtp": supports_mtp, + "mtp_probe_inconclusive": mtp_probe_inconclusive, "ngram_mod_flavor": ngram_mod_flavor, "supports_ngram_mod": ngram_mod_flavor is not None, "spec_draft_n_max_flag": spec_draft_n_max_flag, @@ -2934,6 +2959,21 @@ class LlamaCppBackend: cls._capability_cache[cache_key] = info return info + @staticmethod + def _mtp_token_from_spec_help(spec_help: str) -> Optional[str]: + """Extract ``draft-mtp`` / ``mtp`` from a ``--spec-type`` help snippet. + + Prefers ``draft-mtp`` (llama.cpp PR #22673) over the later bare ``mtp`` + rename. Returns ``None`` when neither token appears as an enum value. + """ + text = spec_help or "" + if "draft-mtp" in text: + return "draft-mtp" + # Bare `mtp` enum token (`|mtp|`, `,mtp,`, ...), not a substring. + if re.search(r"(? str: + """User-facing error when the text-only --mmproj strip retry also fails. + + Confirmed projector-format mismatches keep the historical wording. + Bare signal crashes (common on some ROCm/driver paths) must not be + reported as "Vision projector incompatible" — that misled #7302. + """ + if projector_confirmed: + return ( + "Vision projector incompatible with this llama.cpp " + "build, and the text-only retry also failed: " + detail + ) + return ( + "Vision model failed to start (llama-server crashed with " + "--mmproj), and the text-only retry also failed: " + detail + ) + @staticmethod def _output_has_nonprojector_diagnostic(output: str) -> bool: """True when the output already names a concrete non-projector cause (out @@ -8388,23 +8446,29 @@ class LlamaCppBackend: self._kill_process() # The #6415 split-axis abort is latched earlier (first spawn). # Skip if a cancel/unload is pending (mirrors the MTP guard). + _projector_msg = self._is_projector_incompatibility(out) + _signal_mmproj_guess = self._is_signal_crash( + _crash_rc + ) and not self._output_has_nonprojector_diagnostic(out) if ( launched_with_mmproj and not self._cancel_event.is_set() - and ( - self._is_projector_incompatibility(out) - or ( - self._is_signal_crash(_crash_rc) - and not self._output_has_nonprojector_diagnostic(out) - ) - ) + and (_projector_msg or _signal_mmproj_guess) ): - logger.warning( - "llama-server could not load this model's vision " - "projector (--mmproj). The installed llama.cpp build is " - "likely too old for it. Loading text-only for this " - "session; run 'unsloth studio update' to enable vision." - ) + if _projector_msg: + logger.warning( + "llama-server could not load this model's vision " + "projector (--mmproj). The installed llama.cpp build is " + "likely too old for it. Loading text-only for this " + "session; run 'unsloth studio update' to enable vision." + ) + else: + logger.warning( + "llama-server crashed while loading this model's vision " + "projector (--mmproj). Retrying text-only for this " + "session; if this persists, run 'unsloth studio update' " + "or check GPU/driver logs." + ) cmd = self._strip_mmproj_args(_last_spawn_cmd) # This retry bypasses _spawn_and_wait, so refresh the # launched-argv snapshot itself -- the zero-offload @@ -8432,14 +8496,16 @@ class LlamaCppBackend: "(e.g. ROCR_VISIBLE_DEVICES=0 exposes only the first " "GPU) before launching Unsloth Studio." ) + _retry_detail = self._classify_llama_start_failure( + "\n".join(self._stdout_lines[-50:]), + gguf_path, + self._model_identifier, + _retry_rc, + ) raise RuntimeError( - "Vision projector incompatible with this llama.cpp " - "build, and the text-only retry also failed: " - + self._classify_llama_start_failure( - "\n".join(self._stdout_lines[-50:]), - gguf_path, - self._model_identifier, - _retry_rc, + self._mmproj_retry_failure_message( + projector_confirmed = _projector_msg, + detail = _retry_detail, ) ) else: @@ -8669,18 +8735,29 @@ class LlamaCppBackend: caps = self.probe_server_capabilities(binary) mtp_token = caps.get("mtp_token") if caps else None if not mtp_token: - logger.warning( - "Requested MTP speculative decoding but " - "llama-server lacks --spec-type mtp/draft-mtp; " - "run `unsloth studio update`. Loading without " - "speculative decoding." - ) + inconclusive = bool(caps.get("mtp_probe_inconclusive")) if caps else True + if inconclusive: + logger.info( + "Requested MTP speculative decoding but llama-server MTP " + "capability probe was inconclusive; loading without " + "speculative decoding." + ) + else: + logger.warning( + "Requested MTP speculative decoding but " + "llama-server lacks --spec-type mtp/draft-mtp; " + "run `unsloth studio update`. Loading without " + "speculative decoding." + ) # Override an inherited LLAMA_ARG_SPEC_TYPE=draft-mtp (CLI wins # over env) so the child matches the binary-capability gate and # the no-MTP budget, like the sibling no-head/non-MTP fallbacks. flags.append("--spec-default") self._speculative_type = "default" - self._spec_fallback_reason = "binary_no_mtp" + if inconclusive: + self._spec_fallback_reason = None + else: + self._spec_fallback_reason = "binary_no_mtp" return False draft_n_max = _resolved_draft_n_max() n_max_flag = caps.get("spec_draft_n_max_flag") or "--spec-draft-n-max" diff --git a/studio/backend/main.py b/studio/backend/main.py index a538f935ff..5af25efa74 100644 --- a/studio/backend/main.py +++ b/studio/backend/main.py @@ -438,7 +438,11 @@ def _run_llama_cpp_startup_probes(app: FastAPI) -> None: import structlog as _structlog _log = _structlog.get_logger(__name__) - if _caps.get("found") and not _caps.get("supports_mtp"): + if ( + _caps.get("found") + and not _caps.get("supports_mtp") + and not _caps.get("mtp_probe_inconclusive") + ): _msg = ( "llama.cpp prebuilt lacks MTP support " "(--spec-type mtp/draft-mtp). Run `unsloth studio update`. " diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index 6dbaa8fcc9..445a26f04d 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -5835,10 +5835,15 @@ async def get_status(current_subject: str = Depends(get_current_subject)): try: _bin = type(llama_backend)._find_llama_server_binary() _caps = type(llama_backend).probe_server_capabilities(_bin) - _supports_mtp = bool(_caps.get("supports_mtp", False)) + # Fail open on inconclusive probes: False means a definitive + # "binary lacks MTP" to API consumers. + _supports_mtp = bool( + _caps.get("supports_mtp", False) + or (_caps.get("found", False) and _caps.get("mtp_probe_inconclusive", False)) + ) except Exception: _bin = None - _supports_mtp = True # fail open + _supports_mtp = False # no usable binary: MTP genuinely unavailable try: from utils.llama_cpp_freshness import check_prebuilt_freshness _freshness = check_prebuilt_freshness(_bin) diff --git a/studio/backend/tests/test_llama_cpp_mmproj_fallback.py b/studio/backend/tests/test_llama_cpp_mmproj_fallback.py index 049058e511..4332a440a5 100644 --- a/studio/backend/tests/test_llama_cpp_mmproj_fallback.py +++ b/studio/backend/tests/test_llama_cpp_mmproj_fallback.py @@ -335,3 +335,24 @@ class TestRetryContract: def test_external_kill_skips_flash_attn_retry(self): # SIGKILL (-9, OOM killer) is not a program fault: no FA-off retry. assert _signal_crash(-9) is False + + +class TestMmprojRetryFailureMessage: + """#7302: bare mmproj crashes must not be reported as projector-format.""" + + def test_confirmed_projector_keeps_historical_wording(self): + msg = LlamaCppBackend._mmproj_retry_failure_message( + projector_confirmed = True, + detail = "llama-server failed to start", + ) + assert msg.startswith("Vision projector incompatible with this llama.cpp") + assert "llama-server failed to start" in msg + + def test_bare_crash_does_not_claim_projector_incompatibility(self): + msg = LlamaCppBackend._mmproj_retry_failure_message( + projector_confirmed = False, + detail = "llama-server failed to start. Check that the GGUF file is valid", + ) + assert "Vision projector incompatible" not in msg + assert "crashed with --mmproj" in msg + assert "GGUF file is valid" in msg diff --git a/studio/backend/tests/test_llama_cpp_mtp_detection.py b/studio/backend/tests/test_llama_cpp_mtp_detection.py index 1d15647967..27c1b17a85 100644 --- a/studio/backend/tests/test_llama_cpp_mtp_detection.py +++ b/studio/backend/tests/test_llama_cpp_mtp_detection.py @@ -637,7 +637,9 @@ def test_probe_server_capabilities_uses_binary_library_env(tmp_path, monkeypatch def fake_run(cmd, **kwargs): captured["cmd"] = cmd captured["env"] = kwargs.get("env") - return _types.SimpleNamespace(stdout = "--spec-type none,mtp,ngram-simple\n", stderr = "") + return _types.SimpleNamespace( + stdout = "--spec-type none,mtp,ngram-simple\n", stderr = "", returncode = 0 + ) monkeypatch.setattr("core.inference.llama_cpp.subprocess.run", fake_run) @@ -678,6 +680,95 @@ def test_probe_server_capabilities_reports_outdated_binary(tmp_path): assert caps["found"] is True assert caps["mtp_token"] is None assert caps["supports_mtp"] is False + assert caps["mtp_probe_inconclusive"] is False + + +@_NEEDS_BASH +def test_probe_server_capabilities_reads_mtp_from_multiline_help(tmp_path): + # Enum on the indented line: first-line-only probing falsely reported + # "lacks MTP" (#7302). + fake = _make_fake_llama_server( + tmp_path / "llama-server", + "--spec-type TYPE\n" + " speculative decoding type\n" + " (none,draft-simple,draft-mtp,ngram-mod)\n", + ) + _clear_caps_cache() + caps = LlamaCppBackend.probe_server_capabilities(str(fake)) + assert caps["mtp_token"] == "draft-mtp" + assert caps["supports_mtp"] is True + assert caps["mtp_probe_inconclusive"] is False + + +@_NEEDS_BASH +def test_probe_server_capabilities_empty_help_fails_open(tmp_path): + # --help prints nothing: must not claim the prebuilt lacks MTP (#7302). + fake = tmp_path / "llama-server" + fake.write_text("#!/usr/bin/env bash\nexit 0\n") + fake.chmod(0o755) + _clear_caps_cache() + caps = LlamaCppBackend.probe_server_capabilities(str(fake)) + assert caps["found"] is True + assert caps["mtp_token"] is None + assert caps["supports_mtp"] is False + assert caps["mtp_probe_inconclusive"] is True + + +@_NEEDS_BASH +def test_probe_server_capabilities_no_spec_type_is_definitive(tmp_path): + # Nonempty --help without --spec-type: pre-spec binary, not inconclusive. + fake = _make_fake_llama_server( + tmp_path / "llama-server", + "--gpu-layers N\n GPU layers to offload\n", + ) + _clear_caps_cache() + caps = LlamaCppBackend.probe_server_capabilities(str(fake)) + assert caps["found"] is True + assert caps["mtp_token"] is None + assert caps["supports_mtp"] is False + assert caps["mtp_probe_inconclusive"] is False + + +@_NEEDS_BASH +def test_probe_server_capabilities_failed_help_with_output_is_inconclusive(tmp_path): + fake = tmp_path / "llama-server" + fake.write_text( + "#!/usr/bin/env bash\n" + 'if [ "$1" = "--help" ]; then\n' + " echo 'illegal instruction'\n" + " exit 1\n" + "fi\n" + ) + fake.chmod(0o755) + _clear_caps_cache() + caps = LlamaCppBackend.probe_server_capabilities(str(fake)) + assert caps["found"] is True + assert caps["supports_mtp"] is False + assert caps["mtp_probe_inconclusive"] is True + + +@_NEEDS_BASH +def test_probe_server_capabilities_crash_on_help_fails_open(tmp_path): + fake = tmp_path / "llama-server" + fake.write_text("#!/usr/bin/env bash\nkill -SEGV $$\n") + fake.chmod(0o755) + _clear_caps_cache() + caps = LlamaCppBackend.probe_server_capabilities(str(fake)) + assert caps["found"] is True + assert caps["mtp_token"] is None + assert caps["supports_mtp"] is False + assert caps["mtp_probe_inconclusive"] is True + + +def test_mtp_token_from_spec_help_prefers_draft_mtp(): + assert ( + LlamaCppBackend._mtp_token_from_spec_help("--spec-type none,draft-mtp,mtp,ngram-mod") + == "draft-mtp" + ) + assert LlamaCppBackend._mtp_token_from_spec_help("--spec-type [none|mtp|ngram-cache]") == "mtp" + assert LlamaCppBackend._mtp_token_from_spec_help("--spec-type none,ngram-mod") is None + # No incidental substring matches. + assert LlamaCppBackend._mtp_token_from_spec_help("prompt cache") is None def test_probe_server_capabilities_handles_missing_binary(): @@ -685,6 +776,7 @@ def test_probe_server_capabilities_handles_missing_binary(): caps = LlamaCppBackend.probe_server_capabilities("/no/such/llama-server") assert caps["found"] is False assert caps["supports_mtp"] is False + assert caps["mtp_probe_inconclusive"] is True assert caps["supports_cache_ram"] is False assert caps["supports_ctx_checkpoints"] is False assert caps["supports_no_cache_prompt"] is False @@ -1176,12 +1268,14 @@ def _resolver_backend( *, ngram_supported = True, mtp_token = "draft-mtp", + mtp_probe_inconclusive = False, ): """Backend with a deterministic probe so the resolver is hermetic.""" fake = { "found": True, "mtp_token": mtp_token, "supports_mtp": bool(mtp_token), + "mtp_probe_inconclusive": mtp_probe_inconclusive, "ngram_mod_flavor": "new" if ngram_supported else None, "supports_ngram_mod": bool(ngram_supported), "spec_draft_n_max_flag": "--spec-draft-n-max", @@ -1879,6 +1973,24 @@ def test_spec_fallback_reason_set_when_binary_lacks_mtp(monkeypatch): assert backend.spec_fallback_reason == "binary_no_mtp" +def test_spec_fallback_reason_none_when_mtp_probe_inconclusive(monkeypatch): + backend = _resolver_backend( + monkeypatch, + mtp_token = None, + mtp_probe_inconclusive = True, + ) + backend._build_speculative_flags( + speculative_type = "mtp", + spec_draft_n_max = None, + extra_args = None, + model_identifier = _MTP_MODEL, + model_path = None, + gpus = True, + binary = "/fake/llama-server", + ) + assert backend.spec_fallback_reason is None + + def test_spec_fallback_reason_none_when_mtp_engages(monkeypatch): backend = _resolver_backend(monkeypatch) backend._build_speculative_flags( From 330586de7c9e849c400b8bbc5112fcde8e964081 Mon Sep 17 00:00:00 2001 From: Souravrajvi0 <144546710+Souravrajvi0@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:52:03 +0530 Subject: [PATCH 05/15] feat(studio): expose full KV cache dtype list in model config UI (#7348) Fixes #7244 The Studio per-model config dropdown only surfaced bf16, q8_0, q5_1, and q4_1 even though llama.cpp already accepts q4_0, q5_0, iq4_nl, and f32. Add the missing options to KV_CACHE_DTYPES and align API field descriptions with the backend _valid_cache_types set. Co-authored-by: Daniel Han --- studio/backend/models/inference.py | 16 +++++++++++++--- studio/backend/routes/models.py | 5 ++++- .../components/model-config-page.tsx | 3 ++- .../model-config/per-model-config.ts | 12 +++++++++++- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/studio/backend/models/inference.py b/studio/backend/models/inference.py index 2663242187..1758efe515 100644 --- a/studio/backend/models/inference.py +++ b/studio/backend/models/inference.py @@ -70,7 +70,10 @@ class LoadRequest(BaseModel): cache_type_kv: Optional[str] = Field( None, - description = "KV cache data type for both K and V (e.g. 'f16', 'bf16', 'q8_0', 'q4_1', 'q5_1')", + description = ( + "KV cache data type for both K and V " + "(e.g. 'f16', 'bf16', 'q8_0', 'q4_0', 'q4_1', 'q5_0', 'q5_1', 'iq4_nl', 'f32')" + ), ) gpu_ids: Optional[List[int]] = Field( None, @@ -442,7 +445,10 @@ class LoadResponse(BaseModel): ) cache_type_kv: Optional[str] = Field( None, - description = "KV cache data type for K and V (e.g. 'f16', 'bf16', 'q8_0')", + description = ( + "KV cache data type for K and V " + "(e.g. 'f16', 'bf16', 'q8_0', 'q4_0', 'q4_1', 'q5_0', 'q5_1', 'iq4_nl', 'f32')" + ), ) chat_template: Optional[str] = Field( None, @@ -602,7 +608,11 @@ class InferenceStatusResponse(BaseModel): ) cache_type_kv: Optional[str] = Field( None, - description = "KV cache quantization dtype (e.g. 'q8_0'), or None for default", + description = ( + "KV cache quantization dtype " + "(e.g. 'f16', 'bf16', 'q8_0', 'q4_0', 'q4_1', 'q5_0', 'q5_1', 'iq4_nl', 'f32'), " + "or None for default" + ), ) chat_template: Optional[str] = Field( None, description = "Model's default chat template (Jinja2 source), if any" diff --git a/studio/backend/routes/models.py b/studio/backend/routes/models.py index 3c0d6ff4ba..ed83a12f48 100644 --- a/studio/backend/routes/models.py +++ b/studio/backend/routes/models.py @@ -2692,7 +2692,10 @@ async def get_kv_cache_estimate( repo_id: str = Query(..., description = "HF repo ID or local path"), quant: str = Query(..., description = "Quantization label (e.g. Q4_K_M)"), n_ctx: int = Query(..., ge = 1, description = "Context length to size the KV cache for"), - cache_type_kv: Optional[str] = Query(None, description = "KV cache dtype (e.g. q8_0)"), + cache_type_kv: Optional[str] = Query( + None, + description = "KV cache dtype (e.g. q8_0, q4_0, q5_0, iq4_nl, f32)", + ), current_subject: str = Depends(get_current_subject), ): """Estimate KV cache + weight bytes for a downloaded GGUF at n_ctx. diff --git a/studio/frontend/src/features/model-picker/components/model-config-page.tsx b/studio/frontend/src/features/model-picker/components/model-config-page.tsx index ee208dc0bc..afbd33af7c 100644 --- a/studio/frontend/src/features/model-picker/components/model-config-page.tsx +++ b/studio/frontend/src/features/model-picker/components/model-config-page.tsx @@ -415,7 +415,8 @@ function GgufAdvancedSettings({ KV Cache Dtype Lower KV cache precision to save VRAM at the cost of some quality. - f16/bf16 are full precision; q8_0/q5_1/q4_1 are quantized. + f16 is the default; bf16 and f32 are full precision; q8_0 through + iq4_nl are quantized.