Commit Graph
2 Commits
Author SHA1 Message Date
6b14338d1e feat(tray): offer the recommended model from the missing-model notification (#687)
* feat(tray): offer the recommended model from the missing-model notification

Dictation without a model on disk showed a dead-end notification telling
the user to open Settings, find the model picker and start the download
by hand, at the moment they wanted to talk. There was no Download button
anywhere: downloads only happen as a side effect of changing a selection.

The notification now carries a "Download <model> (<size>)" button for the
model the picker already stars for this machine, so recommendations stay
in one place: get_recommended_model() for whisper.cpp, including the
English-only variant when an English language is selected, and the RAM
based recommendations for VOSK and Whisper. Clicking it downloads in the
background, reports progress in the notification, and saves the model for
that engine only after it loaded.

Notification servers that do not advertise the actions capability, and
systems without libnotify, still get the plain text they got before.
Dictation is not started for the user afterwards: a download can take
several minutes, and grabbing the microphone unannounced would surprise
someone who has long since moved on.

* feat(tray): actually fetch the offered model, and only claim it when loaded

Review follow-ups on #687.

reconfigure() only re-initializes when engine, model or language change,
and force_download is read during that re-init. First-run defaults are
whisper_cpp + tiny, and tiny is what get_recommended_model() returns on a
CPU-only machine under 16GB, so the click was a no-op that logged
"Applied VAD/silence timeout changes.", saved the model, said "Speech
model ready", and left the next shortcut showing the same missing-model
notification.

Add force_reinit to reconfigure() and pass it here, then gate the save
and the success notification on speech_engine.model_ready instead of on
reconfigure() not raising.

The "starting..." notification is now created in
_download_recommended_model, which runs on the main loop where libnotify
dispatches the action; only the handle travels to the worker, which
already routes update/close/notify through _idle_once.

The offer body no longer promises a button the server cannot draw: with
no action it points at Settings, like the notification it replaced.

Tests cover the case that breaks the feature — the recommendation equal
to the configured model — behaviourally, through a stand-in engine that
gates re-init the way reconfigure() does, plus the real reconfigure()
contract in both directions. They also stop touching sys.modules at
collection time, which is the desync #686 hit, and black is clean.

* test(notifications): cover the libnotify/notify-send boundary directly

Every test that touches notifications so far patches the whole module
away, so the module itself had no coverage: the fallback to notify-send
when the server has no action support or libnotify fails to initialize,
the handle kept alive in _pending until the user answers, and update()
refusing a fallback handle. These drive it with a fake Notify installed
through the module's own lazy cache rather than by patching
gi.repository, which is shared with every other test in the process.

* fix(download): allow only one model download at a time across tray and Settings

The tray's download worker and the Settings dialog's both drive the same
engine, which has a single progress callback slot and a single
configuration. Started together, the later one overwrote the earlier
one's progress callback, the earlier one's finally block then cleared it
mid-download, and whichever finished last became the persisted model.

Give the engine a non-blocking claim. Both Settings entry points and the
tray take it before starting a download and release it when the worker
ends; whoever loses the race is told a download is already running and
the Settings picker goes back to the saved model.

---------

Co-authored-by: Kacper Paczos <23406555+kacperpaczos@users.noreply.github.com>
2026-08-28 23:29:58 -07:00
Kacper PaczosandGitHub a4bd398945 fix(settings): save the model only once the engine loaded it (#685)
* fix(settings): save the model only once the engine loaded it

Applying settings wrote the config first and reconfigured the engine
afterwards, and reconfiguring is what downloads a missing model. A
download that failed, was cancelled, or was interrupted therefore left
the config naming a model that never reached the disk: the next start
reported "No Speech Model" while the previously working model sat on
disk, deselected. Reconfigure first and persist only on success, so a
failed switch keeps the settings that are actually running.

On failure the pickers are put back on the saved model. They otherwise
kept showing the rejected one, which also blocked a retry, since
re-selecting the same entry emits no "changed" signal.

Selecting an engine now applies as well. Every other control on the page
applies on change, but the engine only repainted the pickers: the config
and the recognizer stayed on the previous engine until another control
was touched, and closing the dialog dropped the change without a word.
Remote API is deferred until its server URL is filled in, since applying
it empty can only fail validation.

* fix(settings): restore the engine picker after a failed engine switch

Fixes the known limitation called out in the previous commit (#692):
when applying a newly selected engine failed, the resync restored the
model pickers but left the engine combo displaying the engine that was
never applied, so the page described a state that was not running.

Set the combo back to the saved engine while holding the apply
suppression flag, so _on_engine_changed() repaints the pickers without
cascading into another apply or download prompt.

* fix(settings): resync the engine picker on every path that never applied

Review follow-ups on #685.

#692 was not actually closed. The resync lived only in the download
thread's except block, and _apply_settings_internal() reports failure by
returning False as well as by raising, so the thread fell through to
set_complete(True, "") and reported a switch that never happened. Act on
the return value instead of depending on #690 to make that except live,
and add the same handling to apply_settings()'s modal path, whose except
never resynced at all.

Add _resync_engine_ui_if_unapplied() as the backstop the review asked
for: after the modal closes, and when the dialog closes, compare the
engine on screen against the engine in the config and restore the picker
when they disagree. That also covers Remote API left without a server
URL, where applying stays deliberately deferred.

_on_engine_changed now checks _initializing / _applying_settings itself,
like every other control on this page, so the combo move made by a resync
repaints without rewriting the user's language or cascading into another
apply.

Tests no longer touch sys.modules at collection time: the reimport moved
into a module-scoped fixture that also restores the attribute on
vocalinux.ui, which is what desynced test_model_deletion on 3.9/3.10.
New cases cover the modal path that #692 is about — needs_download true
with the apply returning False, and with it raising "Download cancelled" —
plus the close hook. The stub now uses RecognitionState.IDLE rather than
the string "idle", so tests stop taking the stop_recognition branch and
sleeping 0.5s.
2026-08-26 23:59:37 -07:00