fix(whispercpp): reuse an on-disk same-size weight at startup (#923)
* fix(whispercpp): reuse an on-disk same-size weight at startup The Settings picker already lets a downloaded same-size model stand in when the language-derived sibling is absent, but the startup path never applied it: a headless config with language=en-us and model_size=medium demanded ggml-medium.en.bin while ggml-medium.bin sat in the models directory and loads fine. Move the stand-in into whispercpp_model_info.on_disk_stand_in so both paths share it, and give resolve_whispercpp_variant a prefer_on_disk flag that only language-derived results use: pinned variants and stored specializations still load exactly what they name, the .en preference still wins when that file is present, and checksum verification is untouched. get_model_size_for_engine opts in so startup and headless launches match the Settings semantics. Fixes #916 * fix(whispercpp): keep is_model_downloaded free of makedirs get_model_path creates the model directory for callers that are about to write a file, but is_model_downloaded is a read-only probe: on a fresh profile its makedirs ran inside tests that stub os.makedirs for the config directory, breaking them, and a bare existence check should not mutate the filesystem anyway. Resolve the file path without creating directories in the probe; get_model_path keeps creating for writers. Refs #916 * test(whispercpp): annotate the stand-in fixture's return type AGENTS.md requires type hints on every signature; _downloaded returns the context manager from patch.object. Refs #916 --------- Co-authored-by: Jatin K Malik <jatinkrmalik@gmail.com>
This commit is contained in:
co-authored by
Jatin K Malik
parent
295386f08b
commit
8ce85f1e9e
@@ -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):
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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]:
|
||||
|
||||
@@ -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"],
|
||||
),
|
||||
)
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user