diff --git a/src/vocalinux/speech_recognition/recognition_manager.py b/src/vocalinux/speech_recognition/recognition_manager.py index 82e96c7..2f0e24a 100644 --- a/src/vocalinux/speech_recognition/recognition_manager.py +++ b/src/vocalinux/speech_recognition/recognition_manager.py @@ -1893,6 +1893,11 @@ class SpeechRecognitionManager: logger.info(f"Downloading whisper.cpp '{self.model_size}' model...") self._download_whispercpp_model() + # Backend detection plus the load itself run after the download + # reports "Complete!", so a UI watching the download needs to hear + # that this is still going. No callback is set outside that flow. + if self._download_progress_callback: + self._download_progress_callback(1.0, 0, "Loading model...") self._load_whispercpp_model(model_path) except ImportError as e: @@ -2957,6 +2962,11 @@ class SpeechRecognitionManager: try: self._stream_model_download(url, temp_file) + # Say so before hashing: on a multi-hundred-MB model this is + # seconds with nothing else to show, and the UI would otherwise + # sit on the last chunk's update. Same step the Vosk path reports. + if self._download_progress_callback: + self._download_progress_callback(1.0, 0, "Verifying model...") # Verify before the rename: whisper.cpp loads ggml files through # ctypes, so an unverified file must never reach its final path # where is_model_downloaded() would treat it as good. diff --git a/src/vocalinux/ui/settings_dialog.py b/src/vocalinux/ui/settings_dialog.py index 3fd3bf5..498f1dd 100644 --- a/src/vocalinux/ui/settings_dialog.py +++ b/src/vocalinux/ui/settings_dialog.py @@ -2238,6 +2238,11 @@ class SettingsPage: self.extras.append(child) +# What the downloaders report when the transfer itself ends. It is not the +# end of the dialog's work, so it must not be shown as one. +_DOWNLOAD_DONE_STATUSES = frozenset({"complete", "completed", "done"}) + + class ModelDownloadDialog(Gtk.Dialog): """Dialog showing model download progress with cancel support.""" @@ -2302,15 +2307,26 @@ class ModelDownloadDialog(Gtk.Dialog): self.show_all() + self._pulse_timeout = None # For Whisper, we can't track progress, so pulse if engine == "whisper": + self._start_pulsing() + + def _start_pulsing(self) -> None: + """Bounce the bar, for work that reports no measurable progress.""" + if self._pulse_timeout is None: self._pulse_timeout = GLib.timeout_add(100, self._pulse_progress) - else: + + def _stop_pulsing(self) -> None: + """Stop the bounce if it is running.""" + if self._pulse_timeout is not None: + GLib.source_remove(self._pulse_timeout) self._pulse_timeout = None def _pulse_progress(self): """Pulse the progress bar while downloading (for Whisper).""" if self.cancelled: + self._pulse_timeout = None return False self.progress_bar.pulse() return True # Continue pulsing @@ -2327,20 +2343,47 @@ class ModelDownloadDialog(Gtk.Dialog): if self.cancelled: return - # Stop pulsing if we were pulsing - if self._pulse_timeout: - GLib.source_remove(self._pulse_timeout) - self._pulse_timeout = None + if fraction >= 1.0: + self._show_finishing(status_text) + return + + self._stop_pulsing() self.progress_bar.set_fraction(fraction) self.progress_bar.set_text(f"{fraction * 100:.0f}%") self.status_label.set_markup(f"{status_text}") + def _show_finishing(self, status_text: str) -> None: + """Show the work that follows the last byte as work, not as an end. + + The bytes being in is not the dialog being done: the checksum still + has to run and the engine still has to load the file, seconds of it + on a large model, and neither reports progress or can be interrupted. + A bar parked at 100% — or worse, at the downloader's "Complete!" — + under a lone Cancel button reads as "finished, waiting for me to + confirm", or as a hang. So bounce the bar, name the step, and stop + offering a cancel that no longer cancels anything. + """ + self._start_pulsing() + + step = status_text.strip() + if not step or step.strip("!.").casefold() in _DOWNLOAD_DONE_STATUSES: + step = "Finishing..." + self.progress_bar.set_text(step) + self.status_label.set_markup( + "Download finished, still working - this can take a moment..." + ) + + if self.cancel_button.get_sensitive(): + self.cancel_button.set_sensitive(False) + self.cancel_button.set_tooltip_text( + "The download is finished. Verifying and loading the model " + "cannot be interrupted." + ) + def set_complete(self, success: bool, message: str = ""): """Mark download as complete.""" - if self._pulse_timeout: - GLib.source_remove(self._pulse_timeout) - self._pulse_timeout = None + self._stop_pulsing() # Hide cancel button self.cancel_button.hide() diff --git a/tests/test_download_finishing_feedback.py b/tests/test_download_finishing_feedback.py new file mode 100644 index 0000000..a77263d --- /dev/null +++ b/tests/test_download_finishing_feedback.py @@ -0,0 +1,218 @@ +"""The window must keep saying it is working after the last byte arrives. + +The transfer ending is not the dialog's work ending: the checksum runs, then +the engine loads the file, and on a large model that is seconds. Those seconds +used to show a bar parked at 100% with the downloader's "Complete!" under it +and Cancel as the only button, which reads as a finished download waiting to be +confirmed - or as a hang. +""" + +import importlib +import sys +from typing import Any +from unittest.mock import MagicMock, Mock, patch + +from vocalinux.speech_recognition.recognition_manager import SpeechRecognitionManager + + +def _load_settings_dialog() -> Any: + """Import settings_dialog with real bases so unbound methods stay callable. + + conftest swaps gi for a MagicMock, which leaves every ``class X(Gtk.Y)`` as + a mock. Handing the three bases a real class keeps the methods intact. + + Restore both sys.modules and the vocalinux.ui.settings_dialog attribute, so + mock and later imports see the same module object. + """ + import vocalinux.ui as ui_pkg + + repository = sys.modules["gi.repository"] + bases = {name: type(name, (), {}) for name in ("Box", "ListBoxRow", "Dialog")} + saved = sys.modules.pop("vocalinux.ui.settings_dialog", None) + try: + with patch.object(repository, "Gtk", MagicMock(**bases)): + module = importlib.import_module("vocalinux.ui.settings_dialog") + finally: + sys.modules.pop("vocalinux.ui.settings_dialog", None) + if saved is not None: + sys.modules["vocalinux.ui.settings_dialog"] = saved + ui_pkg.settings_dialog = saved + else: + ui_pkg.__dict__.pop("settings_dialog", None) + return module + + +settings_dialog = _load_settings_dialog() +ModelDownloadDialog = settings_dialog.ModelDownloadDialog + +_REAL_METHODS = ( + "update_progress", + "_show_finishing", + "_start_pulsing", + "_stop_pulsing", + "set_complete", +) + + +def _dialog_stub() -> Mock: + """A ModelDownloadDialog with real progress methods over mocked widgets.""" + dialog = Mock() + dialog.cancelled = False + dialog._pulse_timeout = None + dialog.cancel_button.get_sensitive.return_value = True + for name in _REAL_METHODS: + setattr(dialog, name, getattr(ModelDownloadDialog, name).__get__(dialog)) + return dialog + + +def _bar_text(dialog: Mock) -> str: + return dialog.progress_bar.set_text.call_args[0][0] + + +def _status_markup(dialog: Mock) -> str: + return dialog.status_label.set_markup.call_args[0][0] + + +def test_download_in_flight_still_shows_the_percentage() -> None: + dialog = _dialog_stub() + + with patch.object(settings_dialog, "GLib") as glib: + dialog.update_progress(0.42, 3.5, "3.5 MB/s - 2 min left") + + dialog.progress_bar.set_fraction.assert_called_once_with(0.42) + assert _bar_text(dialog) == "42%" + assert "3.5 MB/s" in _status_markup(dialog) + glib.timeout_add.assert_not_called() + + +def test_finished_transfer_does_not_read_as_a_finished_dialog() -> None: + """The one the bug report is about: "Complete!" while it is not complete.""" + dialog = _dialog_stub() + + with patch.object(settings_dialog, "GLib"): + dialog.update_progress(1.0, 0, "Complete!") + + assert _bar_text(dialog) == "Finishing..." + assert "still working" in _status_markup(dialog) + + +def test_the_bar_moves_while_the_model_is_verified_and_loaded() -> None: + """Motion is what tells the user the app is alive; there is no fraction.""" + dialog = _dialog_stub() + + with patch.object(settings_dialog, "GLib") as glib: + glib.timeout_add.return_value = 77 + dialog.update_progress(1.0, 0, "Verifying model...") + dialog.update_progress(1.0, 0, "Loading model...") + + # Named steps are shown as they come, and the bounce is started once. + assert _bar_text(dialog) == "Loading model..." + assert glib.timeout_add.call_count == 1 + assert dialog._pulse_timeout == 77 + + +def test_cancel_stops_being_offered_once_it_cannot_cancel() -> None: + dialog = _dialog_stub() + + with patch.object(settings_dialog, "GLib"): + dialog.update_progress(1.0, 0, "Verifying model...") + + dialog.cancel_button.set_sensitive.assert_called_once_with(False) + assert dialog.cancel_button.set_tooltip_text.called + + +def test_set_complete_ends_the_bounce_and_offers_the_way_out() -> None: + dialog = _dialog_stub() + + with patch.object(settings_dialog, "GLib") as glib: + glib.timeout_add.return_value = 77 + dialog.update_progress(1.0, 0, "Loading model...") + dialog.set_complete(True) + + glib.source_remove.assert_called_once_with(77) + assert dialog._pulse_timeout is None + assert _bar_text(dialog) == "Complete!" + dialog.add_button.assert_called_once() + + +def _make_manager() -> SpeechRecognitionManager: + """A whisper.cpp manager whose engine init is stubbed out.""" + with ( + patch.object(SpeechRecognitionManager, "_init_vosk"), + patch.object(SpeechRecognitionManager, "_init_whisper"), + patch.object(SpeechRecognitionManager, "_init_whispercpp"), + patch.object(SpeechRecognitionManager, "_init_parakeet"), + patch.object(SpeechRecognitionManager, "_init_faster_whisper"), + ): + return SpeechRecognitionManager( + engine="whisper_cpp", + model_size="small", + language="en-us", + defer_download=True, + ) + + +def test_whispercpp_download_reports_the_verification_before_running_it() -> None: + """Hashing a 465 MB model is seconds of silence if nobody announces it. + + Asserted as ordering, not presence: a status reported after the hash tells + the user nothing while they are waiting on it. The Vosk path has reported + this step all along. + """ + manager = _make_manager() + statuses = [] + manager._download_progress_callback = lambda fraction, speed, text: statuses.append(text) + seen_when_hashing = [] + + requests = MagicMock() + response = MagicMock() + response.headers = {"content-length": "1000"} + response.iter_content.return_value = [b"x" * 1000] + requests.get.return_value = response + requests.exceptions.RequestException = Exception + + with ( + patch.dict("sys.modules", {"requests": requests}), + patch( + "vocalinux.speech_recognition.recognition_manager.get_model_path", + return_value="/tmp/ggml-small.bin", + ), + patch.object(SpeechRecognitionManager, "_stream_model_download"), + patch("os.rename"), + patch( + "vocalinux.speech_recognition.recognition_manager.verify_model_file", + side_effect=lambda *a, **kw: seen_when_hashing.extend(statuses), + ), + ): + manager._download_whispercpp_model() + + assert "Verifying model..." in seen_when_hashing + + +def test_whispercpp_init_reports_the_load_before_running_it() -> None: + """Backend detection and the load itself come after "Complete!".""" + manager = _make_manager() + statuses = [] + manager._download_progress_callback = lambda fraction, speed, text: statuses.append(text) + seen_when_loading = [] + + with ( + patch("os.path.exists", return_value=True), + patch( + "vocalinux.speech_recognition.recognition_manager." + "_preload_pywhispercpp_shared_libraries" + ), + patch.object(SpeechRecognitionManager, "_whispercpp_model_is_verified", return_value=True), + patch.dict( + "sys.modules", + {"pywhispercpp": MagicMock(), "pywhispercpp.model": MagicMock()}, + ), + patch.object( + SpeechRecognitionManager, + "_load_whispercpp_model", + side_effect=lambda *a, **kw: seen_when_loading.extend(statuses), + ), + ): + manager._init_whispercpp() + + assert "Loading model..." in seen_when_loading