Commit Graph
3 Commits
Author SHA1 Message Date
Jatin K MalikandGitHub 40bc31bebf fix(settings): stop freezing when applying an already-downloaded model (#790)
* fix(settings): apply already-downloaded models off the GTK main loop

Changing Model Size, Specialization, or Language froze Settings when the
model was already on disk, because reconfigure ran in the changed handler.
Move that apply onto a worker thread (same shape as the download path) and
keep the apply-guard held until GLib.idle_add clears it so a second pick
cannot start a parallel load.

Fixes #789.

* fix(settings): guard Test/apply while downloaded-model worker runs

Block apply_settings and Test Dictation while _applying_settings is held
so a concurrent Test click cannot race the shared engine. Annotate the
new worker/helpers per AGENTS.md and cover the no-op/block behavior.

* fix(settings): resync pickers when a second pick lands mid-apply

While an already-downloaded model worker was running, Model Size /
Specialization / Language could still move. The apply guard blocked
saving the new pick, then finish only cleared the flag, so the UI
could show B with dictation still on A. On finish, if the selection
no longer matches the saved config, put the pickers back. Idle
resyncs from workers now skip a destroyed dialog the same way update
check does.

* fix(settings): always resync pickers after model apply finishes

A second Model Size pick on whisper.cpp could move the size combo
while the apply guard skipped rebuilding specialization, so selected
settings still looked like the saved variant and finish skipped the
resync. Always put the pickers back on the saved config when the
worker ends, and restore language from that config too.

* test(settings): prove mid-apply picker handlers round-trip on finish

Call the real size, specialization, and language handlers while the
apply guard is held, and run finish through the real resync path so
whisper.cpp size and language cannot silently stick on the second pick.
2026-09-07 14:11:54 -07:00
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