diff --git a/src/vocalinux/ui/config_manager.py b/src/vocalinux/ui/config_manager.py index f50d073..a9bc06b 100644 --- a/src/vocalinux/ui/config_manager.py +++ b/src/vocalinux/ui/config_manager.py @@ -18,6 +18,7 @@ from ..utils.whispercpp_model_info import WHISPERCPP_MODEL_INFO, default_variant from ..utils.whispercpp_model_info import get_model_size as get_whispercpp_model_size from ..utils.whispercpp_model_info import is_dictation_model from ..utils.whispercpp_model_info import is_english_only_model as is_english_only_whispercpp_model +from ..utils.whispercpp_model_info import on_disk_stand_in logger = logging.getLogger(__name__) @@ -321,7 +322,13 @@ def _multilingual_sibling(model_name: str) -> str: return derived if derived in WHISPERCPP_MODEL_INFO else model_name -def resolve_whispercpp_variant(saved_model: str, pinned_variant: str, language_id: str) -> str: +def resolve_whispercpp_variant( + saved_model: str, + pinned_variant: str, + language_id: str, + *, + prefer_on_disk: bool = False, +) -> str: """Resolve the loadable whisper.cpp id for a saved size, pin, and language. A pin outranks everything except an English-only id when the language is not @@ -330,16 +337,31 @@ def resolve_whispercpp_variant(saved_model: str, pinned_variant: str, language_i plain ``{size}.en`` id is the language-derived default, not a leftover specialization. True leftover specializations (turbo, versioned large, quantized multilingual) are still honoured. + + With ``prefer_on_disk`` a language-derived result stands down for a + downloaded same-size weight instead of forcing a fresh download (#916). + Pinned and leftover specialization picks are explicit choices: they are + returned verbatim and never swapped. """ # The "layout" sentinel is deliberately not in the catalog, so it reads as # non-English here and derives the multilingual variant -- which is what a # mode that can land on any language needs (#821). language_is_english = SUPPORTED_LANGUAGES.get(language_id, {}).get("whisper") == "en" + def stand_in(variant: str) -> str: + # Only language-derived results reach this helper; an explicit pick is + # returned verbatim below and never comes through here. + if not prefer_on_disk: + return variant + return on_disk_stand_in(variant, get_whispercpp_model_size(variant), language_is_english) + pinned = pinned_variant.lower() if isinstance(pinned_variant, str) else "" if pinned in WHISPERCPP_MODEL_INFO and is_dictation_model(pinned): if not language_is_english and is_english_only_whispercpp_model(pinned): - return _multilingual_sibling(pinned) + # English-only weights cannot serve the language, so the pin is + # already overridden; the multilingual sibling is language-derived + # and a downloaded same-size weight may stand in for it. + return stand_in(_multilingual_sibling(pinned)) return pinned saved = saved_model.lower() if isinstance(saved_model, str) else "" @@ -355,13 +377,15 @@ def resolve_whispercpp_variant(saved_model: str, pinned_variant: str, language_i and saved != f"{size}.en" ): if not language_is_english and is_english_only_whispercpp_model(saved): - return _multilingual_sibling(saved) + return stand_in(_multilingual_sibling(saved)) return saved derived = default_variant_for_size(size, language_is_english) if derived in WHISPERCPP_MODEL_INFO: - return derived - return saved if saved in WHISPERCPP_MODEL_INFO and is_dictation_model(saved) else "tiny" + return stand_in(derived) + return stand_in( + saved if saved in WHISPERCPP_MODEL_INFO and is_dictation_model(saved) else "tiny" + ) class ConfigManager: @@ -743,11 +767,14 @@ class ConfigManager: # Unpinned configs store the bare size; the engine still needs the # language-derived loadable id (and leftover ``{size}.en`` must not - # block a later language change). + # block a later language change). The derived default stands down for + # a downloaded same-size weight so headless startup does not demand a + # sibling download when a usable model is already on disk (#916). return resolve_whispercpp_variant( saved, self.get_model_variant_for_engine(engine), sr_config.get("language", "auto"), + prefer_on_disk=True, ) def set_model_size_for_engine(self, engine: str, model_size: str): diff --git a/src/vocalinux/ui/settings_dialog.py b/src/vocalinux/ui/settings_dialog.py index 45a1421..b879a47 100644 --- a/src/vocalinux/ui/settings_dialog.py +++ b/src/vocalinux/ui/settings_dialog.py @@ -105,6 +105,7 @@ from ..utils.whispercpp_model_info import is_model_downloaded as is_whispercpp_m from ..utils.whispercpp_model_info import ( list_downloaded_models as list_downloaded_whispercpp_models, ) +from ..utils.whispercpp_model_info import on_disk_stand_in as whispercpp_on_disk_stand_in from ..version import __copyright__, __url__, __version__ # noqa: E402 from .config_manager import ( # noqa: E402 DEFAULT_CONFIG, @@ -8883,26 +8884,7 @@ class SettingsDialog(Gtk.Dialog): language stands in instead; the info card still offers the better variant as a download, it just no longer forces it. """ - if is_whispercpp_model_downloaded(variant): - return variant - wants_english = _language_is_english(language) - candidates = [ - name - for name in get_whispercpp_model_variants(size) - if is_whispercpp_model_downloaded(name) - and (wants_english or not is_english_only_whispercpp_model(name)) - ] - if not candidates: - return variant - - def rank(name: str) -> tuple: - # Closest to what was derived: English-only first when English is - # wanted, the plain multilingual next, quantized ones last. - english_first = 0 if wants_english and is_english_only_whispercpp_model(name) else 1 - quantized = 1 if "-q" in name else 0 - return (english_first, quantized, name) - - return min(candidates, key=rank) + return whispercpp_on_disk_stand_in(variant, size, _language_is_english(language)) def _apply_simple_choice(self) -> None: """Drive the advanced controls from the simple questions. diff --git a/src/vocalinux/utils/whispercpp_model_info.py b/src/vocalinux/utils/whispercpp_model_info.py index e265f68..cdcfc12 100644 --- a/src/vocalinux/utils/whispercpp_model_info.py +++ b/src/vocalinux/utils/whispercpp_model_info.py @@ -540,6 +540,17 @@ def get_recommended_model() -> tuple[str, str]: return "tiny", "Default recommendation" +def _model_file_path(model_name: str) -> str: + """Resolve the model file path without touching the filesystem.""" + whispercpp_dir = os.path.join(models_dir(), "whispercpp") + + model_info = WHISPERCPP_MODEL_INFO.get(model_name) + if model_info and model_info.get("url"): + return os.path.join(whispercpp_dir, os.path.basename(model_info["url"])) + + return os.path.join(whispercpp_dir, f"ggml-{model_name}.bin") + + def get_model_path(model_name: str) -> str: """ Get the path where a model should be stored. @@ -550,28 +561,57 @@ def get_model_path(model_name: str) -> str: Returns: Path to the model file """ - whispercpp_dir = os.path.join(models_dir(), "whispercpp") - os.makedirs(whispercpp_dir, exist_ok=True) - - model_info = WHISPERCPP_MODEL_INFO.get(model_name) - if model_info and model_info.get("url"): - return os.path.join(whispercpp_dir, os.path.basename(model_info["url"])) - - return os.path.join(whispercpp_dir, f"ggml-{model_name}.bin") + model_path = _model_file_path(model_name) + os.makedirs(os.path.dirname(model_path), exist_ok=True) + return model_path def is_model_downloaded(model_name: str) -> bool: """ Check if a whisper.cpp model is downloaded. + A read-only probe: it must not create the models directory, so it resolves + the file path without get_model_path's makedirs side effect. + Args: model_name: Name of the model Returns: True if model exists, False otherwise """ - model_path = get_model_path(model_name) - return os.path.exists(model_path) + return os.path.exists(_model_file_path(model_name)) + + +def on_disk_stand_in(variant: str, size: str, language_is_english: bool) -> str: + """Prefer a downloaded weight of the same size over fetching a sibling. + + A resolved variant whose weights are absent stands down for a downloaded + same-size weight that can serve the language: an English-only weight + stands in only when English is wanted. The resolved variant itself wins + whenever it is already downloaded, so the ``.en`` preference and explicit + picks are kept whenever their files are present. + """ + if is_model_downloaded(variant): + return variant + + candidates = [ + name + for name in get_model_variants(size) + if is_model_downloaded(name) and (language_is_english or not is_english_only_model(name)) + ] + if not candidates: + return variant + + def rank(name: str) -> tuple: + # Closest to what was derived: English-only first when English is + # wanted, the plain multilingual next, quantized ones last. + english_first = 0 if language_is_english and is_english_only_model(name) else 1 + quantized = 1 if "-q" in name else 0 + return (english_first, quantized, name) + + chosen = min(candidates, key=rank) + logger.info("whisper.cpp model %s is not downloaded; using same-size %s", variant, chosen) + return chosen def list_downloaded_models() -> list[str]: diff --git a/tests/test_simple_model_settings.py b/tests/test_simple_model_settings.py index 5081ee2..4acb0b1 100644 --- a/tests/test_simple_model_settings.py +++ b/tests/test_simple_model_settings.py @@ -7,6 +7,7 @@ from unittest.mock import MagicMock, Mock, patch import pytest import vocalinux.ui +from vocalinux.utils import whispercpp_model_info from vocalinux.utils.model_choice import ( ACCURATE, BALANCED, @@ -634,15 +635,17 @@ def test_restoring_after_auto_detect_falls_back_to_a_real_language(settings_dial def _with_disk(settings_dialog, downloaded): + # The stand-in helper lives in whispercpp_model_info and reads its own + # module globals, so the disk state is patched there, not on the dialog. return ( patch.object( - settings_dialog, - "is_whispercpp_model_downloaded", + whispercpp_model_info, + "is_model_downloaded", side_effect=lambda name: name in downloaded, ), patch.object( - settings_dialog, - "get_whispercpp_model_variants", + whispercpp_model_info, + "get_model_variants", return_value=["base", "base.en", "base-q5_1", "base.en-q5_1", "base-q8_0"], ), ) diff --git a/tests/test_whispercpp_variant_resolution.py b/tests/test_whispercpp_variant_resolution.py index 3172c7a..05881d6 100644 --- a/tests/test_whispercpp_variant_resolution.py +++ b/tests/test_whispercpp_variant_resolution.py @@ -15,6 +15,7 @@ import pytest import vocalinux.ui from vocalinux.ui.config_manager import ConfigManager, resolve_whispercpp_variant +from vocalinux.utils import whispercpp_model_info @pytest.fixture(scope="module") @@ -299,3 +300,92 @@ def test_pinned_save_persists_the_full_variant() -> None: assert sr_config["whisper_cpp_model_variant"] == "medium" assert sr_config["whisper_cpp_model_size"] == "medium" assert manager.get_model_size_for_engine("whisper_cpp") == "medium" + + +# --- headless startup reuses what is on disk (#916) ----------------------- +# +# The Settings picker's same-size stand-in never reached the startup path, so a +# headless config could demand ggml-medium.en.bin while ggml-medium.bin of the +# same size sat in the models directory. ``get_model_size_for_engine`` is the +# single resolution point startup reads. + + +def _manager(model_size: str, variant: str, language: str) -> ConfigManager: + manager = ConfigManager() + sr = manager.config["speech_recognition"] + sr["engine"] = "whisper_cpp" + sr["whisper_cpp_model_size"] = model_size + sr["whisper_cpp_model_variant"] = variant + sr["language"] = language + return manager + + +def _downloaded(*names: str) -> Any: + """Patch only the disk probe; the real variant catalog stays in place.""" + return patch.object( + whispercpp_model_info, + "is_model_downloaded", + side_effect=lambda name: name in names, + ) + + +def test_startup_uses_downloaded_same_size_weight_for_a_missing_en_sibling() -> None: + """The reported bug: ggml-medium.bin must stand in when medium.en is absent.""" + manager = _manager("medium", "", "en-us") + + with _downloaded("medium"): + assert manager.get_model_size_for_engine("whisper_cpp") == "medium" + + +def test_startup_keeps_the_en_preference_when_the_en_model_is_downloaded() -> None: + """An on-disk .en still wins: the stand-in never overrides what is present.""" + manager = _manager("medium", "", "en-us") + + with _downloaded("medium", "medium.en"): + assert manager.get_model_size_for_engine("whisper_cpp") == "medium.en" + + +def test_startup_demands_the_derived_model_when_nothing_of_that_size_is_on_disk() -> None: + """No same-size weight on disk means the language-derived id and a download.""" + manager = _manager("medium", "", "en-us") + + with _downloaded("small"): + assert manager.get_model_size_for_engine("whisper_cpp") == "medium.en" + + +def test_startup_never_stands_an_english_only_weight_in_for_another_language() -> None: + """Polish cannot use .en weights, so a missing multilingual stays a download.""" + manager = _manager("medium", "", "pl") + + with _downloaded("medium.en"): + assert manager.get_model_size_for_engine("whisper_cpp") == "medium" + + +def test_a_pinned_variant_is_never_swapped_for_a_same_size_stand_in() -> None: + """An explicit pin is a choice: it names the weights to fetch or load.""" + manager = _manager("medium", "medium.en", "en-us") + + with _downloaded("medium"): + assert manager.get_model_size_for_engine("whisper_cpp") == "medium.en" + + +def test_a_stored_specialization_is_never_swapped_for_a_same_size_stand_in() -> None: + """A leftover quantized id in the size field is a deliberate pick too.""" + manager = _manager("medium-q5_0", "", "en-us") + + with _downloaded("medium"): + assert manager.get_model_size_for_engine("whisper_cpp") == "medium-q5_0" + + +def test_the_plain_weight_stands_in_ahead_of_a_quantized_one() -> None: + """Same ranking as the picker: unquantized is closest to the derived variant.""" + manager = _manager("medium", "", "en-us") + + with _downloaded("medium-q5_0", "medium"): + assert manager.get_model_size_for_engine("whisper_cpp") == "medium" + + +def test_resolution_without_the_disk_preference_still_derives_the_sibling() -> None: + """The picker path keeps deriving the language variant regardless of disk.""" + with _downloaded("medium"): + assert resolve_whispercpp_variant("medium", "", "en-us") == "medium.en"