feat: voice dictation (speech-to-text)#557
Merged
Merged
Conversation
Adds voice dictation to Zero. Run `/voice` to enter hold-to-record mode, then hold **Space** to dictate and release to transcribe (run `/voice` again to turn it off so Space types normally). Pick a backend with `/stt-model`. ### Backends - **Local (sherpa-onnx)** — offline, no API key. Auto-downloads a model via `/stt-model`; the engine binary is fetched once and shared across models. Family auto-detection covers the mainstream offline families (Whisper, transducer/Zipformer, Moonshine, Paraformer, SenseVoice, and the CTC families FireRedASR, NeMo, TeleSpeech, WeNet, Zipformer-CTC, Dolphin, Omnilingual — plus the encoder/decoder families Canary and FireRedASR-AED). - **Cloud batch** — Groq and OpenAI Whisper. Keys resolve from a configured provider, the encrypted credential store, or the env var; a missing/invalid key is entered inline. - **Streaming** — a warm local sherpa-onnx websocket server (live transcript as you speak) on desktop, or Deepgram / OpenAI Realtime. ### Highlights - Live audio-reactive waveform while recording (real mic levels when streaming; a synthesized wave for batch). - `/stt-model` download chooser: recommended models first, Live/Batch tag per row, size + download state, and a search filter. - Streaming transcript is inserted live into the composer and committed on stop; batch inserts on stop. Insert-for-review by default (`stt.autoSubmit` to fire straight at the agent). - The warm streaming server is released when `/voice` is turned off and on exit. ### Implementation notes - No CGO and no bundled runtimes — every OS integration shells out via `exec.Command` with discrete argv (never a shell string). - The streaming server manager mirrors the LSP manager's warm-subprocess lifecycle. - OS-boundary code (recorders, transcribers, server) is unit-tested via injected fakes; real microphone / websocket behavior is covered by env-gated smoke tests. ### Testing - `go build ./...`, `go vet ./...`, and the config / dictation / cli / tui suites pass. - Verified end-to-end against real downloaded binaries + models (offline batch, streaming websocket server, and the model auto-download), including the FireRedASR-CTC and NeMo streaming transducer families. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added speech-to-text dictation with both batch and streaming modes, supporting local (sherpa-onnx) and cloud providers, including WAV/M4A detection. * Introduced voice dictation mode with STT model selection, local engine/model auto-download, and an inline prompt to save cloud API keys. * Added new TUI commands for voice/STT model selection and enabled F1–F12 key bindings. * **Bug Fixes** * Improved streaming dictation to preserve accumulated transcript partials and keep the editor region consistent. * Refined TUI status/picker rendering, including clearer loading/search behavior and more reliable selection alignment. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Contributor
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR adds STT configuration, dictation runtime primitives, local/cloud/streaming transcribers, warm server and auto-download support, CLI wiring, and TUI flows for dictation, voice mode, model selection, downloads, and API-key prompts. ChangesSpeech-to-Text Dictation Feature
Estimated code review effort: 5 (Critical) | ~150 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (9)
internal/dictation/dictation.go (1)
26-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep one STT provider allowlist
BatchProviders()/StreamProviders()are labeled “for config validation,” butinternal/config/resolver.gostill validates with its own hardcoded switch. That leaves two lists to keep in sync; make one canonical source or drop the validation claim from these comments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/dictation/dictation.go` around lines 26 - 48, Make the STT provider allowlist use a single canonical source instead of duplicating validation logic: update the config validation in resolver.go to consume BatchProviders() and StreamProviders() from internal/dictation/dictation.go, or remove the “for config validation” claim from those helpers if resolver.go must remain the source of truth. Keep the provider constants and the BatchProviders()/StreamProviders() symbols as the authoritative names so future provider additions only need one update path.internal/dictation/transcriber_stream_test.go (1)
34-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead
gotAuth— the Deepgram auth-header contract is never actually asserted. The handler never readsAuthorization, sogotAuthstays empty and_ = gotAuthjust silences the unused-var error. Either capture and assert theToken <key>header inside the handler, or drop the variable to avoid implying coverage that doesn't exist.♻️ Assert the header instead of discarding it
url := wsTestServer(t, func(ctx context.Context, c *websocket.Conn) { defer c.Close(websocket.StatusNormalClosure, "")Note:
wsTestServerdoesn't currently expose the upgrade request headers to the handler, so assertingAuthorizationwould require threading*http.Request(or its headers) through the handler signature.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/dictation/transcriber_stream_test.go` around lines 34 - 55, The test currently leaves gotAuth unused and does not verify the Deepgram Authorization header, so the auth contract is untested. Update wsTestServer and the anonymous handler in transcriber_stream_test.go so the upgrade request headers (or *http.Request) are available, capture the Authorization value during NewDeepgramTranscriber handshake, and assert it equals the expected Token header; if you do not plan to expose headers, remove gotAuth and the dummy assignment instead of keeping dead coverage.internal/dictation/download_test.go (1)
101-150: 📐 Maintainability & Code Quality | 🔵 TrivialConsider adding a regression test for the "truncated-file-treated-as-installed" scenario.
Once the atomic-extraction fix from
download.golands, a test that writes a truncatedbin/sherpa-onnx-offlineintoengineDirbefore callingEnsureLocalEngine(asserting it does NOT get treated as installed) would guard against regressing back to the check-then-act idempotency bug.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/dictation/download_test.go` around lines 101 - 150, Add a regression test for the truncated-engine cache case in TestEnsureLocalEngineDownloadsAndExtracts or a nearby test helper: write a partial bin/sherpa-onnx-offline into the target engineDir before calling EnsureLocalEngine, then verify it is not treated as an already-installed engine and that the code re-downloads/re-extracts instead of returning the truncated binary. Use EnsureLocalEngine, DownloadOptions, and the existing fakeReleaseServer setup to keep the test aligned with the current download flow.internal/dictation/download.go (1)
216-249: 🚀 Performance & Scalability | 🔵 TrivialPagination heuristic relies on a cumulative modulo instead of the Link header.
The loop continues fetching while
len(assets)%100==0, which happens to work for the current release sizes (theasr-modelsrelease currently has 460 assets, confirmed via GitHub), but it's fragile: it's guessing "there might be more" from a cumulative count rather than reading the standardLink: rel="next"response header GitHub provides for paginated endpoints. FollowingLinkwould be more robust against future changes in how many assets the inline release payload embeds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/dictation/download.go` around lines 216 - 249, The asset pagination in ListModels is using a brittle len(assets)%100 heuristic instead of the GitHub pagination metadata. Update the pagination logic in ListModels to follow the response Link header for rel="next" when fetching release assets, rather than inferring more pages from the cumulative asset count. Keep the existing ListModels/getJSON flow, but have the asset-fetching loop continue only while a next-page link is present so future changes to inline asset counts do not break pagination.internal/config/stt_test.go (1)
1-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct test coverage for
SetSTTProvider.
SetSTTModelandSetSTTLocalEngineeach get dedicated persistence tests, butSetSTTProvider(writer.go Lines 255-277) has none here, despite being used to persist the "local" provider selection from the picker flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/config/stt_test.go` around lines 1 - 140, Add direct test coverage for SetSTTProvider in the STT config tests, since it currently has no persistence assertion even though it is used by the picker flow. Add a focused test alongside TestSetSTTModelPersists and TestSetSTTLocalEngineResetsStreamProvider that calls SetSTTProvider, then reloads the saved config with ValidateFile or ValidateBytes and verifies the STT provider is persisted as local (and any related stream/provider fields remain correct). Use the SetSTTProvider symbol to locate the implementation and keep the new test consistent with the existing writer.go persistence coverage.internal/config/writer.go (1)
179-277: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared config-load boilerplate.
SetSTTModel,SetSTTLocalEngine, andSetSTTProvidereach repeat the same 13-line "read file, unmarshal, treat ENOENT as empty" block already present inSetTheme. Worth factoring into one helper now that there are 4 copies.♻️ Suggested refactor
+func loadConfigOrEmpty(path string) (FileConfig, error) { + cfg := FileConfig{} + if data, err := os.ReadFile(path); err == nil { + if err := json.Unmarshal(data, &cfg); err != nil { + return FileConfig{}, fmt.Errorf("invalid config JSON %s: %w", path, err) + } + } else if !os.IsNotExist(err) { + return FileConfig{}, fmt.Errorf("read config %s: %w", path, err) + } + return cfg, nil +}Then each
SetSTT*function becomescfg, err := loadConfigOrEmpty(path)+ the field-specific mutation + validate + write.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/config/writer.go` around lines 179 - 277, The three STT writers duplicate the same “trim path, read config, unmarshal JSON, ignore missing file” boilerplate that already exists in SetTheme. Extract that repeated load logic into a shared helper (for example, alongside writeConfigFile) and update SetSTTModel, SetSTTLocalEngine, and SetSTTProvider to call it, then keep only their field-specific mutations, validation, and write steps.internal/tui/dictation_voice_test.go (1)
59-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing positive-case coverage for keyboard enhancement detection.
Only the unsupported (
Flags: 0) branch is tested. Since this flag determines hold-to-record vs. toggle-fallback for the whole voice-mode UX, add a case with the event-types bit set to asserteventTypesSupported == true.✅ Suggested additional test case
func TestKeyboardEnhancementsRecordedSupported(t *testing.T) { m := model{dictation: batchOnlyController()} m = m.handleKeyboardEnhancements(tea.KeyboardEnhancementsMsg{Flags: int(ansi.KittyReportEventTypes)}) if !m.dictation.eventTypesKnown || !m.dictation.eventTypesSupported { t.Error("event-types flag set should be known-and-supported") } }Please confirm the exact flag constant name (
ansi.KittyReportEventTypesor equivalent) used elsewhere in this codebase for constructing this message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/dictation_voice_test.go` around lines 59 - 66, Add positive-case coverage for keyboard enhancement detection in TestKeyboardEnhancementsRecorded by creating a second test (or extending the existing one) that calls model.handleKeyboardEnhancements with the event-types bit set and asserts dictation.eventTypesKnown and dictation.eventTypesSupported are both true. Use the same KeyboardEnhancementsMsg flow and the exact flag constant already used elsewhere for the event-types bit (for example ansi.KittyReportEventTypes or the project’s equivalent) so the test matches the production message construction.internal/tui/dictation_test.go (1)
216-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest mirrors the guard instead of exercising it.
transcribeBatchCtxduplicates the nil-check fromtranscribeBatchCmdrather than calling the real function; a regression in the actual guard wouldn't be caught by this test.If
transcribeBatchCmd's ctx-normalization can be extracted into a small exported-for-test helper (or the command itself tested via its returnedtea.Cmd), prefer that over a hand-duplicated copy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/dictation_test.go` around lines 216 - 229, The test currently duplicates the nil-guard logic instead of verifying the real behavior in transcribeBatchCmd, so a regression in the command would be missed. Refactor the ctx-normalization out of transcribeBatchCmd into a small helper that the test can call directly, or change TestDictationBatchCtxNonNil to exercise the command’s returned tea.Cmd path. Remove the hand-copied transcribeBatchCtx helper and keep the assertion tied to the actual implementation symbols transcribeBatchCmd and dictationController.internal/tui/dictation_download_test.go (1)
24-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for the two behavioral gaps flagged in
dictation_download.go/stt_model_picker.go.Once fixed, add tests for: (1) selecting a second download variant while
m.dictation.downloadingis true is a no-op, and (2) opening the local-model download picker then cancelling (Esc) does not persistProvider=localover a previously-working cloud provider — mirroringTestCloudKeyPromptCancelKeepsPreviousProvider.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/dictation_download_test.go` around lines 24 - 42, Add regression tests covering the two dictation download picker gaps in `model.toggleDictation`, `dictation_download.go`, and `stt_model_picker.go`. Add one test that verifies choosing a second download variant while `m.dictation.downloading` is already true is ignored and leaves the current state unchanged. Add another test that opens the local-model download picker, cancels with Esc, and confirms `Provider=local` is not persisted over an existing working cloud provider, mirroring the existing cloud cancel behavior test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/config/resolver.go`:
- Around line 797-823: validateSTTConfig currently validates provider,
streamProvider, maxDurationSeconds, and localServerPort but misses NumThreads,
allowing negative values to slip through. Update validateSTTConfig in STTConfig
to add a bounds check for cfg.NumThreads and return a clear fmt.Errorf when it
is negative, matching the existing startup-validation style and keeping the
error message specific to stt.numThreads.
In `@internal/dictation/download.go`:
- Around line 454-555: EnsureLocalEngine currently trusts fileExists(binPath)
after extraction, but a partial extract can leave a truncated engine binary in
the final directory and falsely look installed. Update the install flow around
downloadVerifyExtract and resolveEnginePaths so extraction happens in a
temporary staging directory first, then atomically move or rename the completed
tree into engineDir only after the full extraction succeeds. Make sure the final
presence check in EnsureLocalEngine only observes the staged-then-promoted
result, not partially written files.
In `@internal/dictation/runner.go`:
- Around line 48-70: The startProcess function leaves pipe file descriptors open
if cmd.Start fails after StdoutPipe or StdinPipe has already allocated them.
Update startProcess to track any opened stdout/stdin pipe ends and close them
before returning the start error, using the existing cmd, stdout, and stdin
handling to ensure no leaked descriptors remain on failure.
In `@internal/dictation/server.go`:
- Around line 136-149: The Shutdown method in ServerManager ignores its ctx
argument, so it can block past the caller’s deadline. Update Shutdown(ctx
context.Context) to honor ctx by wiring it into the graceful-stop/wait path, or
by deriving the timeout from ctx and using it instead of the fixed stopGrace in
proc.StopGracefully and waitWithTimeout. Keep the behavior in
ServerManager.Shutdown consistent with the caller’s shutdown budget while
preserving the existing proc lifecycle handling.
In `@internal/dictation/spill.go`:
- Around line 24-42: The audioSpillDir() path only verifies ownership after
MkdirAll, so an existing spill directory can keep loose permissions. Update
audioSpillDir() to also inspect and enforce the directory mode after os.Lstat,
tightening any pre-existing directory to 0o700 or returning an error if it is
not sufficiently restricted. Keep the check alongside checkAudioDirOwner so both
ownership and permissions are validated for the existing audio spill directory.
In `@internal/tui/dictation_download.go`:
- Around line 41-70: The dictation download flow currently allows a second
download to start while one is already in progress. Add a guard in
startVariantDownload and handleSTTDownloadSelection to check the existing
downloading state before calling EnsureLocalEngine, and short-circuit with a
user-facing notice or no-op if a download is active. Keep the existing
canOfferDownload behavior consistent by reusing the same m.dictation.downloading
/ d.downloading check so reopening the model picker or retrying cannot trigger
overlapping downloads against the same destination.
In `@internal/tui/stt_model_picker.go`:
- Around line 117-143: The local STT picker flow in maybeOpenSTTDownloadPicker
commits config too early by setting Provider to local and persisting it before
the user selects a model, which breaks the cancel-preserves-provider behavior.
Move the provider commit to the successful-selection path by relying on
applyEngineComponents as the only place that applies the local provider/model,
matching the cloud flow’s commit-on-success pattern. Also stop discarding the
SetSTTProvider error in maybeOpenSTTDownloadPicker and handle or surface it
consistently with the rest of the file.
- Around line 263-265: The isCurrent helper in stt_model_picker.go is matching
currentPath with strings.Contains, which can incorrectly mark a variant as
current when DirName is only a substring of another path segment. Update
isCurrent to compare against path segments or a normalized path boundary using
filepath so it only returns true when currentPath actually contains the exact
variant DirName, and keep the check localized to the ModelVariant row selection
logic.
- Around line 162-169: The STT model lookup in fetchSTTModelsCmd uses
context.Background(), so a stalled GitHub request can hang the picker
indefinitely. Update fetchSTTModelsCmd to create and pass a context with timeout
into dictation.ListModels, and make sure the timeout is long enough for the
GitHub round-trip but still guarantees the tea.Cmd returns with an error if the
request stalls.
---
Nitpick comments:
In `@internal/config/stt_test.go`:
- Around line 1-140: Add direct test coverage for SetSTTProvider in the STT
config tests, since it currently has no persistence assertion even though it is
used by the picker flow. Add a focused test alongside TestSetSTTModelPersists
and TestSetSTTLocalEngineResetsStreamProvider that calls SetSTTProvider, then
reloads the saved config with ValidateFile or ValidateBytes and verifies the STT
provider is persisted as local (and any related stream/provider fields remain
correct). Use the SetSTTProvider symbol to locate the implementation and keep
the new test consistent with the existing writer.go persistence coverage.
In `@internal/config/writer.go`:
- Around line 179-277: The three STT writers duplicate the same “trim path, read
config, unmarshal JSON, ignore missing file” boilerplate that already exists in
SetTheme. Extract that repeated load logic into a shared helper (for example,
alongside writeConfigFile) and update SetSTTModel, SetSTTLocalEngine, and
SetSTTProvider to call it, then keep only their field-specific mutations,
validation, and write steps.
In `@internal/dictation/dictation.go`:
- Around line 26-48: Make the STT provider allowlist use a single canonical
source instead of duplicating validation logic: update the config validation in
resolver.go to consume BatchProviders() and StreamProviders() from
internal/dictation/dictation.go, or remove the “for config validation” claim
from those helpers if resolver.go must remain the source of truth. Keep the
provider constants and the BatchProviders()/StreamProviders() symbols as the
authoritative names so future provider additions only need one update path.
In `@internal/dictation/download_test.go`:
- Around line 101-150: Add a regression test for the truncated-engine cache case
in TestEnsureLocalEngineDownloadsAndExtracts or a nearby test helper: write a
partial bin/sherpa-onnx-offline into the target engineDir before calling
EnsureLocalEngine, then verify it is not treated as an already-installed engine
and that the code re-downloads/re-extracts instead of returning the truncated
binary. Use EnsureLocalEngine, DownloadOptions, and the existing
fakeReleaseServer setup to keep the test aligned with the current download flow.
In `@internal/dictation/download.go`:
- Around line 216-249: The asset pagination in ListModels is using a brittle
len(assets)%100 heuristic instead of the GitHub pagination metadata. Update the
pagination logic in ListModels to follow the response Link header for rel="next"
when fetching release assets, rather than inferring more pages from the
cumulative asset count. Keep the existing ListModels/getJSON flow, but have the
asset-fetching loop continue only while a next-page link is present so future
changes to inline asset counts do not break pagination.
In `@internal/dictation/transcriber_stream_test.go`:
- Around line 34-55: The test currently leaves gotAuth unused and does not
verify the Deepgram Authorization header, so the auth contract is untested.
Update wsTestServer and the anonymous handler in transcriber_stream_test.go so
the upgrade request headers (or *http.Request) are available, capture the
Authorization value during NewDeepgramTranscriber handshake, and assert it
equals the expected Token header; if you do not plan to expose headers, remove
gotAuth and the dummy assignment instead of keeping dead coverage.
In `@internal/tui/dictation_download_test.go`:
- Around line 24-42: Add regression tests covering the two dictation download
picker gaps in `model.toggleDictation`, `dictation_download.go`, and
`stt_model_picker.go`. Add one test that verifies choosing a second download
variant while `m.dictation.downloading` is already true is ignored and leaves
the current state unchanged. Add another test that opens the local-model
download picker, cancels with Esc, and confirms `Provider=local` is not
persisted over an existing working cloud provider, mirroring the existing cloud
cancel behavior test.
In `@internal/tui/dictation_test.go`:
- Around line 216-229: The test currently duplicates the nil-guard logic instead
of verifying the real behavior in transcribeBatchCmd, so a regression in the
command would be missed. Refactor the ctx-normalization out of
transcribeBatchCmd into a small helper that the test can call directly, or
change TestDictationBatchCtxNonNil to exercise the command’s returned tea.Cmd
path. Remove the hand-copied transcribeBatchCtx helper and keep the assertion
tied to the actual implementation symbols transcribeBatchCmd and
dictationController.
In `@internal/tui/dictation_voice_test.go`:
- Around line 59-66: Add positive-case coverage for keyboard enhancement
detection in TestKeyboardEnhancementsRecorded by creating a second test (or
extending the existing one) that calls model.handleKeyboardEnhancements with the
event-types bit set and asserts dictation.eventTypesKnown and
dictation.eventTypesSupported are both true. Use the same
KeyboardEnhancementsMsg flow and the exact flag constant already used elsewhere
for the event-types bit (for example ansi.KittyReportEventTypes or the project’s
equivalent) so the test matches the production message construction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d906cbf6-dcc6-4672-aec8-e65a58046570
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (54)
go.modinternal/cli/app.gointernal/cli/dictation.gointernal/cli/dictation_test.gointernal/config/resolver.gointernal/config/stt_test.gointernal/config/types.gointernal/config/validate.gointernal/config/writer.gointernal/dictation/dictation.gointernal/dictation/download.gointernal/dictation/download_smoke_test.gointernal/dictation/download_test.gointernal/dictation/listmodels_test.gointernal/dictation/local_smoke_test.gointernal/dictation/owner_unix.gointernal/dictation/owner_windows.gointernal/dictation/recorder.gointernal/dictation/recorder_test.gointernal/dictation/runner.gointernal/dictation/server.gointernal/dictation/server_test.gointernal/dictation/spill.gointernal/dictation/stream_smoke_test.gointernal/dictation/transcriber.gointernal/dictation/transcriber_cloud.gointernal/dictation/transcriber_cloud_test.gointernal/dictation/transcriber_deepgram.gointernal/dictation/transcriber_local.gointernal/dictation/transcriber_local_stream.gointernal/dictation/transcriber_local_test.gointernal/dictation/transcriber_openai_realtime.gointernal/dictation/transcriber_stream_test.gointernal/providermodelcatalog/filter.gointernal/providermodelcatalog/stt_filter_test.gointernal/tui/autocomplete_test.gointernal/tui/commands.gointernal/tui/dictation.gointernal/tui/dictation_download.gointernal/tui/dictation_download_test.gointernal/tui/dictation_stream.gointernal/tui/dictation_test.gointernal/tui/dictation_voice.gointernal/tui/dictation_voice_test.gointernal/tui/keybindings.gointernal/tui/model.gointernal/tui/mouse.gointernal/tui/mouse_test.gointernal/tui/options.gointernal/tui/picker.gointernal/tui/stt_key_prompt.gointernal/tui/stt_key_prompt_test.gointernal/tui/stt_model_picker.gointernal/tui/view.go
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
Contributor
|
nice feature this is really useful! kindly address some coderabbit comments @anandh8x |
- validateSTTConfig: add NumThreads bounds check - downloadVerifyExtract: stage extraction into a sibling temp dir, atomic-rename into destDir on success so a partial extract cannot leave a truncated engine binary that passes fileExists() on next run - startProcess: close pipe fds when cmd.Start() fails - ServerManager.Shutdown: honor the caller's context deadline - audioSpillDir: tighten any pre-existing directory to 0700 - startVariantDownload: refuse re-entry while a download is in flight - maybeOpenSTTDownloadPicker: defer Provider=local persistence to applyEngineComponents (commit-on-success), matching the cloud flow - fetchSTTModelsCmd: bound with a 30s timeout client+context - newSTTDownloadPickerFrom.isCurrent: compare path segments via filepath.Base to avoid substring false-positives
Contributor
|
Hey folks, I took a look at this PR and noticed a few correctness issues with the dictation features: 1. Nil Pointer Panic in
|
…region Three correctness issues from PR review: 1) recorder.Stop() panicked (nil-deref) when called on a StartStreaming session because StartStreaming intentionally leaves r.proc nil. Reject that misuse with a clear error. Termux's batch path also leaves r.proc nil by design — keep that path working. 2) Esc was consumed by an active dictation session before the confirming second Esc could cancel a pending run, so a user mid-recording who double-Esc's to kill the run found the first Esc swallowed by dictation. Gate dictation-cancel on the Esc not being a pending-run confirm. 3) applyStreamingText hard-reset the cursor to regionStart on every partial, yanking focus away from users editing elsewhere in the composer, and stored regionStart/End as absolute indices that became stale the moment the user typed or pasted before the live region. Preserve the cursor when it's outside the region, and snapshot the prefix BEFORE the region so the next partial can detect external edits and shift [start,end) to stay aligned.
yanalialiuk
pushed a commit
to yanalialiuk/zero
that referenced
this pull request
Jul 8, 2026
* ## Speech-to-text dictation Adds voice dictation to Zero. Run `/voice` to enter hold-to-record mode, then hold **Space** to dictate and release to transcribe (run `/voice` again to turn it off so Space types normally). Pick a backend with `/stt-model`. ### Backends - **Local (sherpa-onnx)** — offline, no API key. Auto-downloads a model via `/stt-model`; the engine binary is fetched once and shared across models. Family auto-detection covers the mainstream offline families (Whisper, transducer/Zipformer, Moonshine, Paraformer, SenseVoice, and the CTC families FireRedASR, NeMo, TeleSpeech, WeNet, Zipformer-CTC, Dolphin, Omnilingual — plus the encoder/decoder families Canary and FireRedASR-AED). - **Cloud batch** — Groq and OpenAI Whisper. Keys resolve from a configured provider, the encrypted credential store, or the env var; a missing/invalid key is entered inline. - **Streaming** — a warm local sherpa-onnx websocket server (live transcript as you speak) on desktop, or Deepgram / OpenAI Realtime. ### Highlights - Live audio-reactive waveform while recording (real mic levels when streaming; a synthesized wave for batch). - `/stt-model` download chooser: recommended models first, Live/Batch tag per row, size + download state, and a search filter. - Streaming transcript is inserted live into the composer and committed on stop; batch inserts on stop. Insert-for-review by default (`stt.autoSubmit` to fire straight at the agent). - The warm streaming server is released when `/voice` is turned off and on exit. ### Implementation notes - No CGO and no bundled runtimes — every OS integration shells out via `exec.Command` with discrete argv (never a shell string). - The streaming server manager mirrors the LSP manager's warm-subprocess lifecycle. - OS-boundary code (recorders, transcribers, server) is unit-tested via injected fakes; real microphone / websocket behavior is covered by env-gated smoke tests. ### Testing - `go build ./...`, `go vet ./...`, and the config / dictation / cli / tui suites pass. - Verified end-to-end against real downloaded binaries + models (offline batch, streaming websocket server, and the model auto-download), including the FireRedASR-CTC and NeMo streaming transducer families. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added speech-to-text dictation with both batch and streaming modes, supporting local (sherpa-onnx) and cloud providers, including WAV/M4A detection. * Introduced voice dictation mode with STT model selection, local engine/model auto-download, and an inline prompt to save cloud API keys. * Added new TUI commands for voice/STT model selection and enabled F1–F12 key bindings. * **Bug Fixes** * Improved streaming dictation to preserve accumulated transcript partials and keep the editor region consistent. * Refined TUI status/picker rendering, including clearer loading/search behavior and more reliable selection alignment. <!-- end of auto-generated comment: release notes by coderabbit.ai --> * fix(dictation): address CodeRabbit review feedback - validateSTTConfig: add NumThreads bounds check - downloadVerifyExtract: stage extraction into a sibling temp dir, atomic-rename into destDir on success so a partial extract cannot leave a truncated engine binary that passes fileExists() on next run - startProcess: close pipe fds when cmd.Start() fails - ServerManager.Shutdown: honor the caller's context deadline - audioSpillDir: tighten any pre-existing directory to 0700 - startVariantDownload: refuse re-entry while a download is in flight - maybeOpenSTTDownloadPicker: defer Provider=local persistence to applyEngineComponents (commit-on-success), matching the cloud flow - fetchSTTModelsCmd: bound with a 30s timeout client+context - newSTTDownloadPickerFrom.isCurrent: compare path segments via filepath.Base to avoid substring false-positives * fix(dictation): harden recorder.Stop, Esc-cancel priority, streaming region Three correctness issues from PR review: 1) recorder.Stop() panicked (nil-deref) when called on a StartStreaming session because StartStreaming intentionally leaves r.proc nil. Reject that misuse with a clear error. Termux's batch path also leaves r.proc nil by design — keep that path working. 2) Esc was consumed by an active dictation session before the confirming second Esc could cancel a pending run, so a user mid-recording who double-Esc's to kill the run found the first Esc swallowed by dictation. Gate dictation-cancel on the Esc not being a pending-run confirm. 3) applyStreamingText hard-reset the cursor to regionStart on every partial, yanking focus away from users editing elsewhere in the composer, and stored regionStart/End as absolute indices that became stale the moment the user typed or pasted before the live region. Preserve the cursor when it's outside the region, and snapshot the prefix BEFORE the region so the next partial can detect external edits and shift [start,end) to stay aligned.
5 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
4 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.
Speech-to-text dictation
Adds voice dictation to Zero. Run
/voiceto enter hold-to-record mode, then hold Space to dictate and release to transcribe (run/voiceagain to turn it off so Space types normally). Pick a backend with/stt-model.Backends
/stt-model; the engine binary is fetched once and shared across models. Family auto-detection covers the mainstream offline families (Whisper, transducer/Zipformer, Moonshine, Paraformer, SenseVoice, and the CTC families FireRedASR, NeMo, TeleSpeech, WeNet, Zipformer-CTC, Dolphin, Omnilingual — plus the encoder/decoder families Canary and FireRedASR-AED).Highlights
/stt-modeldownload chooser: recommended models first, Live/Batch tag per row, size + download state, and a search filter.stt.autoSubmitto fire straight at the agent)./voiceis turned off and on exit.Implementation notes
exec.Commandwith discrete argv (never a shell string).Testing
go build ./...,go vet ./..., and the config / dictation / cli / tui suites pass.Summary by CodeRabbit