fix(shortcut): keep toggle parity when presses arrive mid-pipeline - #1910
Merged
Conversation
…xternal triggers Toggle-style triggers (SIGUSR2, CLI flags, pedals that signal on both edges) flip state on every edge. A press arriving while the previous pipeline was still processing used to be dropped, desyncing parity: the next edge then started a recording nobody would stop, leaving the overlay waiting for input with the button long released. The coordinator now remembers at most one press while the pipeline is busy: a second toggle press (or a PTT release) during the same busy window cancels it, preserving parity, and a remembered press starts recording as soon as the pipeline drains. External triggers bypass the 30 ms keyboard debounce, which could eat one edge of a quick external tap and break parity the same way.
…ransport loop The coordinator thread mixed transport (mpsc/recv_timeout), state, decisions and effect execution, so sequence tests had to mirror the transitions in a hand-written simulator that covered only the cjpais#1539 auto-repeat path and could silently drift from production behavior. All transition logic now lives in a pure CoordinatorState (on_input / on_grace_expired / on_cancel / on_processing_finished) returning Effect values; the thread only transports commands and executes effects, reporting start failures back via on_start_result so the optimistic Recording transition rolls back when recording fails to begin. Tests drive the real machine directly and additionally cover the busy-pipeline remember/forget and cancel-drain sequences.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before Submitting This PR
Human Written Description
I found this bug while working on #1909, but split these fixes into a separate PR to keep that one small and ease review.
Rapid re-triggers of push-to-talk via external toggle triggers (e.g. SIGUSR2 sent on both key press and release) could wedge the recording state: the overlay would sit in "recording" with the button long released, waiting for input.
Two places in the transcription coordinator dropped toggle edges:
A press arriving while the previous transcription is still processing was ignored. Toggle triggers flip state on every edge, so a dropped press desyncs the parity: the next edge then starts a recording nobody will stop. Now a busy press is remembered (or forgotten, if the key was already released) and started when the pipeline drains — a quick tap inside the busy window nets to no-op, an odd press starts exactly one recording.
The 30 ms debounce also swallowed external triggers. Two edges faster than 30 ms (a scripted double-tap) lost the second one — same desync. External triggers (signals, CLI flags) are deliberate edges from the user's own integration and are now exempt; physical key repeats still debounce.
The second commit is a behavior-neutral refactor that moves all transition logic into a pure
CoordinatorStatethe tests drive directly, replacing a hand-written test simulator that could drift from production.Known remaining limitation
Cancel mid-hold with directionless toggle triggers can still desync parity (the release edge arrives at idle and toggles a fresh recording). A proper fix requires direction-carrying triggers (explicit press/release API) — invasive changes that need to be discussed with the maintainer before implementation, so they are out of scope for this PR.
Related Issues/Discussions
Community Feedback
Bug fix found while dogfooding push-to-talk via external triggers (triggerhappy -> SIGUSR2 on both key edges); no prior discussion.
Testing
cargo test: 210 passed — classification unit tests plus sequence-level tests driven through the real CoordinatorState ([BUG] Push-to-talk rapidly toggles recording while the key is held (X11 auto-repeat synthesizes KeyRelease/KeyPress) #1539 auto-repeat regression, busy-window remember/forget, cancel during processing, failed-start rollback).cargo clippyclean.Screenshots/Videos
AI Assistance
If AI was used: