fix(settings): stage Advanced model spec until Download is confirmed (#908)
* fix(settings): stage Advanced model spec until Download is confirmed Picker changes in Speech Model -> Advanced staged a spec but immediately triggered a model download for every intermediate selection. Gate the auto-apply download path so a missing model only stages: a Download action on the model info card now runs apply_settings() as the single explicit confirmation. Simple-mode questions keep the instant apply-and-download flow. Fixes #894 * fix(settings): repaint model info card after a completed download The post-download paths repopulated pickers and resynced the engine but never repainted the info card, so a finished download left the amber status and the Download action visible for a model already on disk. The cancel path already repainted via the idle resync; give success its own. * fix(settings): keep staged spec through picker rebuilds and save non-model edits * fix(settings): persist non-model edits without a model-reloading reconfigure * fix(settings): live-apply non-model edits, keep whispercpp params saved-only while staged * fix(settings): live-set changed VAD/timeout attrs without reconfigure while staged --------- Co-authored-by: Jatin K Malik <jatinkrmalik@gmail.com>
This commit is contained in:
co-authored by
Jatin K Malik
parent
775863fd30
commit
329a7d7f6a
@@ -318,6 +318,20 @@ def _model_specialization_display_name(model_name: str) -> str:
|
||||
#: attribute, so tests driving the methods against a Mock still reach it.
|
||||
FOLLOW_LAYOUT_ENGINES = ("whisper", "whisper_cpp", "faster_whisper", "remote_api")
|
||||
|
||||
#: Selected-setting keys that make up the model spec. While a spec that needs a
|
||||
#: download is staged (#894), these stay out of the saved config until the
|
||||
#: explicit Download confirmation; every other key applies and saves as usual.
|
||||
_MODEL_SPEC_KEYS = ("engine", "model_size", "model_variant", "language")
|
||||
|
||||
#: Engine attributes a Settings edit can update live with the same clamping
|
||||
#: ``reconfigure()`` applies — minus its recording cancel and any model
|
||||
#: reload. Every other key (all ``whispercpp_*`` load-time params) stays
|
||||
#: saved-only while a download is staged (#894).
|
||||
_LIVE_ENGINE_SETTINGS = {
|
||||
"vad_sensitivity": lambda value: max(1, min(5, int(value))),
|
||||
"silence_timeout": lambda value: max(0.5, min(5.0, float(value))),
|
||||
}
|
||||
|
||||
|
||||
def _is_following_layout(dialog: Any) -> bool:
|
||||
"""Whether the follow-keyboard-layout mode is on for this dialog (#821).
|
||||
@@ -2666,6 +2680,10 @@ class SettingsDialog(Gtk.Dialog):
|
||||
# Cached so Mock-driven tests and early calls read a real bool.
|
||||
self._follow_layout_active = False
|
||||
self._applying_settings = False # Flag to prevent recursive settings application
|
||||
# The last Advanced picker spec that needs a download and is still
|
||||
# unconfirmed: picker rebuilds keep offering it instead of falling back
|
||||
# to the saved model, and a real apply or resync clears it (#894).
|
||||
self._staged_model_spec: Optional[dict] = None
|
||||
self._advanced_prompt_dirty = False
|
||||
self._language_candidates_dirty = False
|
||||
# Deferred text edits stashed when the dialog closes mid-apply; persisted
|
||||
@@ -4757,6 +4775,15 @@ class SettingsDialog(Gtk.Dialog):
|
||||
self.model_info_subtitle.get_style_context().add_class("model-info-subtitle")
|
||||
self.model_info_card.pack_start(self.model_info_subtitle, False, False, 0)
|
||||
|
||||
# Picking a spec only stages it; this button is the explicit
|
||||
# confirmation that turns the staged spec into a download (#894).
|
||||
self.model_download_button = Gtk.Button(label="Download")
|
||||
self.model_download_button.get_style_context().add_class("suggested-action")
|
||||
self.model_download_button.set_halign(Gtk.Align.START)
|
||||
self.model_download_button.set_no_show_all(True)
|
||||
self.model_download_button.connect("clicked", self._on_download_model_clicked)
|
||||
self.model_info_card.pack_start(self.model_download_button, False, False, 0)
|
||||
|
||||
# The recommendation used to be a plain label, which left the panel stating
|
||||
# the right answer while the pickers kept the wrong one (#778).
|
||||
self.model_recommendation_box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=8)
|
||||
@@ -8080,11 +8107,22 @@ class SettingsDialog(Gtk.Dialog):
|
||||
combo_id = size if engine == "faster_whisper" else size.capitalize()
|
||||
self.model_combo.append(combo_id, display_text)
|
||||
|
||||
# Determine which model to select
|
||||
# Determine which model to select. A staged spec wins over the
|
||||
# saved model so a rebuild (e.g. a language change) keeps the
|
||||
# unconfirmed pick instead of snapping back to the saved one.
|
||||
saved_model = saved_model_for_engine.lower()
|
||||
valid_models = [m.lower() for m in ENGINE_MODELS.get(engine, [])]
|
||||
|
||||
if saved_model in valid_models:
|
||||
staged = self._staged_model_spec
|
||||
staged_model = None
|
||||
if isinstance(staged, dict) and staged.get("engine") == engine:
|
||||
candidate = str(staged.get("model_size") or "").lower()
|
||||
if candidate in valid_models:
|
||||
staged_model = candidate
|
||||
|
||||
if staged_model:
|
||||
selected = staged_model
|
||||
elif saved_model in valid_models:
|
||||
selected = saved_model
|
||||
elif downloaded_models:
|
||||
selected = downloaded_models[0]
|
||||
@@ -8135,7 +8173,16 @@ class SettingsDialog(Gtk.Dialog):
|
||||
recommended_model, _ = self._get_recommended_whispercpp_model_for_language()
|
||||
recommended_size = get_whispercpp_model_size(recommended_model)
|
||||
|
||||
saved_model = self._resolve_saved_whispercpp_variant(saved_model_for_engine)
|
||||
staged = self._staged_model_spec
|
||||
staged_model = ""
|
||||
if isinstance(staged, dict) and staged.get("engine") == "whisper_cpp":
|
||||
staged_model = str(staged.get("model_size") or "")
|
||||
if staged_model in WHISPERCPP_MODEL_INFO:
|
||||
# The staged spec survives rebuilds until its Download confirmation
|
||||
# or a real apply (#894); it wins over the saved seed.
|
||||
saved_model = staged_model
|
||||
else:
|
||||
saved_model = self._resolve_saved_whispercpp_variant(saved_model_for_engine)
|
||||
if saved_model not in WHISPERCPP_MODEL_INFO:
|
||||
saved_model = (
|
||||
recommended_model if recommended_model in WHISPERCPP_MODEL_INFO else "tiny"
|
||||
@@ -8948,7 +8995,9 @@ class SettingsDialog(Gtk.Dialog):
|
||||
)
|
||||
self.config_manager.set("speech_recognition", "simple_second_language", second or "")
|
||||
self._update_simple_visibility()
|
||||
self._auto_apply_settings()
|
||||
# A simple answer resolves to a complete spec, so its pick keeps the
|
||||
# instant apply-and-download flow the Advanced pickers no longer get.
|
||||
self._auto_apply_settings(allow_download=True)
|
||||
|
||||
def _commit_or_restore_simple_language_entry(self) -> bool:
|
||||
"""Resolve text typed into the simple language box, or restore the last pick."""
|
||||
@@ -9124,6 +9173,9 @@ class SettingsDialog(Gtk.Dialog):
|
||||
else:
|
||||
status = f"<span foreground='#e5a50a'>Download ~{_format_size(info['size_mb'])}</span>"
|
||||
self.model_info_subtitle.set_markup(f"{extra_info} · {status}")
|
||||
# The confirm action follows the displayed spec: offered only while it
|
||||
# names a model that is not on disk (#894).
|
||||
self.model_download_button.set_visible(not is_downloaded)
|
||||
|
||||
target = recommended
|
||||
message = None
|
||||
@@ -9155,8 +9207,16 @@ class SettingsDialog(Gtk.Dialog):
|
||||
self.model_info_title.show()
|
||||
self.model_info_subtitle.show()
|
||||
|
||||
def _auto_apply_settings(self):
|
||||
"""Automatically apply settings when changed."""
|
||||
def _auto_apply_settings(self, allow_download: bool = False) -> None:
|
||||
"""Automatically apply settings when changed.
|
||||
|
||||
A picker change stages a model spec; it never confirms a download.
|
||||
``allow_download`` is passed only by the simple questions, whose each
|
||||
answer resolves to a complete spec, so their pick keeps the existing
|
||||
apply-and-download flow. Every other caller leaves a missing model
|
||||
staged: the pickers keep it, the info card offers Download, and only
|
||||
``apply_settings`` (that button, or Test) starts the transfer (#894).
|
||||
"""
|
||||
if _handlers_suppressed(self):
|
||||
return
|
||||
|
||||
@@ -9194,6 +9254,45 @@ class SettingsDialog(Gtk.Dialog):
|
||||
# Check if model needs to be downloaded
|
||||
missing_model = _undownloaded_model_info(self.language, engine, model_name)
|
||||
|
||||
if missing_model is not None and not allow_download:
|
||||
# Staging, not confirming: leave the staged spec in the pickers
|
||||
# and the saved one in the config, and repaint the info card so
|
||||
# its Download action offers the explicit confirmation (#894).
|
||||
self._staged_model_spec = {
|
||||
"engine": engine,
|
||||
"model_size": model_name,
|
||||
"model_variant": settings.get("model_variant", ""),
|
||||
"language": self.language,
|
||||
}
|
||||
# Edits outside the spec are not part of the staging: apply
|
||||
# and persist them so VAD, timeouts and advanced params are
|
||||
# not dropped while a download is staged. Not reconfigure() —
|
||||
# it cancels a buffered recording and waits on its worker
|
||||
# here on the GTK thread, and every whispercpp_* kwarg forces
|
||||
# a model reload. Live-set only what actually changed; the
|
||||
# load-time params stay saved-only until the Download apply
|
||||
# or the next start.
|
||||
non_model_settings = {
|
||||
key: value for key, value in settings.items() if key not in _MODEL_SPEC_KEYS
|
||||
}
|
||||
applied = True
|
||||
for key, clamp in _LIVE_ENGINE_SETTINGS.items():
|
||||
if key not in non_model_settings:
|
||||
continue
|
||||
try:
|
||||
live_value = clamp(non_model_settings[key])
|
||||
except (TypeError, ValueError) as e:
|
||||
logger.error(f"Invalid {key} while staging: {e}")
|
||||
applied = False
|
||||
continue
|
||||
if getattr(self.speech_engine, key, None) == live_value:
|
||||
continue
|
||||
setattr(self.speech_engine, key, live_value)
|
||||
if non_model_settings and applied:
|
||||
self._save_selected_settings(non_model_settings)
|
||||
self._update_model_info()
|
||||
return
|
||||
|
||||
if missing_model is not None:
|
||||
if not self.speech_engine.try_begin_download():
|
||||
# The tray is already downloading a model; a second download
|
||||
@@ -9272,6 +9371,10 @@ class SettingsDialog(Gtk.Dialog):
|
||||
# Whatever ended the modal — cancel, failure, or a path not
|
||||
# covered above — the pickers must not outlive the config.
|
||||
self._resync_engine_ui_if_unapplied()
|
||||
# Repaint the info card too: after a success it still showed the
|
||||
# pre-download render (amber size line, Download action) until
|
||||
# some other change repainted it (#894 follow-up).
|
||||
self._update_model_info()
|
||||
return
|
||||
|
||||
logger.info(f"Auto-applying settings: {settings}")
|
||||
@@ -9386,6 +9489,9 @@ class SettingsDialog(Gtk.Dialog):
|
||||
for key, value in advanced_settings.items():
|
||||
self.config_manager.set("advanced", key, value)
|
||||
self.config_manager.save_settings()
|
||||
if any(key in settings for key in _MODEL_SPEC_KEYS):
|
||||
# A real spec just persisted, so nothing is staged anymore (#894).
|
||||
self._staged_model_spec = None
|
||||
|
||||
def get_selected_settings(self) -> dict[str, Any]:
|
||||
"""Return the currently selected settings from the UI."""
|
||||
@@ -9783,6 +9889,16 @@ For now, the engine has been reverted to VOSK."""
|
||||
self._populate_model_options()
|
||||
self._update_engine_specific_ui()
|
||||
|
||||
def _on_download_model_clicked(self, _button: Any) -> None:
|
||||
"""Confirm the staged model spec and start its download (#894).
|
||||
|
||||
The pickers only stage a specification; this is the one explicit
|
||||
confirmation that turns it into a transfer. It goes through
|
||||
``apply_settings`` so the engine claim, the modal progress dialog, and
|
||||
the post-download apply behave exactly like any other confirmed apply.
|
||||
"""
|
||||
self.apply_settings()
|
||||
|
||||
def apply_settings(self) -> bool:
|
||||
"""Apply the selected settings."""
|
||||
if self._applying_settings:
|
||||
@@ -9866,6 +9982,9 @@ For now, the engine has been reverted to VOSK."""
|
||||
|
||||
self._populate_model_options()
|
||||
self._resync_engine_ui_if_unapplied()
|
||||
# Same repaint as the auto-apply path: a finished download must not
|
||||
# leave the card offering a Download for the model just fetched.
|
||||
self._update_model_info()
|
||||
return True
|
||||
|
||||
return self._apply_settings_internal(settings)
|
||||
|
||||
Reference in New Issue
Block a user