From e290ebd0acd768167776fb83399274a0a86fefd8 Mon Sep 17 00:00:00 2001 From: Souravrajvi0 Date: Mon, 27 Jul 2026 03:18:41 +0000 Subject: [PATCH] fix: address Codex review on offline GGUF tokenizer paths (#7481) - Only rewrite Hub repo ids to cached snapshot dirs when offline - Copy tokenizer.model from cache offline in preserve_sentencepiece - Do not cache negative offline tokenizer.model probe results - Add regression tests for all three review items --- .../test_offline_gguf_vlm_tokenizer_7481.py | 67 +++++++++++++++++-- unsloth/models/loader_utils.py | 28 +++++++- unsloth/save.py | 43 +++++++----- 3 files changed, 117 insertions(+), 21 deletions(-) diff --git a/tests/saving/test_offline_gguf_vlm_tokenizer_7481.py b/tests/saving/test_offline_gguf_vlm_tokenizer_7481.py index 0735856046..f25fd716ad 100644 --- a/tests/saving/test_offline_gguf_vlm_tokenizer_7481.py +++ b/tests/saving/test_offline_gguf_vlm_tokenizer_7481.py @@ -66,11 +66,25 @@ def _offline_env(monkeypatch, cache_root): monkeypatch.setenv("HF_HUB_CACHE", str(cache_root)) +def test_resolve_hub_repo_cached_file_finds_tokenizer_model(tmp_path, monkeypatch): + snap = _write_gemma4_cache(tmp_path) + (snap / "tokenizer.model").write_bytes(b"sp-model") + _offline_env(monkeypatch, tmp_path) + + got = L._resolve_hub_repo_cached_file( + _REPO, + "tokenizer.model", + local_files_only=True, + cache_dir=str(tmp_path), + ) + assert got == str(snap / "tokenizer.model") + + def test_resolve_hub_repo_local_dir_from_cached_snapshot(tmp_path, monkeypatch): snap = _write_gemma4_cache(tmp_path) _offline_env(monkeypatch, tmp_path) - got = L._resolve_hub_repo_local_dir(_REPO, local_files_only = True, cache_dir = str(tmp_path)) + got = L._resolve_hub_repo_local_dir(_REPO, local_files_only=True, cache_dir=str(tmp_path)) assert got == str(snap) @@ -78,11 +92,55 @@ def test_hub_repo_or_local_path_prefers_snapshot_over_repo_id(tmp_path, monkeypa snap = _write_gemma4_cache(tmp_path) _offline_env(monkeypatch, tmp_path) - got = L._hub_repo_or_local_path(_REPO, local_files_only = True, cache_dir = str(tmp_path)) + got = L._hub_repo_or_local_path(_REPO, local_files_only=True, cache_dir=str(tmp_path)) assert got == str(snap) assert got != _REPO +def test_hub_repo_or_local_path_keeps_repo_id_online(tmp_path, monkeypatch): + snap = _write_gemma4_cache(tmp_path) + monkeypatch.delenv("HF_HUB_OFFLINE", raising=False) + monkeypatch.delenv("TRANSFORMERS_OFFLINE", raising=False) + monkeypatch.setenv("HF_HUB_CACHE", str(tmp_path)) + + got = L._hub_repo_or_local_path(_REPO, local_files_only=False, cache_dir=str(tmp_path)) + assert got == _REPO + assert got != str(snap) + + +def test_has_tokenizer_model_offline_does_not_cache_negative(tmp_path, monkeypatch): + from unsloth.save import _TOKENIZER_MODEL_CACHE, _has_tokenizer_model + + snap = _write_gemma4_cache(tmp_path) + _offline_env(monkeypatch, tmp_path) + _TOKENIZER_MODEL_CACHE.clear() + + tok = SimpleNamespace(name_or_path=_REPO) + assert _has_tokenizer_model(tok, token=None) is False + assert _REPO not in _TOKENIZER_MODEL_CACHE + + (snap / "tokenizer.model").write_bytes(b"sp-model") + assert _has_tokenizer_model(tok, token=None) is True + + +def test_preserve_sentencepiece_offline_copies_cached_model(tmp_path, monkeypatch): + from unsloth.save import _TOKENIZER_MODEL_CACHE, _preserve_sentencepiece_tokenizer_assets + + snap = _write_gemma4_cache(tmp_path) + (snap / "tokenizer.model").write_bytes(b"cached-sp-model") + _offline_env(monkeypatch, tmp_path) + _TOKENIZER_MODEL_CACHE.clear() + + save_dir = tmp_path / "export" + save_dir.mkdir() + (save_dir / "tokenizer_config.json").write_text("{}", encoding="utf-8") + tok = SimpleNamespace(name_or_path=_REPO) + + _preserve_sentencepiece_tokenizer_assets(tok, str(save_dir)) + + assert (save_dir / "tokenizer.model").read_bytes() == b"cached-sp-model" + + def test_load_pretrained_tokenizer_fast_passes_snapshot_not_repo_id(tmp_path, monkeypatch): snap = _write_gemma4_cache(tmp_path) _offline_env(monkeypatch, tmp_path) @@ -111,12 +169,13 @@ def test_load_pretrained_tokenizer_fast_passes_snapshot_not_repo_id(tmp_path, mo def test_has_tokenizer_model_offline_skips_model_info(tmp_path, monkeypatch): - from unsloth.save import _has_tokenizer_model + from unsloth.save import _TOKENIZER_MODEL_CACHE, _has_tokenizer_model _write_gemma4_cache(tmp_path) _offline_env(monkeypatch, tmp_path) + _TOKENIZER_MODEL_CACHE.clear() - tok = SimpleNamespace(name_or_path = _REPO, tokenizer = None) + tok = SimpleNamespace(name_or_path = _REPO) with patch("huggingface_hub.HfApi.model_info") as model_info: model_info.side_effect = AssertionError("model_info must not run offline") diff --git a/unsloth/models/loader_utils.py b/unsloth/models/loader_utils.py index 3f7eca8fb3..97fc96e265 100644 --- a/unsloth/models/loader_utils.py +++ b/unsloth/models/loader_utils.py @@ -1170,6 +1170,28 @@ def _resolve_hub_repo_local_dir( return None +def _resolve_hub_repo_cached_file( + repo_id, + filename, + *, + token = None, + cache_dir = None, + local_files_only = False, +): + """Return a cached file path under a Hub snapshot, or None if absent.""" + local_dir = _resolve_hub_repo_local_dir( + repo_id, + token = token, + cache_dir = cache_dir, + local_files_only = local_files_only, + filenames = (filename,), + ) + if local_dir is None: + return None + path = os.path.join(local_dir, filename) + return path if os.path.isfile(path) else None + + def _hub_repo_or_local_path( repo_id, *, @@ -1179,14 +1201,16 @@ def _hub_repo_or_local_path( filenames = None, ): """Prefer a cached snapshot path over a Hub repo id when offline or ``local_files_only``.""" + if isinstance(repo_id, str) and os.path.isdir(repo_id): + return repo_id lfo = bool(local_files_only) or _env_says_offline() - if not lfo and os.path.isdir(repo_id): + if not lfo: return repo_id local_dir = _resolve_hub_repo_local_dir( repo_id, token = token, cache_dir = cache_dir, - local_files_only = lfo, + local_files_only = True, filenames = filenames or ( "tokenizer_config.json", diff --git a/unsloth/save.py b/unsloth/save.py index a778650fdb..0785188a14 100644 --- a/unsloth/save.py +++ b/unsloth/save.py @@ -52,7 +52,7 @@ import traceback import psutil import re from transformers.models.llama.modeling_llama import logger -from .models.loader_utils import get_model_name, _env_says_offline, _resolve_hub_repo_local_dir +from .models.loader_utils import get_model_name, _env_says_offline, _resolve_hub_repo_cached_file from .models._utils import _convert_torchao_model from .ollama_template_mappers import OLLAMA_TEMPLATES, MODEL_TO_OLLAMA_TEMPLATE_MAPPER from transformers import ProcessorMixin, PreTrainedTokenizerBase @@ -448,17 +448,16 @@ def _has_tokenizer_model(tokenizer, token = None): # Offline: probe the local cache instead of model_info (issue #7481). if _env_says_offline(): - local_dir = _resolve_hub_repo_local_dir( + cached_path = _resolve_hub_repo_cached_file( source, + "tokenizer.model", token = token, local_files_only = True, - filenames = ("tokenizer.model", "tokenizer.json", "tokenizer_config.json"), cache_dir = os.environ.get("HF_HUB_CACHE"), ) - if local_dir is not None: - has_tokenizer_model = os.path.isfile(os.path.join(local_dir, "tokenizer.model")) - _TOKENIZER_MODEL_CACHE[source] = has_tokenizer_model - return has_tokenizer_model + if cached_path is not None: + _TOKENIZER_MODEL_CACHE[source] = True + return True return False try: @@ -520,15 +519,29 @@ def _preserve_sentencepiece_tokenizer_assets( if os.path.isfile(local_path): downloaded_path = local_path else: - from huggingface_hub import hf_hub_download - try: - downloaded_path = hf_hub_download( - repo_id = source, - filename = "tokenizer.model", + cached_path = None + if _env_says_offline(): + cached_path = _resolve_hub_repo_cached_file( + source, + "tokenizer.model", token = token, + local_files_only = True, + cache_dir = os.environ.get("HF_HUB_CACHE"), ) - except Exception: - downloaded_path = None + if cached_path is not None: + downloaded_path = cached_path + else: + from huggingface_hub import hf_hub_download + try: + downloaded_path = hf_hub_download( + repo_id = source, + filename = "tokenizer.model", + token = token, + local_files_only = _env_says_offline(), + cache_dir = os.environ.get("HF_HUB_CACHE"), + ) + except Exception: + downloaded_path = None if not os.path.isfile(tokenizer_model) and downloaded_path is not None: shutil.copy2(downloaded_path, tokenizer_model) @@ -3808,7 +3821,7 @@ def unsloth_convert_lora_to_ggml_and_save_locally( return _unsloth_save_lora_gguf(self, tokenizer, save_directory, outtype = outtype) -from .models.loader_utils import get_model_name, _env_says_offline, _resolve_hub_repo_local_dir +from .models.loader_utils import get_model_name, _env_says_offline, _resolve_hub_repo_cached_file from unsloth_zoo.saving_utils import ( merge_and_overwrite_lora, prepare_saving,