From ca476c41f86b661f454ac687de56db3401be3f4a Mon Sep 17 00:00:00 2001 From: Datta Nimmaturi Date: Mon, 8 Jun 2026 20:20:08 +0530 Subject: [PATCH] Merge studio_gemma4_vlm CI fixes Merged latest main, resolved model_config.py conflict, removed redundant VLM checks --- studio/backend/tests/test_vision_cache.py | 202 ++++++++++++++++++++ studio/backend/utils/models/model_config.py | 144 +++++++++----- 2 files changed, 296 insertions(+), 50 deletions(-) diff --git a/studio/backend/tests/test_vision_cache.py b/studio/backend/tests/test_vision_cache.py index 2af64dac91..78f88fd7ea 100644 --- a/studio/backend/tests/test_vision_cache.py +++ b/studio/backend/tests/test_vision_cache.py @@ -117,6 +117,18 @@ class TestVisionCacheSubprocessPath: mock_subprocess.assert_called_once() assert _vision_detection_cache[("unsloth/Qwen3.5-2B", None)] is True + @patch("utils.models.model_config._raw_config_has_vision_config", return_value = True) + @patch("utils.models.model_config._is_vision_model_subprocess", return_value = None) + @patch("utils.transformers_version.needs_transformers_5", return_value = True) + def test_subprocess_none_falls_back_to_raw_vision_config( + self, mock_needs_t5, mock_subprocess, mock_raw_config + ): + assert is_vision_model("unsloth/gemma-4-E4B-it") is True + assert is_vision_model("unsloth/gemma-4-E4B-it") is True + + mock_subprocess.assert_called_once() + mock_raw_config.assert_called_once_with("unsloth/gemma-4-E4B-it", hf_token = None) + # --------------------------------------------------------------------------- # Exception handling — cache the False fallback @@ -221,6 +233,42 @@ class TestVisionCacheDirectPath: 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_gemma4_model_type_detected_and_cached(self, mock_load_config, mock_needs_t5): + cfg = MagicMock(spec = []) + cfg.model_type = "gemma4" + cfg.architectures = ["Gemma4ForConditionalGeneration"] + mock_load_config.return_value = cfg + + assert is_vision_model("google/gemma-4-E4B-it") is True + assert is_vision_model("google/gemma-4-E4B-it") 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_gemma4_audio_subconfig_not_detected_as_vision(self, mock_load_config, mock_needs_t5): + cfg = MagicMock(spec = []) + cfg.model_type = "gemma4_audio" + cfg.architectures = ["Gemma4AudioModel"] + mock_load_config.return_value = cfg + + assert is_vision_model("local/gemma4-audio-encoder") is False + assert is_vision_model("local/gemma4-audio-encoder") 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_gemma4_text_subconfig_not_detected_as_vision(self, mock_load_config, mock_needs_t5): + cfg = MagicMock(spec = []) + cfg.model_type = "gemma4_text" + cfg.architectures = ["Gemma4ForCausalLM"] + mock_load_config.return_value = cfg + + assert is_vision_model("local/gemma-4-text") is False + assert is_vision_model("local/gemma-4-text") 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_audio_model_excluded_and_cached(self, mock_load_config, mock_needs_t5): @@ -261,3 +309,157 @@ class TestVisionCacheTokenHandling: 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() + + +# --------------------------------------------------------------------------- +# Direct unit tests for _raw_config_has_vision_config +# --------------------------------------------------------------------------- + + +import json as _json + +from utils.models.model_config import ( + _AUDIO_ONLY_MODEL_TYPES, + _VISION_CHECK_INLINE_HELPERS, + _VISION_CHECK_SCRIPT, + _is_vlm, + _raw_config_has_vision_config, +) + + +def _write_config(tmp_path, config): + (tmp_path / "config.json").write_text(_json.dumps(config)) + return tmp_path + + +class TestRawConfigVlmDetection: + """Direct coverage of _raw_config_has_vision_config across the same + indicator set used by _is_vlm. The cache integration tests above mock + this function; these exercise its real implementation.""" + + def test_truthy_vision_config(self, tmp_path): + p = _write_config(tmp_path, {"vision_config": {"hidden_size": 1024}}) + assert _raw_config_has_vision_config(str(p)) is True + + def test_empty_vision_config_key(self, tmp_path): + p = _write_config(tmp_path, {"vision_config": {}}) + assert _raw_config_has_vision_config(str(p)) is True + + def test_arch_suffix_detection(self, tmp_path): + p = _write_config( + tmp_path, + { + "architectures": ["Gemma4ForConditionalGeneration"], + "model_type": "gemma4", + }, + ) + assert _raw_config_has_vision_config(str(p)) is True + + def test_img_processor_key(self, tmp_path): + p = _write_config(tmp_path, {"img_processor": {"image_size": 336}}) + assert _raw_config_has_vision_config(str(p)) is True + + def test_image_token_index_key(self, tmp_path): + p = _write_config(tmp_path, {"image_token_index": 32000}) + assert _raw_config_has_vision_config(str(p)) is True + + def test_known_vlm_model_type(self, tmp_path): + p = _write_config(tmp_path, {"model_type": "gemma4"}) + assert _raw_config_has_vision_config(str(p)) is True + + def test_plain_text_model_returns_false(self, tmp_path): + p = _write_config( + tmp_path, + {"model_type": "llama", "architectures": ["LlamaForCausalLM"]}, + ) + assert _raw_config_has_vision_config(str(p)) is False + + def test_missing_config_returns_none(self, tmp_path): + assert _raw_config_has_vision_config(str(tmp_path)) is None + + +# --------------------------------------------------------------------------- +# Self-contained subprocess script (no parent backend imports) +# --------------------------------------------------------------------------- + + +class TestSubprocessScript: + def test_does_not_import_parent_module(self): + assert "from utils.models.model_config" not in _VISION_CHECK_SCRIPT + + def test_inline_is_vlm_executes_correctly(self): + ns: dict = {} + exec(_VISION_CHECK_INLINE_HELPERS, ns) + inline_is_vlm = ns["_is_vlm"] + + class _C: + def __init__(self, **kw): + for k, v in kw.items(): + setattr(self, k, v) + + assert ( + inline_is_vlm( + _C( + model_type = "gemma4", + architectures = ["Gemma4ForConditionalGeneration"], + ) + ) + is True + ) + assert ( + inline_is_vlm(_C(model_type = "gemma4_text", architectures = ["Gemma4ForCausalLM"])) + is False + ) + assert inline_is_vlm(_C(model_type = "llama", architectures = ["LlamaForCausalLM"])) is False + + +# --------------------------------------------------------------------------- +# Audio-only model exclusion must apply across every detection path +# --------------------------------------------------------------------------- + + +class TestVlmAudioExclusion: + """The {csm, whisper} guard previously lived only in the direct caller + branch. These tests assert it now applies inside _is_vlm, the raw + fallback, and the inlined subprocess helper too.""" + + def test_audio_only_set_canonical(self): + assert _AUDIO_ONLY_MODEL_TYPES == {"csm", "whisper"} + + def test_is_vlm_excludes_whisper(self): + cfg = MagicMock(spec = []) + cfg.model_type = "whisper" + cfg.architectures = ["WhisperForConditionalGeneration"] + assert _is_vlm(cfg) is False + + def test_raw_fallback_excludes_whisper(self, tmp_path): + p = _write_config( + tmp_path, + { + "architectures": ["WhisperForConditionalGeneration"], + "model_type": "whisper", + }, + ) + assert _raw_config_has_vision_config(str(p)) is False + + def test_inline_subprocess_helper_excludes_whisper(self): + ns: dict = {} + exec(_VISION_CHECK_INLINE_HELPERS, ns) + cfg = MagicMock(spec = []) + cfg.model_type = "whisper" + cfg.architectures = ["WhisperForConditionalGeneration"] + assert ns["_is_vlm"](cfg) is False + + @patch("utils.models.model_config._is_vision_model_subprocess", return_value = None) + @patch("utils.transformers_version.needs_transformers_5", return_value = True) + def test_t5_subprocess_none_falls_back_through_raw_for_whisper( + self, mock_needs_t5, mock_subprocess, tmp_path + ): + _write_config( + tmp_path, + { + "architectures": ["WhisperForConditionalGeneration"], + "model_type": "whisper", + }, + ) + assert is_vision_model(str(tmp_path)) is False diff --git a/studio/backend/utils/models/model_config.py b/studio/backend/utils/models/model_config.py index 92960485a4..2947bab3d3 100644 --- a/studio/backend/utils/models/model_config.py +++ b/studio/backend/utils/models/model_config.py @@ -507,8 +507,13 @@ _VLM_MODEL_TYPES = { "internvl_chat", "cogvlm2", "minicpmv", + "gemma4", } +# Audio-only models that share the ForConditionalGeneration suffix +# (e.g. CsmForConditionalGeneration, WhisperForConditionalGeneration). +_AUDIO_ONLY_MODEL_TYPES = {"csm", "whisper"} + # Pre-computed .venv_t5 paths and backend dir for subprocess version switching. # Vision check uses 5.5.0 (newest, recognizes all architectures). from utils.paths.storage_roots import studio_root as _studio_root # noqa: E402 @@ -516,9 +521,77 @@ from utils.paths.storage_roots import studio_root as _studio_root # noqa: E402 _VENV_T5_DIR = str(_studio_root() / ".venv_t5_550") _BACKEND_DIR = str(Path(__file__).resolve().parent.parent.parent) + +def _is_vlm(config) -> bool: + architectures = getattr(config, "architectures", None) or [] + model_type = getattr(config, "model_type", None) + if model_type in _AUDIO_ONLY_MODEL_TYPES: + return False + return ( + any(x.endswith(_VLM_ARCH_SUFFIXES) for x in architectures) + or hasattr(config, "vision_config") + or hasattr(config, "img_processor") + or hasattr(config, "image_token_index") + or model_type in _VLM_MODEL_TYPES + ) + + +def _raw_config_has_vision_config( + model_name: str, hf_token: Optional[str] = None +) -> Optional[bool]: + try: + if is_local_path(model_name): + config_path = Path(normalize_path(model_name)).expanduser() / "config.json" + else: + from huggingface_hub import hf_hub_download + config_path = Path( + hf_hub_download( + repo_id = model_name, + filename = "config.json", + token = hf_token, + ) + ) + config = json.loads(config_path.read_text()) + architectures = config.get("architectures") or [] + model_type = config.get("model_type") + if model_type in _AUDIO_ONLY_MODEL_TYPES: + return False + return ( + any(isinstance(x, str) and x.endswith(_VLM_ARCH_SUFFIXES) for x in architectures) + or "vision_config" in config + or "img_processor" in config + or "image_token_index" in config + or model_type in _VLM_MODEL_TYPES + ) + except Exception as exc: + logger.warning("Could not read config.json for '%s': %s", model_name, exc) + return None + + +# why: inline _is_vlm and constants are prepended so the subprocess stays +# self-contained and does not import the parent backend module graph. +_VISION_CHECK_INLINE_HELPERS = ( + "_VLM_ARCH_SUFFIXES = " + repr(_VLM_ARCH_SUFFIXES) + "\n" + "_VLM_MODEL_TYPES = " + repr(_VLM_MODEL_TYPES) + "\n" + "_AUDIO_ONLY_MODEL_TYPES = " + repr(_AUDIO_ONLY_MODEL_TYPES) + "\n" + "def _is_vlm(config):\n" + " architectures = getattr(config, 'architectures', None) or []\n" + " model_type = getattr(config, 'model_type', None)\n" + " if model_type in _AUDIO_ONLY_MODEL_TYPES:\n" + " return False\n" + " return (\n" + " any(x.endswith(_VLM_ARCH_SUFFIXES) for x in architectures)\n" + " or hasattr(config, 'vision_config')\n" + " or hasattr(config, 'img_processor')\n" + " or hasattr(config, 'image_token_index')\n" + " or model_type in _VLM_MODEL_TYPES\n" + " )\n" +) + # Inline script executed in a subprocess with transformers 5.x activated. # Receives model_name and token via argv, prints JSON result to stdout. -_VISION_CHECK_SCRIPT = r""" +_VISION_CHECK_SCRIPT = ( + r""" import sys, os, json os.environ["TOKENIZERS_PARALLELISM"] = "false" @@ -532,32 +605,20 @@ sys.path.insert(0, venv_t5) if backend_dir not in sys.path: sys.path.insert(0, backend_dir) +""" + + _VISION_CHECK_INLINE_HELPERS + + r""" try: from transformers import AutoConfig + kwargs = {"trust_remote_code": True} if token: kwargs["token"] = token config = AutoConfig.from_pretrained(model_name, **kwargs) - is_vlm = False - if hasattr(config, "architectures"): - is_vlm = any( - x.endswith(("ForConditionalGeneration", "ForVisionText2Text")) - for x in config.architectures - ) - if not is_vlm and hasattr(config, "vision_config"): - is_vlm = True - if not is_vlm and hasattr(config, "img_processor"): - is_vlm = True - if not is_vlm and hasattr(config, "image_token_index"): - is_vlm = True - if not is_vlm and hasattr(config, "model_type"): - vlm_types = {"phi3_v","llava","llava_next","llava_onevision", - "internvl_chat","cogvlm2","minicpmv"} - if config.model_type in vlm_types: - is_vlm = True + is_vlm = _is_vlm(config) - model_type = getattr(config, "model_type", "unknown") + model_type = getattr(config, "model_type", None) archs = getattr(config, "architectures", []) print(json.dumps({"is_vision": is_vlm, "model_type": model_type, "architectures": archs})) @@ -565,6 +626,7 @@ except Exception as exc: print(json.dumps({"error": str(exc)})) sys.exit(1) """ +) def _is_vision_model_subprocess(model_name: str, hf_token: Optional[str] = None) -> Optional[bool]: @@ -728,48 +790,30 @@ def _is_vision_model_uncached(model_name: str, hf_token: Optional[str] = None) - "Model '%s' needs transformers 5.x -- checking vision via subprocess", model_name, ) - return _is_vision_model_subprocess(model_name, hf_token = hf_token) + result = _is_vision_model_subprocess(model_name, hf_token = hf_token) + if result is not None: + return result + return _raw_config_has_vision_config(model_name, hf_token = hf_token) try: config = load_model_config(model_name, use_auth = True, token = hf_token) # Exclude audio-only models that share ForConditionalGeneration suffix # (e.g. CsmForConditionalGeneration, WhisperForConditionalGeneration) - _audio_only_model_types = {"csm", "whisper"} model_type = getattr(config, "model_type", None) - if model_type in _audio_only_model_types: + if model_type in _AUDIO_ONLY_MODEL_TYPES: return False - # Check 1: Architecture class name patterns - if hasattr(config, "architectures"): - is_vlm = any(x.endswith(_VLM_ARCH_SUFFIXES) for x in config.architectures) - if is_vlm: - logger.info( - f"Model {model_name} detected as VLM: architecture {config.architectures}" - ) - return True - - # Check 2: Has vision_config (most VLMs: LLaVA, Gemma-3, Qwen2-VL, etc.) - if hasattr(config, "vision_config"): - logger.info(f"Model {model_name} detected as VLM: has vision_config") + if _is_vlm(config): + archs = getattr(config, "architectures", None) or [] + logger.info( + "Model %s detected as VLM (model_type=%s, architectures=%s)", + model_name, + model_type, + archs, + ) return True - # Check 3: Has img_processor (Phi-3.5 Vision uses this instead of vision_config) - if hasattr(config, "img_processor"): - logger.info(f"Model {model_name} detected as VLM: has img_processor") - return True - - # Check 4: Has image_token_index (common in VLMs for image placeholder tokens) - if hasattr(config, "image_token_index"): - logger.info(f"Model {model_name} detected as VLM: has image_token_index") - return True - - # Check 5: Known VLM model_type values that may not match above checks - if hasattr(config, "model_type"): - if config.model_type in _VLM_MODEL_TYPES: - logger.info(f"Model {model_name} detected as VLM: model_type={config.model_type}") - return True - return False except Exception as e: