Repository navigation
fix(orcad): voice dictation on headless orca serve hosts - #31217
Conversation
|
Review status for head 5f7f8a5
Follow-ups to file:
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 35.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 34 functions across 45 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: restoring voice dictation for headless Orca serve hosts. Description check The description is complete and follows the required template. It explains the user impact, implementation, rationale, linked issue, testing, platform coverage, follow-ups, and checklist status. Linked Issues check Issue [ #31174] requires mobile voice dictation on headless Linuxorca servehosts. The PR wires speech services intoorcad, packagesstt-worker.js, discovers the installed Sherpa recognizer, and…Out of Scope Changes check The changes support [ #31174]. Node download adapters, retry handling, recognizer discovery, worker packaging, key-readability checks, platform handling, install-completeness checks, and mobile error m…Full details: Linked Issues check
-
Explanation
Issue [
#31174] requires mobile voice dictation on headless Linuxorca servehosts. The PR wires speech services intoorcad, packagesstt-worker.js, discovers the installed Sherpa recognizer, and passes its directory toorcad. The PR adds model listing, downloads, dictation startup, compatibility handling, and automated tests for these paths.
Full details: Out of Scope Changes check
-
Explanation
The changes support [
#31174]. Node download adapters, retry handling, recognizer discovery, worker packaging, key-readability checks, platform handling, install-completeness checks, and mobile error mapping enable or protect server-side dictation. Windows, macOS, cloud-only, and no-recognizer behavior remain within the same speech feature scope. No unrelated change is demonstrated.
-
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
bb7c302c-33da-494c-90e4-c5d2901145e3
📒 Files selected for processing: 38
config/scripts/orcad-entry-build.mjsmobile/src/dictation/mobile-dictation-setup.test.tsmobile/src/dictation/mobile-dictation-setup.tsmobile/src/session/use-mobile-session-native-chat-dictation.tssrc/cli/runtime/launch-serve-runtime.test.tssrc/cli/runtime/launch.tssrc/cli/runtime/serve-orcad-launch.tssrc/cli/runtime/serve-speech-recognizer.test.tssrc/cli/runtime/serve-speech-recognizer.tssrc/main/host/electron-speech-services.tssrc/main/ipc/speech-worker-exit-lifetime.test.tssrc/main/network/transient-download-error.test.tssrc/main/network/transient-download-error.tssrc/main/orcad/orcad-host-adapters.tssrc/main/orcad/orcad-speech-services.test.tssrc/main/orcad/orcad-speech-services.tssrc/main/runtime/construction/runtime-composition-parity.test.tssrc/main/runtime/runtime-mobile-dictation-controller.tssrc/main/runtime/runtime-mobile-speech-catalog.tssrc/main/speech/model-manager-download-error.test.tssrc/main/speech/model-manager-download-resume.test.tssrc/main/speech/model-manager-progress-callback.test.tssrc/main/speech/model-manager-stream-cleanup.test.tssrc/main/speech/model-manager-windows-path.test.tssrc/main/speech/model-manager.test.tssrc/main/speech/model-manager.tssrc/main/speech/node-speech-download-request.test.tssrc/main/speech/node-speech-download-request.tssrc/main/speech/openai-api-key-store.test.tssrc/main/speech/openai-api-key-store.tssrc/main/speech/speech-model-download-response.tssrc/main/speech/speech-model-http-download.tssrc/main/speech/speech-runtime-service.tssrc/main/speech/stt-session-start.tssrc/main/speech/stt-worker-paths.test.tssrc/main/speech/stt-worker-paths.tssrc/shared/orcad-artifacts.tssrc/shared/speech-recognizer-package.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
| const roots: string[] = [] | ||
| afterEach(() => { | ||
| delete process.env[SPEECH_RECOGNIZER_DIR_ENV] | ||
| for (const root of roots.splice(0)) { | ||
| rmSync(root, { recursive: true, force: true }) | ||
| } | ||
| }) | ||
|
|
||
| // orcad: a packaged host with no Electron resources tree. | ||
| describe('speech worker paths on orcad (#31174)', () => { | ||
| it('has no on-device recognizer unless orca serve names one', () => { | ||
| installFakeAppEnvironment({ isPackaged: () => true }) | ||
| expect(getSherpaModulePath()).toBeNull() | ||
| expect(hasLocalSpeechRecognizer()).toBe(false) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear ORCA_SPEECH_RECOGNIZER_DIR before each test.
The suite deletes SPEECH_RECOGNIZER_DIR_ENV only in afterEach. The first test therefore uses the value it inherits from the parent process. serveWithOrcad always sets ORCA_SPEECH_RECOGNIZER_DIR on the orcad process. The comment in src/cli/runtime/serve-orcad-launch.ts says an Orca terminal can pass this value on to child processes. A developer can run vitest in a shell that orcad hosts. In that shell, the variable points to the installed app's real recognizer, which contains sherpa-onnx.node. getSherpaModulePath() then returns that directory, and the assertions on Line 21 and Line 22 fail.
💚 Proposed fix
-import { afterEach, describe, expect, it } from 'vitest'
+import { afterEach, beforeEach, describe, expect, it } from 'vitest'
...
const roots: string[] = []
+beforeEach(() => {
+ delete process.env[SPEECH_RECOGNIZER_DIR_ENV]
+})
afterEach(() => {
514bdbc to
b4368e6
Compare
Mobile dictation against an orca serve host answered speech_unavailable since serve moved to orcad: orcad never installed the speech service factories, and the model manager and download imported Electron. The model download now takes a host request opener (Chromium net on the desktop, Node https on orcad, with redirects mapped to the same event), the STT worker ships in the orcad slot, and orca serve hands orcad the installed app's on-device recognizer. A host without a recognizer (an SSH or WSL managed slot) lists only cloud models and sends a phone that picked an on-device model to the setup sheet. Mobile names the fix when an older server still has no speech at all. The parity test now requires both roots to install speech. Reported-by: delorean2020 (@delorean2020)
…s, review fixes - Node https errors (ECONNRESET, ETIMEDOUT, ...) are retryable, so an interrupted orcad model download resumes (Codex P2). - On Windows orca serve hands orcad a per-version copy of the recognizer in the profile, so a loaded addon never locks the app's files against an update (Opus). - Drop a recognizer candidate path that could never match; one SAFETY helper per test file instead of repeated casts; comments.
…pt; prune superseded recognizer copies Codex round 2: a desktop-sealed OpenAI key exists on disk but orcad's secret store cannot open it, so the catalog reported cloud models ready and dictation failed at the end. orcad's model manager now proves the key decrypts. The desktop keeps its existence check (no keychain prompt).
…opy linked packages as real files Codex round 3. ref-oss check: versioned copies are cleaned only by the process that owns them, so the pre-lock prune is dropped rather than patched.
…key (Codex round 5)
…loud dictation only Design check after review round 5: every remaining finding came from the Windows copy that dodges addon file locks. Dropping it shrinks the surface to a path lookup; Linux and macOS serve keep on-device models.
…s set the recognizer env; retry ENOTFOUND (final Opus review)
CI: shipping stt-worker.js added one more absolute slot path to the install-complete probe, pushing it further past the exec-size budget. Testing each artifact relative to the slot (inside a subshell cd) cuts the probe from ~12.2 KB to ~3.6 KB, so it drops off the over-budget list instead of raising a ceiling.
… of which zod copy resolves
With mobile's own dependencies installed (which CI does for any PR that
touches mobile/), the e2e program resolves two zod copies, and the
salvaged platform arm from src/shared inferred as {}. The schema now
narrows through an explicit platform guard.
…nt; narrow in a second step Mobile verify: the recorded-RPC mutant anchors on the read line, which the previous guard rewrote, so the mutant could not apply.
…platform guard now zod is aligned launch.ts went one line past max-lines with main; resolveAppRoot, resolveForegroundOrcaExecutable, getExecutableAppArgs and stripElectronRunAsNode move to orca-app-executable.ts (re-exported, so callers are unchanged). With mobile and root on one zod (#31260), the host.platform schema no longer needs the extra narrowing step.
4d67328 to
f32b1da
Compare
…-exports from launch.ts
ELI5
Voice dictation from Orca Mobile stopped working on headless
orca servehosts onceservemoved to the separate Node server (orcad). That server never switched on speech, so every voice request answered "this host has no speech services". With this PR the server handles dictation again. It uses the speech recognizer the installed Orca app already ships, and it keeps downloaded voice models in the same place as before.What Changed
Before: on an
orca servehost (Linux .deb, 1.4.229 and later), the mobile Voice screen showsspeech_unavailable: this host has no speech services.After:
orca servefrom an installed app on Linux or macOS: the mobile Voice screen lists models again. Models download on the server, and dictation works with both on-device and cloud models.orca serveon Windows: cloud models only. A loaded native module locks its files for as long as the server runs, which would make the next app update fail.How it works:
appandnet, so the Node server couldn't build them. The download now asks its host to open a request.host/electron-speech-services.ts).https(speech/node-speech-download-request.ts). That adapter reports a redirect the same way Electron does, and keeps listening for errors after an abort so a late socket error can't crash the server.network/transient-download-error.ts) now also recognizes Node's error codes (ECONNRESET,ETIMEDOUTand so on), so an interrupted download on orcad resumes instead of starting over.orcad/orcad-speech-services.ts), and the dictation worker (stt-worker.js) now ships in the server package.orca servefinds the recognizer bundled with the installed app (resources/node_modules/sherpa-onnx-<platform>-<arch>) and passes its folder to orcad asORCA_SPEECH_RECOGNIZER_DIR. The recognizer is a Node-API module, so the server's own Node build loads it as-is. Without that folder,getSherpaModulePath()returns null, the setup screen hides on-device models, and starting dictation answersvoice_model_not_ready:unavailable.Why
Run speech on the server, not advertise it as unsupported. The reporter's dictation worked on 1.4.205. Their machine already has the recognizer, and the server already has their models in its profile. Calling dictation unsupported would have taken away a feature that used to work.
Borrow the app's recognizer instead of shipping one in the server package. The recognizer is about 70 MB per platform. Managed SSH and WSL servers download the server package over the network, so adding it there would grow every one of those installs.
orca servealready hands orcad the app's browser client the same way, so this follows an existing pattern. Those managed servers still get cloud dictation.Hide what can't run, rather than adding a new model status. Older phones only know a fixed set of model statuses, and an unknown one would show up as a model they can pick. Leaving the model out of the list, and reusing the
voice_model_not_ready:error that every phone build already sends to the setup sheet, works with both older and newer phones and needs no protocol change.One shared download, with the host choosing the network stack. The download keeps its resume, retry and validation logic in one place. A separate Node-only downloader would have duplicated about 300 lines of tested code.
Linked Issue
Fixes #31174
Thanks to @delorean2020 for the report and for pointing at the missing service provider and the shipped recognizer module.
Builds on #31197 (merged), which extended the parity check this PR relies on.
Visual Proof
N/A. The server now answers these requests; no screen layout changed. The new mobile message is only a different error string. The live server checks are listed below.
Testing
Verified live on macOS arm64, using a built orcad package (
pnpm build:orcad) with its own HOME and user-data folder under/tmp.With the app's recognizer folder passed in:
speech.models.listreturned the full catalog. On main, the same call answersspeech_unavailable.speech.models.download whisper-tinydownloaded about 150 MB over Nodehttps, through the release redirects, and the model reportedready.out/orcad/stt-worker.js, running on plain Node with that recognizer, loaded the downloaded model. It answeredready, thenfinal, thenstoppedon a second of audio.Without the recognizer folder:
speech.models.listlisted only the two OpenAI models.whisper-tinywas refused withvoice_model_not_downloadable.Covered by tests only:
orca servefinding the recognizer inside a real .deb install. Covered with temporary folder layouts.Unit tests added:
stt-worker-paths.test.ts: how orcad resolves the recognizer and the worker.node-speech-download-request.test.ts: redirects, responses, and a late error after an abort.orcad-speech-services.test.ts: the model list, downloads and dictation start on a server with no recognizer.transient-download-error.test.ts: Node errors that should and shouldn't be retried.serve-orcad-launch.test.ts.speech_unavailablemessage.Gates run locally: the full static job (lint, format, anti-slop, code quality,
pnpm tc, ratchets, generated catalogs),pnpm --filter mobiledictation tests, and mobiletsc.Review
Codex (
gpt-6.1-sol, high)Independent Opus review:
Round 1 asked for the Windows file-lock handling.
Round 2 was clean.
The final pass found nothing blocking. I adopted three of its suggestions:
ENOTFOUNDIt also noted that a cloud key saved on the desktop can't be read by orcad (see the follow-ups).
Design check (ref-oss, after round 3; decided after round 5): I compared this with putting a "speech unsupported on headless hosts" capability in front of the mic. I kept speech running in orcad:
Every late finding came from one side mechanism: copying the recognizer on Windows so the app's files stay unlocked. Comparable servers keep versioned copies that only their owning process cleans up. Rather than build that here, I removed the copy entirely. Windows orcad gets cloud dictation, and the launcher now only looks up a path.
Follow-ups:
Agent skill upstream boundary
Notes
httpsignores proxy settings, so a server behind a proxy can't download models. The desktop path is unchanged.require.resolve.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)