From a25c064dea310618101d7a445d1dc5528c3b602e Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Sun, 5 Apr 2026 02:34:42 +0000 Subject: [PATCH 1/4] Add tests for is_vision_model() caching behaviour --- studio/backend/tests/test_vision_cache.py | 252 ++++++++++++++++++++++ 1 file changed, 252 insertions(+) create mode 100644 studio/backend/tests/test_vision_cache.py diff --git a/studio/backend/tests/test_vision_cache.py b/studio/backend/tests/test_vision_cache.py new file mode 100644 index 0000000000..6181c77dd3 --- /dev/null +++ b/studio/backend/tests/test_vision_cache.py @@ -0,0 +1,252 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +"""Tests for is_vision_model() caching behaviour. + +The vision detection cache (``_vision_detection_cache``) mirrors the existing +``_audio_detection_cache`` pattern used by ``detect_audio_type()``. These +tests verify that: + +* Repeated calls for the same model hit the cache (no redundant work). +* Different models each trigger their own detection. +* Both True and False results are cached. +* The subprocess path (transformers 5.x models) is also cached. +* Exceptions that fall back to False are cached. +""" + +import sys +import types as _types +from pathlib import Path +from unittest.mock import patch, MagicMock + +import pytest + +# --------------------------------------------------------------------------- +# sys.path + logger stub — same pattern as the rest of the test suite +# --------------------------------------------------------------------------- +_BACKEND_DIR = str(Path(__file__).resolve().parent.parent) +if _BACKEND_DIR not in sys.path: + sys.path.insert(0, _BACKEND_DIR) + +_loggers_stub = _types.ModuleType("loggers") +_loggers_stub.get_logger = lambda name: __import__("logging").getLogger(name) +sys.modules.setdefault("loggers", _loggers_stub) + +from utils.models.model_config import ( + is_vision_model, + _is_vision_model_uncached, + _vision_detection_cache, +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +@pytest.fixture(autouse = True) +def _clear_vision_cache(): + """Ensure every test starts with a fresh cache.""" + _vision_detection_cache.clear() + yield + _vision_detection_cache.clear() + + +def _make_config(**attrs): + """Return a lightweight mock config with the given attributes.""" + cfg = MagicMock() + # Remove all default MagicMock attributes so hasattr checks are explicit + cfg.configure_mock(**attrs) + # Only expose the attrs we explicitly set + real_attrs = set(attrs.keys()) + original_hasattr = hasattr + + def _controlled_hasattr(obj, name): + if obj is cfg: + return name in real_attrs + return original_hasattr(obj, name) + + return cfg, _controlled_hasattr + + +# --------------------------------------------------------------------------- +# Cache hit / miss tests +# --------------------------------------------------------------------------- + + +class TestVisionCacheHitMiss: + """Verify the cache prevents redundant detection calls.""" + + @patch("utils.models.model_config._is_vision_model_uncached", return_value = True) + def test_second_call_uses_cache(self, mock_uncached): + """Calling is_vision_model() twice for the same model should invoke + the uncached function only once.""" + assert is_vision_model("org/my-vlm") is True + assert is_vision_model("org/my-vlm") is True + mock_uncached.assert_called_once_with("org/my-vlm", None) + + @patch("utils.models.model_config._is_vision_model_uncached", return_value = False) + def test_different_models_each_detected(self, mock_uncached): + """Different model names should each trigger detection.""" + is_vision_model("model-a") + is_vision_model("model-b") + assert mock_uncached.call_count == 2 + + @patch("utils.models.model_config._is_vision_model_uncached", return_value = True) + def test_cache_returns_correct_value(self, mock_uncached): + """The cached value must match what _is_vision_model_uncached returned.""" + first = is_vision_model("org/vlm") + second = is_vision_model("org/vlm") + assert first is True + assert second is True + + +class TestVisionCacheStoresFalse: + """Non-VLM results (False) must also be cached to avoid re-detection.""" + + @patch("utils.models.model_config._is_vision_model_uncached", return_value = False) + def test_false_result_cached(self, mock_uncached): + assert is_vision_model("org/text-only") is False + assert is_vision_model("org/text-only") is False + mock_uncached.assert_called_once() + assert _vision_detection_cache[("org/text-only", None)] is False + + +# --------------------------------------------------------------------------- +# Subprocess path (transformers 5.x) caching +# --------------------------------------------------------------------------- + + +class TestVisionCacheSubprocessPath: + """Models needing transformers 5.x go through _is_vision_model_subprocess. + The cache should prevent the subprocess from being spawned more than once + per model per process.""" + + @patch("utils.models.model_config._is_vision_model_subprocess", return_value = True) + @patch("utils.transformers_version.needs_transformers_5", return_value = True) + def test_subprocess_called_once_with_cache(self, mock_needs_t5, mock_subprocess): + """Subprocess should only fire on the first call; second is cached.""" + # First call: goes through uncached → subprocess + assert is_vision_model("unsloth/Qwen3.5-2B") is True + # Second call: cache hit, no subprocess + assert is_vision_model("unsloth/Qwen3.5-2B") is True + + mock_subprocess.assert_called_once() + assert _vision_detection_cache[("unsloth/Qwen3.5-2B", None)] is True + + +# --------------------------------------------------------------------------- +# Exception handling — cache the False fallback +# --------------------------------------------------------------------------- + + +class TestVisionCacheOnException: + """When detection raises an exception, the function returns False. + That False must be cached so subsequent calls don't retry and fail again.""" + + @patch( + "utils.models.model_config._is_vision_model_uncached", + side_effect = [False], + ) + def test_exception_result_cached(self, mock_uncached): + """After an exception-triggered False, the cache should serve False.""" + # The uncached function returns False (simulating the except branch) + assert is_vision_model("broken/model") is False + # Second call should not invoke uncached again + assert is_vision_model("broken/model") is False + mock_uncached.assert_called_once() + + +# --------------------------------------------------------------------------- +# Direct detection path (non-transformers-5 models) caching +# --------------------------------------------------------------------------- + + +class TestVisionCacheDirectPath: + """For models that do NOT need transformers 5.x, the detection goes through + load_model_config directly. The cache must work the same way.""" + + @patch("utils.transformers_version.needs_transformers_5", return_value = False) + @patch("utils.models.model_config.load_model_config") + def test_direct_vlm_detection_cached(self, mock_load_config, mock_needs_t5): + """A standard VLM detected via vision_config should be cached.""" + cfg = MagicMock() + cfg.model_type = "gemma3" + cfg.architectures = ["Gemma3ForConditionalGeneration"] + mock_load_config.return_value = cfg + + assert is_vision_model("google/gemma-3-4b-it") is True + assert is_vision_model("google/gemma-3-4b-it") is True + # load_model_config should only be called once + mock_load_config.assert_called_once() + + @patch("utils.transformers_version.needs_transformers_5", return_value = False) + @patch("utils.models.model_config.load_model_config") + def test_direct_non_vlm_detection_cached(self, mock_load_config, mock_needs_t5): + """A standard text model (no VLM indicators) should cache False.""" + cfg = MagicMock(spec = []) # spec=[] means no attributes at all + cfg.model_type = "llama" + cfg.architectures = ["LlamaForCausalLM"] + mock_load_config.return_value = cfg + + # LlamaForCausalLM doesn't end with VLM suffixes, no vision_config, etc. + assert is_vision_model("meta-llama/Llama-3-8B") is False + assert is_vision_model("meta-llama/Llama-3-8B") is False + mock_load_config.assert_called_once() + + @patch("utils.transformers_version.needs_transformers_5", return_value = False) + @patch("utils.models.model_config.load_model_config") + def test_vision_config_attr_detected_and_cached( + self, mock_load_config, mock_needs_t5 + ): + """Models with vision_config (LLaVA, Qwen2-VL, etc.) should be cached as True.""" + cfg = MagicMock() + cfg.model_type = "qwen2_vl" + cfg.architectures = ["Qwen2VLForCausalLM"] # Doesn't match VLM suffixes + cfg.vision_config = {"hidden_size": 1024} + mock_load_config.return_value = cfg + + assert is_vision_model("Qwen/Qwen2-VL-7B") is True + assert is_vision_model("Qwen/Qwen2-VL-7B") is True + mock_load_config.assert_called_once() + + @patch("utils.transformers_version.needs_transformers_5", return_value = False) + @patch("utils.models.model_config.load_model_config") + def test_audio_model_excluded_and_cached(self, mock_load_config, mock_needs_t5): + """Audio-only models (csm, whisper) with ForConditionalGeneration + should be excluded from VLM detection and cached as False.""" + cfg = MagicMock() + cfg.model_type = "whisper" + cfg.architectures = ["WhisperForConditionalGeneration"] + mock_load_config.return_value = cfg + + assert is_vision_model("openai/whisper-large-v3") is False + assert is_vision_model("openai/whisper-large-v3") is False + mock_load_config.assert_called_once() + + +# --------------------------------------------------------------------------- +# hf_token handling +# --------------------------------------------------------------------------- + + +class TestVisionCacheTokenHandling: + """The cache is keyed on (model_name, hf_token). + Different tokens for the same model should trigger separate detections + to handle gated models correctly.""" + + @patch("utils.models.model_config._is_vision_model_uncached", return_value = True) + def test_different_tokens_trigger_new_detection(self, mock_uncached): + """Calls with different tokens should trigger separate detections to + handle gated models correctly (e.g. unauthenticated probe → False, + then authenticated call should re-check).""" + assert is_vision_model("gated/model", hf_token = "token-a") is True + assert is_vision_model("gated/model", hf_token = "token-b") is True + assert mock_uncached.call_count == 2 + + @patch("utils.models.model_config._is_vision_model_uncached", return_value = True) + def test_same_token_uses_cache(self, mock_uncached): + """Repeated calls with identical model + token should hit cache.""" + assert is_vision_model("gated/model", hf_token = "token-a") is True + assert is_vision_model("gated/model", hf_token = "token-a") is True + mock_uncached.assert_called_once() From 14d673175ba1dff4293281095af6a56bb65454f2 Mon Sep 17 00:00:00 2001 From: Roland Tannous Date: Sun, 5 Apr 2026 04:05:33 +0000 Subject: [PATCH 2/4] Fix review feedback: remove dead helper, fix exception test - Remove unused _make_config() helper function (dead code) - Fix test_exception_result_cached to actually exercise the exception path by mocking load_model_config to raise OSError instead of using side_effect=[False] which only tested normal False returns --- studio/backend/tests/test_vision_cache.py | 41 +++++++---------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/studio/backend/tests/test_vision_cache.py b/studio/backend/tests/test_vision_cache.py index 6181c77dd3..e2df254e29 100644 --- a/studio/backend/tests/test_vision_cache.py +++ b/studio/backend/tests/test_vision_cache.py @@ -52,23 +52,6 @@ def _clear_vision_cache(): _vision_detection_cache.clear() -def _make_config(**attrs): - """Return a lightweight mock config with the given attributes.""" - cfg = MagicMock() - # Remove all default MagicMock attributes so hasattr checks are explicit - cfg.configure_mock(**attrs) - # Only expose the attrs we explicitly set - real_attrs = set(attrs.keys()) - original_hasattr = hasattr - - def _controlled_hasattr(obj, name): - if obj is cfg: - return name in real_attrs - return original_hasattr(obj, name) - - return cfg, _controlled_hasattr - - # --------------------------------------------------------------------------- # Cache hit / miss tests # --------------------------------------------------------------------------- @@ -141,20 +124,20 @@ def test_subprocess_called_once_with_cache(self, mock_needs_t5, mock_subprocess) class TestVisionCacheOnException: - """When detection raises an exception, the function returns False. - That False must be cached so subsequent calls don't retry and fail again.""" - - @patch( - "utils.models.model_config._is_vision_model_uncached", - side_effect = [False], - ) - def test_exception_result_cached(self, mock_uncached): - """After an exception-triggered False, the cache should serve False.""" - # The uncached function returns False (simulating the except branch) + """When detection raises an exception, _is_vision_model_uncached catches + it and returns False. That False must be cached so subsequent calls don't + retry and fail again.""" + + @patch("utils.models.model_config.load_model_config", side_effect = OSError("network down")) + @patch("utils.transformers_version.needs_transformers_5", return_value = False) + def test_exception_result_cached(self, mock_needs_t5, mock_load_config): + """A real exception inside _is_vision_model_uncached should be caught, + return False, and that False should be cached for subsequent calls.""" + # First call: load_model_config raises → except branch → False assert is_vision_model("broken/model") is False - # Second call should not invoke uncached again + # Second call: cache hit, load_model_config not called again assert is_vision_model("broken/model") is False - mock_uncached.assert_called_once() + mock_load_config.assert_called_once() # --------------------------------------------------------------------------- From 44f04582deda4a453e60881d33afc6704a47cd3d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 5 Apr 2026 04:05:54 +0000 Subject: [PATCH 3/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- studio/backend/tests/test_vision_cache.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/studio/backend/tests/test_vision_cache.py b/studio/backend/tests/test_vision_cache.py index e2df254e29..762f54836e 100644 --- a/studio/backend/tests/test_vision_cache.py +++ b/studio/backend/tests/test_vision_cache.py @@ -128,7 +128,10 @@ class TestVisionCacheOnException: it and returns False. That False must be cached so subsequent calls don't retry and fail again.""" - @patch("utils.models.model_config.load_model_config", side_effect = OSError("network down")) + @patch( + "utils.models.model_config.load_model_config", + side_effect = OSError("network down"), + ) @patch("utils.transformers_version.needs_transformers_5", return_value = False) def test_exception_result_cached(self, mock_needs_t5, mock_load_config): """A real exception inside _is_vision_model_uncached should be caught, From 203d67811bdff19383e7249964f64e4383aca6cd Mon Sep 17 00:00:00 2001 From: Roland Tannous Date: Sun, 5 Apr 2026 04:20:58 +0000 Subject: [PATCH 4/4] Use strict mock specs so tests exercise intended detection paths Use MagicMock(spec=[]) for all config mocks so hasattr() only returns True for explicitly set attributes. Without this, MagicMock defaults make all hasattr checks truthy, allowing tests to pass via unintended detection paths (e.g. img_processor instead of vision_config). --- studio/backend/tests/test_vision_cache.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/studio/backend/tests/test_vision_cache.py b/studio/backend/tests/test_vision_cache.py index 762f54836e..fae1e95311 100644 --- a/studio/backend/tests/test_vision_cache.py +++ b/studio/backend/tests/test_vision_cache.py @@ -155,8 +155,8 @@ class TestVisionCacheDirectPath: @patch("utils.transformers_version.needs_transformers_5", return_value = False) @patch("utils.models.model_config.load_model_config") def test_direct_vlm_detection_cached(self, mock_load_config, mock_needs_t5): - """A standard VLM detected via vision_config should be cached.""" - cfg = MagicMock() + """A standard VLM detected via architecture suffix should be cached.""" + cfg = MagicMock(spec = []) # strict: only explicitly set attrs exist cfg.model_type = "gemma3" cfg.architectures = ["Gemma3ForConditionalGeneration"] mock_load_config.return_value = cfg @@ -186,7 +186,7 @@ def test_vision_config_attr_detected_and_cached( self, mock_load_config, mock_needs_t5 ): """Models with vision_config (LLaVA, Qwen2-VL, etc.) should be cached as True.""" - cfg = MagicMock() + cfg = MagicMock(spec = []) # strict: only explicitly set attrs exist cfg.model_type = "qwen2_vl" cfg.architectures = ["Qwen2VLForCausalLM"] # Doesn't match VLM suffixes cfg.vision_config = {"hidden_size": 1024} @@ -201,7 +201,7 @@ def test_vision_config_attr_detected_and_cached( def test_audio_model_excluded_and_cached(self, mock_load_config, mock_needs_t5): """Audio-only models (csm, whisper) with ForConditionalGeneration should be excluded from VLM detection and cached as False.""" - cfg = MagicMock() + cfg = MagicMock(spec = []) # strict: only explicitly set attrs exist cfg.model_type = "whisper" cfg.architectures = ["WhisperForConditionalGeneration"] mock_load_config.return_value = cfg