mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-12 18:34:19 +00:00
* fix(studio): harden training setup and model loading Scan every model load root before approving remote code, and pin third-party codec sources to verified revisions. Align dataset option validation across the UI and backend, preserve manual drafts, and include edits in training start identity. Require job-scoped stop requests and retain bounded early-cancel tombstones without unsafe eviction. * fix(studio): harden training lifecycle and audio loading Preserve early start cancellations with bounded tombstones and explicit capacity handling for concurrent requests. Pin and verify third-party audio sources and codec artifacts with safe archive extraction and offline cache migration. Keep automatic evaluation data separate from every split included in a combined training instruction. Keep training summaries and GPU progress state aligned with the active configuration through public feature exports. Make route and lifecycle tests deterministic by isolating Hub reachability and executor behavior. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix(studio): correct completion masking and offline training Apply Alpaca response markers safely when completion-only training is enabled. Skip optional runtime dependency installs while Studio is offline. Limit causal-conv1d hooks to recognized model families without dropping supported architectures. Disable known-broken TileLang dispatch when offline repair is unavailable. Use cached GGUF and model-size metadata without offline Hub retries. * fix(studio): resolve causal conv kernels from model configs Detect causal-conv1d requirements from resolved model architectures before loading model code. Keep name matching as a fallback while excluding unrelated renamed checkpoints. * fix(studio): preserve registered training cancellation Allow registered start cancellations to reclaim the oldest expiring tombstone when unknown cancellation capacity is full. Preserve the hard capacity limit and 429 response for unregistered request IDs. * Fix reset job scoping, DAC fast path, scan target and subset splits for PR #8103 - /api/train/reset: an unscoped reset could force-terminate a run mid-cancel. The guard now refuses to touch a live run it cannot prove it owns. The field stays optional so pre-rework clients, which POST /reset with no body, keep working. - ensure_dac_speech_weights: install the download into the pinned destination, so later loads hit the fast path instead of re-downloading and re-hashing 295 MB under the install lock. - _requires_security_review_for_model: apply the same load_scan_target alias normalization the sibling remote-code check gained, else the Spark-TTS alias 404s and fails open to "no review needed". - _purge_package_bytecode: best effort. It runs without the install lock over a cache shared by the inference and training workers; 7 of 8 concurrent imports died on it. Also moved inside the try so a failure cannot strand the cache dir on sys.path, and snapshot sys.modules before the origin audit. - commitSubset: clear the backing splits too, else the render-phase draft sync reads the previous subset's split back into the boxes it just reset. - Fix two tests that fail on the branch: the DAC assertion pinned the old return value, and the causal-conv1d assertion matched call formatting. * Make the SSM runtime tests Windows aware ensure_ssm_runtime deliberately skips causal-conv1d on win32 (no prebuilt wheel), so the two install-order assertions only hold off Windows. Caught on a real windows-latest runner. * Update two frontend source contracts the branch moved - captureTrainingStartInputs now delegates to createTrainingStartInputIdentity, so the normalize/flags assertions belong against training-start-inputs.ts. - resetTraining takes a RequiredTrainingJobScope and always sends the body, which is stronger than the hasScope branch the contract pinned. Both fail on the branch today; caught by tests/studio, which the studio backend job does not cover. * Fix unscoped reset compat, pyc purge fail-open and DAC fallback for PR #8103 Corrects four things in my earlier commits on this branch. Unscoped /api/train/reset returned "superseded" (HTTP 200) for a live run. The pre-rework cancel dialog chains stopTrainingRun then a bodyless reset, so an older client read that 200 as success and cleared its UI while training kept running. Return "active" (409) instead: same answer a live run already gives, and one those clients already handle. It still never force-terminates, so a bodyless reset landing between current_job_id being set and _cancel_requested being cleared cannot kill the run that just started. The bytecode purge was made best-effort, but it is the only thing stopping a stale or planted .pyc shadowing a verified .py: the manifest skips __pycache__ and the origin audit reads __file__, which still names the .py. Tolerate only FileNotFoundError, the real concurrent-purge race, and let PermissionError fail the load again. The DAC fast path copies 295 MB inside the hub cache with only Timeout caught, so a full disk turned a hash-verified download into a hard failure. Fall back to the verified hub path on OSError. commitSubset cleared both splits, but setDatasetSubset already does that; the extra setDatasetSplit(null) only cost a runDatasetCheck against an assumed "train" split. * Restore the eval split reset and the cancelled-run dismiss for PR #8103 Two corrections to 4e5389c21. commitSubset: I removed setDatasetEvalSplit(null) because setDatasetSubset already nulls datasetEvalSplit. It does, but it never resets evalSteps, and setDatasetEvalSplit is not a plain setter: it zeroes evalSteps and runs streamingCompatiblePatch. Without it, changing the subset left evaluation armed with no split, which routes/training.py rejects with 422 once streaming is on, and which silently auto-detects an eval split otherwise. Restored. setDatasetSplit(null) stays out, since its only unique effect was a runDatasetCheck against an assumed "train". Unscoped reset: returning "active" for every live run was too broad. The pre-rework cancel dialog only dismisses after stopTrainingRun succeeded, so _cancel_requested is already set and clearing the UI is right; 409 there just wedged the overlay behind a "Training still active" toast. Now 409 only when no stop was requested, which is the stale-tab case the change was for. Still no force_terminate on an unscoped reset. Also covers the __pycache__ branch of the purge, which is the route a planted .pyc actually takes; the existing test only reached the top-level .pyc loop. * Keep live start cancellations and fall back on a full disk for PR #8103 Two fixes plus the regression tests the earlier lifecycle fixes shipped without. Cancelling the active start at tombstone capacity reclaimed a slot by deleting the soonest-expiring entry. Expired ones are already pruned a few lines above, so that entry was always live, and dropping it let a delayed /start spawn the job it had cancelled. Reserve capacity instead: only the owner of the active start reaches that branch and there is at most one, so the table lands at cap + 1 rather than forgetting a cancellation. Unregistered ids still hit the hard cap. ensure_dac_speech_weights migrated a pre-existing legacy file with an unguarded copy, so a hub cache that cannot absorb a second 295 MB copy failed weights that had already passed size and sha256. Same fallback the download branch below it already uses. Tests: renamed and local SSM checkpoints resolving from config rather than name, the owner of an active start staying cancellable at capacity, live cancellations surviving that cancel, and the full-disk legacy DAC fallback. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Opt the pinned source checkouts into Git long paths for PR #8103 Cross-OS CI caught 14 failures on windows-latest, all of them the pinned checkout dying with "error: unable to write file ...: Filename too long". Git for Windows still enforces MAX_PATH unless told otherwise, and the cache nests a 40-char revision, a staging dir and .git/objects under the studio home. A venv-inferred home already measures about 253 of the 260 characters, so a slightly longer user or install path fails on a normal Windows machine, not just under the deeper pytest tmp dir. Passed per invocation with -c so no user or system Git config is touched, and it is a no-op off Windows. * Bound pending cancels, reach legacy DAC weights, and delete read-only checkouts for PR #8103 Three fixes, two of them on my own previous commit. Moving the owner cancel from evict-oldest to a one-slot overshoot also changed what the hardcoded reclaim_capacity=True on the pending non-owner branch did: it used to evict, so the table stayed at the cap, and it started overshooting instead. Start plus cancel could then be repeated to grow it without bound (1224 entries against a cap of 1024). That branch now takes the plain reservation, so the extra slot belongs to the owner of the active run alone. The DAC legacy fallback sat behind destination.parent.mkdir() and the install lock, both of which need a writable cache, so a read-only or full hub cache raised before weights we can already verify were ever looked at. Fall back at both points, and only to an artifact that passes the same size and sha256 check. Cross-OS CI then caught replacing a pinned checkout failing on windows-latest with WinError 5: Git marks .git/objects read-only and Windows will not delete a read-only file, so any repair or revision change died there. Clear the attribute and retry, only when the path is genuinely not writable, so an open handle still surfaces. * Delete the cached checkout the Windows-safe way in the migration test for PR #8103 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <moonshotaisubstack@gmail.com>
260 lines
9.5 KiB
Python
260 lines
9.5 KiB
Python
# SPDX-License-Identifier: AGPL-3.0-only
|
|
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
|
|
|
|
"""routes/inference.py::validate_model surfaces actionable RuntimeError/ValueError
|
|
messages (e.g. "llama-server binary not found - run setup.sh") instead of a blank
|
|
"Invalid model", while keeping unexpected exceptions generic so internals never
|
|
leak to the client.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
import sys
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
_BACKEND = Path(__file__).resolve().parents[1]
|
|
if str(_BACKEND) not in sys.path:
|
|
sys.path.insert(0, str(_BACKEND))
|
|
|
|
pytest.importorskip("fastapi")
|
|
|
|
from fastapi import HTTPException # noqa: E402
|
|
|
|
import routes.inference as inf # noqa: E402
|
|
from models.inference import ValidateModelRequest # noqa: E402
|
|
|
|
|
|
async def _inline_to_thread(function, /, *args, **kwargs):
|
|
return function(*args, **kwargs)
|
|
|
|
|
|
@pytest.fixture(autouse = True)
|
|
def _run_route_helpers_inline(monkeypatch):
|
|
monkeypatch.setattr(inf.asyncio, "to_thread", _inline_to_thread)
|
|
|
|
|
|
def _provoke(
|
|
monkeypatch,
|
|
exc: BaseException,
|
|
*,
|
|
native: bool = False,
|
|
) -> HTTPException:
|
|
"""Drive validate_model so from_identifier raises ``exc``; return the
|
|
HTTPException it converts that into."""
|
|
monkeypatch.setattr(
|
|
inf,
|
|
"_resolve_model_identifier_for_request",
|
|
lambda request, operation: ("org/repo", "org/repo", native),
|
|
)
|
|
|
|
def _raise(*_args, **_kwargs):
|
|
raise exc
|
|
|
|
monkeypatch.setattr(inf.ModelConfig, "from_identifier", staticmethod(_raise))
|
|
|
|
req = ValidateModelRequest(model_path = "org/repo")
|
|
with pytest.raises(HTTPException) as excinfo:
|
|
asyncio.run(inf.validate_model(req, current_subject = "tester"))
|
|
return excinfo.value
|
|
|
|
|
|
def test_runtime_error_surfaces_actionable_message(monkeypatch):
|
|
err = RuntimeError(
|
|
"llama-server binary not found - cannot load GGUF models. "
|
|
"Run setup.sh to build it, or set LLAMA_SERVER_PATH."
|
|
)
|
|
http = _provoke(monkeypatch, err)
|
|
assert http.status_code == 400
|
|
assert "llama-server binary not found" in http.detail
|
|
assert http.detail != "Invalid model"
|
|
|
|
|
|
def test_value_error_not_supported_is_wrapped(monkeypatch):
|
|
http = _provoke(monkeypatch, ValueError("architecture FooBar is not supported"))
|
|
assert http.status_code == 400
|
|
assert "not supported yet" in http.detail.lower()
|
|
# Original cause is preserved for context.
|
|
assert "FooBar" in http.detail
|
|
|
|
|
|
def test_unexpected_exception_stays_generic(monkeypatch):
|
|
# A non-user-facing exception type must NOT have its message surfaced.
|
|
http = _provoke(monkeypatch, KeyError("secret-internal-detail"))
|
|
assert http.status_code == 400
|
|
assert http.detail == "Invalid model"
|
|
assert "secret-internal-detail" not in http.detail
|
|
|
|
|
|
def test_empty_runtime_error_falls_back_to_generic(monkeypatch):
|
|
# A RuntimeError with no message should not produce an empty 400 detail.
|
|
http = _provoke(monkeypatch, RuntimeError(""))
|
|
assert http.status_code == 400
|
|
assert http.detail == "Invalid model"
|
|
|
|
|
|
def _drive_validate(monkeypatch, *, is_gguf: bool):
|
|
"""Run validate_model with both security helpers forced True; return the response."""
|
|
from types import SimpleNamespace
|
|
|
|
import utils.models.model_config as mc
|
|
|
|
monkeypatch.setattr(
|
|
inf,
|
|
"_resolve_model_identifier_for_request",
|
|
lambda request, operation: ("org/mixed-repo", "org/mixed-repo", False),
|
|
)
|
|
config = SimpleNamespace(
|
|
identifier = "org/mixed-repo",
|
|
display_name = "org/mixed-repo",
|
|
is_gguf = is_gguf,
|
|
is_lora = False,
|
|
is_vision = False,
|
|
gguf_file = None,
|
|
)
|
|
monkeypatch.setattr(inf.ModelConfig, "from_identifier", staticmethod(lambda **_kw: config))
|
|
# No LoRA base to resolve; keep it offline.
|
|
monkeypatch.setattr(mc, "get_base_model_from_lora_identifier", lambda *_a, **_k: None)
|
|
# Both gates WOULD flag this repo (mixed repo with auto_map + an unsafe pickle).
|
|
monkeypatch.setattr(inf, "_requires_trust_remote_code_for_model", lambda *_a, **_k: True)
|
|
monkeypatch.setattr(inf, "_requires_security_review_for_model", lambda *_a, **_k: True)
|
|
|
|
req = ValidateModelRequest(model_path = "org/mixed-repo")
|
|
return asyncio.run(inf.validate_model(req, current_subject = "tester"))
|
|
|
|
|
|
def test_selected_gguf_variant_skips_trc_and_security_review(monkeypatch):
|
|
# GGUF loads via llama.cpp: auto_map and root pickles are inert, so neither gate fires.
|
|
resp = _drive_validate(monkeypatch, is_gguf = True)
|
|
assert resp.is_gguf is True
|
|
assert resp.requires_trust_remote_code is False
|
|
assert resp.requires_security_review is False
|
|
|
|
|
|
def test_non_gguf_load_still_runs_trc_and_security_review(monkeypatch):
|
|
# Control: a Transformers (non-GGUF) load must still honor both gates.
|
|
resp = _drive_validate(monkeypatch, is_gguf = False)
|
|
assert resp.is_gguf is False
|
|
assert resp.requires_trust_remote_code is True
|
|
assert resp.requires_security_review is True
|
|
|
|
|
|
def test_resolve_loaded_trc_prefers_stored_value():
|
|
# A value stored at load time wins, so a status refresh does not re-derive it.
|
|
assert (
|
|
inf._resolve_loaded_trust_remote_code("org/m", {"requires_trust_remote_code": True}, {})
|
|
is True
|
|
)
|
|
assert (
|
|
inf._resolve_loaded_trust_remote_code(
|
|
"org/m", {"requires_trust_remote_code": False}, {"trust_remote_code": True}
|
|
)
|
|
is False
|
|
)
|
|
|
|
|
|
def test_resolve_loaded_trc_uses_runtime_and_yaml():
|
|
# No stored value: the trust_remote_code the load used, then the YAML default.
|
|
assert (
|
|
inf._resolve_loaded_trust_remote_code("org/m", {}, {}, trust_remote_code_used = True) is True
|
|
)
|
|
assert inf._resolve_loaded_trust_remote_code("org/m", {}, {"trust_remote_code": True}) is True
|
|
|
|
|
|
def test_resolve_loaded_trc_falls_back_to_raw_auto_map(monkeypatch):
|
|
# No stored value or runtime/YAML signal: fall back to the raw auto_map check.
|
|
monkeypatch.setattr(inf, "_requires_trust_remote_code_for_model", lambda *_a, **_k: True)
|
|
assert inf._resolve_loaded_trust_remote_code("org/custom", {}, {}) is True
|
|
monkeypatch.setattr(inf, "_requires_trust_remote_code_for_model", lambda *_a, **_k: False)
|
|
assert inf._resolve_loaded_trust_remote_code("org/plain", {}, {}) is False
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"model_identifier, expected_target",
|
|
[
|
|
("Spark-TTS-0.5B/LLM", "unsloth/Spark-TTS-0.5B"),
|
|
("unsloth/Spark-TTS-0.5B", "unsloth/Spark-TTS-0.5B"),
|
|
],
|
|
)
|
|
def test_requires_trc_checks_bicodec_load_subdirectory(
|
|
monkeypatch, model_identifier, expected_target
|
|
):
|
|
import utils.inference as inference_utils
|
|
import utils.models.model_config as model_config
|
|
import utils.security.consent as consent
|
|
|
|
calls = []
|
|
monkeypatch.setattr(inference_utils, "load_inference_config", lambda *_a, **_k: {})
|
|
monkeypatch.setattr(model_config, "detect_audio_type", lambda *_a, **_k: "bicodec")
|
|
monkeypatch.setattr(
|
|
model_config,
|
|
"load_model_defaults",
|
|
lambda *_a, **_k: {"audio_type": "bicodec"},
|
|
)
|
|
|
|
def config_has_auto_map(
|
|
target,
|
|
token,
|
|
*,
|
|
load_subdirs = (),
|
|
):
|
|
calls.append((target, token, load_subdirs))
|
|
return True
|
|
|
|
monkeypatch.setattr(consent, "_config_has_auto_map", config_has_auto_map)
|
|
|
|
assert inf._requires_trust_remote_code_for_model(model_identifier, "hf_test") is True
|
|
assert calls == [(expected_target, "hf_test", ("LLM",))]
|
|
|
|
|
|
def _drive_validate_lora(monkeypatch, *, adapter_needs_trc, base_needs_trc):
|
|
"""Run validate_model for a LoRA adapter whose base resolves, with per-target
|
|
trust_remote_code answers; return the response."""
|
|
from types import SimpleNamespace
|
|
|
|
import utils.models.model_config as mc
|
|
|
|
adapter, base = "org/lora-adapter", "org/base-model"
|
|
monkeypatch.setattr(
|
|
inf,
|
|
"_resolve_model_identifier_for_request",
|
|
lambda request, operation: (adapter, adapter, False),
|
|
)
|
|
config = SimpleNamespace(
|
|
identifier = adapter,
|
|
display_name = adapter,
|
|
is_gguf = False,
|
|
is_lora = True,
|
|
is_vision = False,
|
|
gguf_file = None,
|
|
)
|
|
monkeypatch.setattr(inf.ModelConfig, "from_identifier", staticmethod(lambda **_kw: config))
|
|
monkeypatch.setattr(mc, "get_base_model_from_lora_identifier", lambda *_a, **_k: base)
|
|
trc = {adapter: adapter_needs_trc, base: base_needs_trc}
|
|
monkeypatch.setattr(
|
|
inf,
|
|
"_requires_trust_remote_code_for_model",
|
|
lambda target, *_a, **_k: trc.get(target, False),
|
|
)
|
|
monkeypatch.setattr(inf, "_requires_security_review_for_model", lambda *_a, **_k: False)
|
|
req = ValidateModelRequest(model_path = adapter)
|
|
return asyncio.run(inf.validate_model(req, current_subject = "tester"))
|
|
|
|
|
|
def test_validate_lora_flags_trc_from_adapter_only(monkeypatch):
|
|
# Adapter ships auto_map, base does not: the requirement follows either repo.
|
|
resp = _drive_validate_lora(monkeypatch, adapter_needs_trc = True, base_needs_trc = False)
|
|
assert resp.requires_trust_remote_code is True
|
|
|
|
|
|
def test_validate_lora_flags_trc_from_base_only(monkeypatch):
|
|
# The classic case: the base ships custom code, the adapter does not.
|
|
resp = _drive_validate_lora(monkeypatch, adapter_needs_trc = False, base_needs_trc = True)
|
|
assert resp.requires_trust_remote_code is True
|
|
|
|
|
|
def test_validate_lora_clean_when_neither_needs_trc(monkeypatch):
|
|
resp = _drive_validate_lora(monkeypatch, adapter_needs_trc = False, base_needs_trc = False)
|
|
assert resp.requires_trust_remote_code is False
|