Repository navigation
Add live dictation and Soniox provider bypass - #66
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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:
📝 WalkthroughWalkthroughAdds Live Dictation and Soniox bypass concepts, extends configuration and loading for Soniox API keys and live dictation settings, and wires the daemon, providers, IPC, and tests around both recording modes. ChangesLive Dictation and Soniox Bypass
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 5
🧹 Nitpick comments (6)
docs/ISSUES-LIVE-DICTATION-SONIOX.md (1)
99-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor grammar: hyphenate compound modifier.
"scratch focused input" should be "scratch focused-input" for clarity.
🤖 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 `@docs/ISSUES-LIVE-DICTATION-SONIOX.md` at line 99, The checklist item in the live dictation notes uses an unhyphenated compound modifier; update the wording to hyphenate the phrase in the “Live Dictation” bullet so the description reads with “scratch focused-input” for clarity.tests/config.test.ts (1)
53-56: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd assertion for
liveDictation.soniox.triggerKeydefault.The test verifies other live dictation defaults but omits
triggerKey. Add:expect(config.liveDictation.soniox.triggerKey).toBe("Right Alt");🤖 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 `@tests/config.test.ts` around lines 53 - 56, The config default test for live dictation is missing coverage for the soniox trigger key. In the config test around the existing config.liveDictation assertions, add an expectation for config.liveDictation.soniox.triggerKey to verify it defaults to "Right Alt" alongside the other defaults.src/config/schema.ts (1)
237-240: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSoniox API key validation message could be clearer.
.min(1, { message: "Soniox API key is too short" })validates non-emptiness, but "too short" implies a length requirement. Consider"Soniox API key must not be empty"or adding a realistic minimum length if the key format is known.🤖 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 `@src/config/schema.ts` around lines 237 - 240, The Soniox API key validation in the schema uses .min(1) with a message that implies a length requirement, so update the validation on the soniox field to use clearer wording or a real minimum length if the key format is known. Adjust the message in the z.string() chain for soniox so it reflects non-empty input, or replace the current .min(1, ...) check with an appropriate length rule tied to the Soniox key format.src/transcribe/deepgram-streaming.ts (1)
130-147: 🎯 Functional Correctness | 🔵 TrivialDuplicated event construction;
data.is_final/data.speech_finalmay beundefinedviolating strict TypeScript types.
LiveTranscriptEvent.isFinalandspeechFinalare strictly typed asbooleaninsrc/transcribe/live-provider.ts. However, Deepgram's payload propertiesdata.is_finalanddata.speech_finalcan beundefined. The current code assigns these optional values directly to strict boolean fields inside the conditional blocks where only one is guaranteed to be truthy, leaving the other potentiallyundefined. Additionally, the event construction logic is duplicated in both branches.Refactor to deduplicate the logic and explicitly coerce values to
booleanusingBoolean(...)to satisfy strict type checking.♻️ Suggested dedup + boolean normalization
- if (data.speech_final) { - this.transcriptChunks.push(transcript.trim()); - logger.debug( - { transcript: transcript.trim(), isFinal: true }, - "Deepgram chunk finalized (speech_final)", - ); - const event: LiveTranscriptEvent = { - text: transcript.trim(), - isFinal: data.is_final, - speechFinal: data.speech_final, - }; - this.emit("transcript", transcript.trim(), event); - } else if (data.is_final) { - this.transcriptChunks.push(transcript.trim()); - logger.debug( - { transcript: transcript.trim(), isFinal: data.is_final }, - "Deepgram chunk finalized (is_final)", - ); - const event: LiveTranscriptEvent = { - text: transcript.trim(), - isFinal: data.is_final, - speechFinal: data.speech_final, - }; - this.emit("transcript", transcript.trim(), event); - } + if (data.speech_final || data.is_final) { + const trimmed = transcript.trim(); + this.transcriptChunks.push(trimmed); + const event: LiveTranscriptEvent = { + text: trimmed, + isFinal: Boolean(data.is_final), + speechFinal: Boolean(data.speech_final), + }; + logger.debug( + { transcript: trimmed, isFinal: event.isFinal, speechFinal: event.speechFinal }, + "Deepgram chunk finalized", + ); + this.emit("transcript", trimmed, event); + }🤖 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 `@src/transcribe/deepgram-streaming.ts` around lines 130 - 147, The Deepgram transcript handling in deepgram-streaming.ts duplicates LiveTranscriptEvent निर्माण in both branches and assigns optional payload fields directly to strictly typed booleans. Refactor the transcript emission logic around the existing transcript processing block to build the event once, and explicitly coerce data.is_final and data.speech_final with Boolean(...) before assigning them to LiveTranscriptEvent.isFinal and speechFinal so the types stay strict and the duplication is removed.src/transcribe/soniox-streaming.ts (2)
217-228: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
waitForClosemisses an already-fired close, inflatingcloseWaitMs.If the socket closes (server-side or via earlier error) before
stop()runs, theonce("close")listener never fires and the promise only resolves via the 2000ms timeout, reporting a spuriousfinalize_transcript+close_timeoutand a full-timeoutcloseWaitMs. Short-circuit when the connection is already closed (e.g. checkreadyState/isConnected) before awaiting.🤖 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 `@src/transcribe/soniox-streaming.ts` around lines 217 - 228, `waitForClose` in `SonioxStreaming` only waits for a future `close` event, so if the socket has already closed before `stop()` calls it, the promise times out and inflates `closeWaitMs`. Update `waitForClose()` to short-circuit when the connection is already closed by checking the stream/socket state first (for example via an existing `readyState` or `isConnected`-style flag used in `SonioxStreaming`) and resolve immediately instead of registering `once("close")` in that case.
71-73: 🩺 Stability & Availability | 🔵 TrivialConfirm global
WebSocketavailability for the intended runtime and address missing quality guardrails.The default factory relies on the global
WebSocket. While yourpackage.jsonspecifiesbun@1.3.3(which supports globalWebSocket), the absence of anenginesfield creates portability risk if executed on Node < 22. Additionally, files insrc/transcribe/require quality guardrails for prompt artifacts and mixed-script garbage, which are missing in the currenthandleMessageimplementation.
- Add
"engines": { "bun": ">=1.3.3" }topackage.jsonto prevent execution on incompatible runtimes.- Implement quality validation logic (e.g.,
validateTranscript) for final tokens insrc/transcribe/soniox-streaming.tsbefore emitting to "transcript".🤖 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 `@src/transcribe/soniox-streaming.ts` around lines 71 - 73, The default WebSocket factory in SonioxStreaming assumes a global WebSocket and the transcript flow lacks quality filtering before emitting final results. Update package.json to declare the Bun runtime requirement with an engines constraint, and add a validation step in SonioxStreaming.handleMessage (or a helper like validateTranscript) so final transcript tokens are checked for prompt artifacts and mixed-script garbage before calling the transcript emitter.
🤖 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 `@src/daemon/hotkey.ts`:
- Around line 56-77: The hotkey matcher in matchesHotkeyBinding is too
permissive because it only checks that required modifiers are present, so
overlapping bindings can both fire when extra modifiers are held. Update the
matching logic to require an exact modifier match: normalize the pressed state
and compare both the set and count of active modifiers against
binding.modifiers, while preserving the current LEFT/RIGHT CTRL, ALT, SHIFT, and
META/WIN alias handling. Ensure the function only returns true when the pressed
modifiers exactly equal the binding’s required modifiers, so bindings like Space
and CTRL+Space do not match at the same time.
In `@src/daemon/service.ts`:
- Around line 632-666: The Soniox startup flow in the starting branch does not
honor cancelPending, so a second hotkey press during startup still proceeds into
recording. Add cancellation checks in the Soniox start sequence around the await
points in the same method that sets up SonioxStreamingTranscriber, especially
after start() and before/after recorder.start(), and if cancelPending is set
then call teardownStreaming, setStatus("idle"), and exit early just like
handleTrigger does.
- Around line 714-726: The PerfEvent schema is missing the Soniox-specific
fields used by appendStatsAggregateEntry in service.ts, so TypeScript will
reject the PerfEvent object. Update PerfEventSchema in src/stats/perf-event.ts
to include sonioxChunkCount, sonioxStopReason, and sonioxCloseWaitMs with the
correct optional types so the object literal in src/daemon/service.ts matches
PerfEvent.
In `@src/output/live-dictation.ts`:
- Around line 75-91: acceptCommittedTranscript in LiveDictationWriter is
treating incoming transcripts as cumulative, but the streaming transcribers emit
per-segment finals, so the current startsWith/slice delta logic can concatenate
text incorrectly. Update the handling in acceptCommittedTranscript to support
non-cumulative segment input, either by appending a separator when a new segment
does not extend committedText or by making DeepgramStreamingTranscriber and
SonioxStreamingTranscriber emit cumulative transcripts before the transcript
event is consumed. Keep the fix aligned with the current committedText and
typer.typeText flow so successive final segments render with proper spacing.
In `@src/transcribe/soniox-streaming.ts`:
- Around line 180-205: The Soniox streaming path is bypassing the transcribe
quality checks by emitting final tokens directly from handleMessage and
returning them unchanged from stop(). Update SonioxStreaming so its final text
flows through the same guardrails used elsewhere in src/transcribe/** before
being pushed to transcriptChunks or emitted, including the artifact, suffix,
mixed-script, and merge-expansion validations. Then ensure stop() only returns
already-validated text by reusing the same validation path from
handleMessage/processSonioxAudio rather than exposing raw renderFinalTokenText
output.
---
Nitpick comments:
In `@docs/ISSUES-LIVE-DICTATION-SONIOX.md`:
- Line 99: The checklist item in the live dictation notes uses an unhyphenated
compound modifier; update the wording to hyphenate the phrase in the “Live
Dictation” bullet so the description reads with “scratch focused-input” for
clarity.
In `@src/config/schema.ts`:
- Around line 237-240: The Soniox API key validation in the schema uses .min(1)
with a message that implies a length requirement, so update the validation on
the soniox field to use clearer wording or a real minimum length if the key
format is known. Adjust the message in the z.string() chain for soniox so it
reflects non-empty input, or replace the current .min(1, ...) check with an
appropriate length rule tied to the Soniox key format.
In `@src/transcribe/deepgram-streaming.ts`:
- Around line 130-147: The Deepgram transcript handling in deepgram-streaming.ts
duplicates LiveTranscriptEvent निर्माण in both branches and assigns optional
payload fields directly to strictly typed booleans. Refactor the transcript
emission logic around the existing transcript processing block to build the
event once, and explicitly coerce data.is_final and data.speech_final with
Boolean(...) before assigning them to LiveTranscriptEvent.isFinal and
speechFinal so the types stay strict and the duplication is removed.
In `@src/transcribe/soniox-streaming.ts`:
- Around line 217-228: `waitForClose` in `SonioxStreaming` only waits for a
future `close` event, so if the socket has already closed before `stop()` calls
it, the promise times out and inflates `closeWaitMs`. Update `waitForClose()` to
short-circuit when the connection is already closed by checking the
stream/socket state first (for example via an existing `readyState` or
`isConnected`-style flag used in `SonioxStreaming`) and resolve immediately
instead of registering `once("close")` in that case.
- Around line 71-73: The default WebSocket factory in SonioxStreaming assumes a
global WebSocket and the transcript flow lacks quality filtering before emitting
final results. Update package.json to declare the Bun runtime requirement with
an engines constraint, and add a validation step in
SonioxStreaming.handleMessage (or a helper like validateTranscript) so final
transcript tokens are checked for prompt artifacts and mixed-script garbage
before calling the transcript emitter.
In `@tests/config.test.ts`:
- Around line 53-56: The config default test for live dictation is missing
coverage for the soniox trigger key. In the config test around the existing
config.liveDictation assertions, add an expectation for
config.liveDictation.soniox.triggerKey to verify it defaults to "Right Alt"
alongside the other defaults.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b5d59ec-1a91-487c-875c-b2f022af254e
📒 Files selected for processing (20)
CONTEXT.mddocs/CONFIGURATION.mddocs/ISSUES-LIVE-DICTATION-SONIOX.mddocs/PRD-LIVE-DICTATION-SONIOX.mddocs/STT_FLOW.mdsrc/config/loader.tssrc/config/schema.tssrc/daemon/hotkey.tssrc/daemon/recording-session.tssrc/daemon/service.tssrc/output/live-dictation.tssrc/transcribe/deepgram-streaming.tssrc/transcribe/live-provider.tssrc/transcribe/soniox-streaming.tstests/config.test.tstests/hotkey-bindings.test.tstests/live-dictation.test.tstests/live-provider.test.tstests/recording-session-live-dictation.test.tstests/soniox-streaming.test.ts
Comment on lines
+56
to
+77
| return binding.modifiers.every((mod) => { | ||
| if (mod === "CTRL" || mod === "CONTROL") { | ||
| return ( | ||
| down["LEFT CTRL"] || | ||
| down["RIGHT CTRL"] || | ||
| down["LEFT CONTROL"] || | ||
| down["RIGHT CONTROL"] | ||
| ); | ||
| } | ||
| if (mod === "ALT") return down["LEFT ALT"] || down["RIGHT ALT"]; | ||
| if (mod === "SHIFT") return down["LEFT SHIFT"] || down["RIGHT SHIFT"]; | ||
| if (mod === "META" || mod === "SUPER" || mod === "WIN") { | ||
| return ( | ||
| down["LEFT META"] || | ||
| down["RIGHT META"] || | ||
| down["LEFT WIN"] || | ||
| down["RIGHT WIN"] | ||
| ); | ||
| } | ||
|
|
||
| return down[mod]; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
cat -n src/daemon/hotkey.tsRepository: Snehit70/hyprvox
Length of output: 5651
Hotkey matching allows extra modifiers, causing potential double-firing on overlapping bindings.
The current logic uses binding.modifiers.every(...), which returns true even if extra modifiers are held down.
- If a binding specifies no modifiers (e.g.,
Space) and another specifiesCTRL+Space, pressingCTRL+Spacematches both bindings. - The
pressedBindingsset only prevents re-triggering the same binding ID, not competing bindings in the same event cycle.
Update matchesHotkeyBinding to ensure an exact match: verify that the count and set of pressed modifiers equal the required modifiers.
Current Logic (Lines 56-77)
- return binding.modifiers.every((mod) => {
- if (mod === "CTRL" || mod === "CONTROL") {
- return (
- down["LEFT CTRL"] ||
- down["RIGHT CTRL"] ||
- down["LEFT CONTROL"] ||
- down["RIGHT CONTROL"]
- );
- }
- if (mod === "ALT") return down["LEFT ALT"] || down["RIGHT ALT"];
- if (mod === "SHIFT") return down["LEFT SHIFT"] || down["RIGHT SHIFT"];
- if (mod === "META" || mod === "SUPER" || mod === "WIN") {
- return (
- down["LEFT META"] ||
- down["RIGHT META"] ||
- down["LEFT WIN"] ||
- down["RIGHT WIN"]
- );
- }
-
- return down[mod];
- });
+ // Count and verify exact modifier match
+ let pressedCount = 0;
+ for (const mod of binding.modifiers) {
+ let isPressed = false;
+ if (mod === "CTRL" || mod === "CONTROL") {
+ isPressed =
+ down["LEFT CTRL"] ||
+ down["RIGHT CTRL"] ||
+ down["LEFT CONTROL"] ||
+ down["RIGHT CONTROL"];
+ } else if (mod === "ALT") {
+ isPressed = down["LEFT ALT"] || down["RIGHT ALT"];
+ } else if (mod === "SHIFT") {
+ isPressed = down["LEFT SHIFT"] || down["RIGHT SHIFT"];
+ } else if (mod === "META" || mod === "SUPER" || mod === "WIN") {
+ isPressed =
+ down["LEFT META"] ||
+ down["RIGHT META"] ||
+ down["LEFT WIN"] ||
+ down["RIGHT WIN"];
+ } else {
+ isPressed = !!down[mod];
+ }
+
+ if (!isPressed) return false;
+ pressedCount++;
+ }
+
+ // Reject if additional modifiers are held
+ const pressedModifierKeys = Object.keys(down).filter(
+ (k) =>
+ down[k] &&
+ (k.includes("CTRL") ||
+ k.includes("ALT") ||
+ k.includes("SHIFT") ||
+ k.includes("META") ||
+ k.includes("WIN") ||
+ k.includes("SUPER"))
+ );
+ return pressedCount === pressedModifierKeys.length;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return binding.modifiers.every((mod) => { | |
| if (mod === "CTRL" || mod === "CONTROL") { | |
| return ( | |
| down["LEFT CTRL"] || | |
| down["RIGHT CTRL"] || | |
| down["LEFT CONTROL"] || | |
| down["RIGHT CONTROL"] | |
| ); | |
| } | |
| if (mod === "ALT") return down["LEFT ALT"] || down["RIGHT ALT"]; | |
| if (mod === "SHIFT") return down["LEFT SHIFT"] || down["RIGHT SHIFT"]; | |
| if (mod === "META" || mod === "SUPER" || mod === "WIN") { | |
| return ( | |
| down["LEFT META"] || | |
| down["RIGHT META"] || | |
| down["LEFT WIN"] || | |
| down["RIGHT WIN"] | |
| ); | |
| } | |
| return down[mod]; | |
| }); | |
| let pressedCount = 0; | |
| for (const mod of binding.modifiers) { | |
| let isPressed = false; | |
| if (mod === "CTRL" || mod === "CONTROL") { | |
| isPressed = | |
| down["LEFT CTRL"] || | |
| down["RIGHT CTRL"] || | |
| down["LEFT CONTROL"] || | |
| down["RIGHT CONTROL"]; | |
| } else if (mod === "ALT") { | |
| isPressed = down["LEFT ALT"] || down["RIGHT ALT"]; | |
| } else if (mod === "SHIFT") { | |
| isPressed = down["LEFT SHIFT"] || down["RIGHT SHIFT"]; | |
| } else if (mod === "META" || mod === "SUPER" || mod === "WIN") { | |
| isPressed = | |
| down["LEFT META"] || | |
| down["RIGHT META"] || | |
| down["LEFT WIN"] || | |
| down["RIGHT WIN"]; | |
| } else { | |
| isPressed = !!down[mod]; | |
| } | |
| if (!isPressed) return false; | |
| pressedCount++; | |
| } | |
| const pressedModifierKeys = Object.keys(down).filter( | |
| (k) => | |
| down[k] && | |
| (k.includes("CTRL") || | |
| k.includes("ALT") || | |
| k.includes("SHIFT") || | |
| k.includes("META") || | |
| k.includes("WIN") || | |
| k.includes("SUPER")) | |
| ); | |
| return pressedCount === pressedModifierKeys.length; |
🤖 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 `@src/daemon/hotkey.ts` around lines 56 - 77, The hotkey matcher in
matchesHotkeyBinding is too permissive because it only checks that required
modifiers are present, so overlapping bindings can both fire when extra
modifiers are held. Update the matching logic to require an exact modifier
match: normalize the pressed state and compare both the set and count of active
modifiers against binding.modifiers, while preserving the current LEFT/RIGHT
CTRL, ALT, SHIFT, and META/WIN alias handling. Ensure the function only returns
true when the pressed modifiers exactly equal the binding’s required modifiers,
so bindings like Space and CTRL+Space do not match at the same time.
Comment on lines
+632
to
+666
| if (this.status === "idle" || this.status === "error") { | ||
| this.recordingMode = "soniox"; | ||
| this.cancelPending = false; | ||
| try { | ||
| this.setStatus("starting"); | ||
| this.sonioxStreaming = new SonioxStreamingTranscriber(); | ||
| this.sonioxStreaming.on("error", (error) => { | ||
| logError("Soniox streaming error", error); | ||
| }); | ||
| this.liveDictationTranscriptDetach = | ||
| attachLiveDictationTranscriptHandler({ | ||
| provider: this.sonioxStreaming, | ||
| typer: new DesktopTextTyper({ | ||
| preferredCommand: this.config.liveDictation.insertionCommand, | ||
| }), | ||
| }); | ||
| await this.sonioxStreaming.start(this.config.transcription.language); | ||
| this.streamingPcmHandler = attachStreamingPcmHandler({ | ||
| recorder: this.recorder, | ||
| liveProvider: this.sonioxStreaming, | ||
| }); | ||
| await this.recorder.start(); | ||
| } catch (error) { | ||
| this.cancelPending = false; | ||
| logError("Failed to start Soniox recording", error); | ||
| await this.teardownStreaming( | ||
| "Failed to stop Soniox streaming after start failure", | ||
| ); | ||
| this.setStatus("idle"); | ||
| notify( | ||
| "Soniox Error", | ||
| "Failed to start Soniox live dictation", | ||
| "error", | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Cancellation during Soniox startup is never honored.
The "starting" branch (Line 670-673) sets this.cancelPending = true, but unlike handleTrigger, this path has no if (this.cancelPending) checkpoints after await this.sonioxStreaming.start(...) or before/after await this.recorder.start(). A user pressing the Soniox hotkey again during startup sets the flag, but the flow proceeds to record anyway and the flag is only reset on the next idle trigger. Add cancellation checkpoints (with teardownStreaming + setStatus("idle")) mirroring the standard trigger.
🤖 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 `@src/daemon/service.ts` around lines 632 - 666, The Soniox startup flow in the
starting branch does not honor cancelPending, so a second hotkey press during
startup still proceeds into recording. Add cancellation checks in the Soniox
start sequence around the await points in the same method that sets up
SonioxStreamingTranscriber, especially after start() and before/after
recorder.start(), and if cancelPending is set then call teardownStreaming,
setStatus("idle"), and exit early just like handleTrigger does.
Comment on lines
+714
to
+726
| void appendStatsAggregateEntry({ | ||
| timestamp: new Date().toISOString(), | ||
| engine: "soniox", | ||
| processingMs: processingTime, | ||
| mergeStrategy: "provider_bypass", | ||
| mergeReason: "soniox_live", | ||
| validationReasons: [], | ||
| validationRetryCount: 0, | ||
| validationFallbackSource: "none", | ||
| sonioxChunkCount: result.chunkCount, | ||
| sonioxStopReason: result.stopReason, | ||
| sonioxCloseWaitMs: result.closeWaitMs, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'aggregate.ts' src/stats --exec sh -c 'echo "== {} =="; cat -n "{}"'
rg -nP 'provider_bypass|soniox_live|sonioxChunkCount|sonioxStopReason|sonioxCloseWaitMs|mergeStrategy|mergeReason' src/statsRepository: Snehit70/hyprvox
Length of output: 2003
🏁 Script executed:
#!/bin/bash
cat -n src/stats/perf-event.tsRepository: Snehit70/hyprvox
Length of output: 3263
🏁 Script executed:
#!/bin/bash
# Check if soniox fields are part of the PerfEvent schema anywhere
rg -i 'sonioxChunkCount|sonioxStopReason|sonioxCloseWaitMs' src/stats/perf-event.tsRepository: Snehit70/hyprvox
Length of output: 154
Add sonioxChunkCount, sonioxStopReason, and sonioxCloseWaitMs to the PerfEvent schema.
The mergeStrategy values "provider_bypass" and "soniox_live" are accepted by the generic string schema, but the Soniox-specific fields are not defined in src/stats/perf-event.ts. Strict TypeScript will reject the object literal in src/daemon/service.ts because these properties do not exist on the PerfEvent type.
Add the following fields to PerfEventSchema in src/stats/perf-event.ts:
sonioxChunkCount: z.number().int().nonnegative().optional(),
sonioxStopReason: z.string().nullable().optional(),
sonioxCloseWaitMs: z.number().nonnegative().optional(),🤖 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 `@src/daemon/service.ts` around lines 714 - 726, The PerfEvent schema is
missing the Soniox-specific fields used by appendStatsAggregateEntry in
service.ts, so TypeScript will reject the PerfEvent object. Update
PerfEventSchema in src/stats/perf-event.ts to include sonioxChunkCount,
sonioxStopReason, and sonioxCloseWaitMs with the correct optional types so the
object literal in src/daemon/service.ts matches PerfEvent.
Comment on lines
+180
to
+205
| private handleMessage(data: unknown): void { | ||
| try { | ||
| const response = JSON.parse(String(data)) as SonioxResponse; | ||
| if (response.error_code || response.error_message) { | ||
| const message = | ||
| response.error_message ?? | ||
| `Soniox streaming error: ${response.error_code ?? "unknown"}`; | ||
| const error = new Error(message); | ||
| this.emit("error", error); | ||
| return; | ||
| } | ||
|
|
||
| const finalText = renderFinalTokenText(response.tokens ?? []); | ||
| if (!finalText) return; | ||
|
|
||
| this.transcriptChunks.push(finalText); | ||
| const event: LiveTranscriptEvent = { | ||
| text: finalText, | ||
| isFinal: true, | ||
| speechFinal: true, | ||
| }; | ||
| this.emit("transcript", finalText, event); | ||
| } catch (error) { | ||
| logError("Failed to parse Soniox streaming response", error); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Soniox final tokens bypass transcription quality guardrails.
Final-token text is pushed and emitted directly, and stop() returns it untouched. Downstream (processSonioxAudio) writes it to clipboard/history without running the prompt-artifact / hallucination-suffix / mixed-script / merge-expansion validation that the rest of the transcribe layer applies. Per path instructions for src/transcribe/** ("Implement quality guardrails to validate prompt artifacts, suffix hallucinations, mixed-script garbage, and long-recording merge expansion"), the Soniox output should be validated before it reaches the user.
🤖 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 `@src/transcribe/soniox-streaming.ts` around lines 180 - 205, The Soniox
streaming path is bypassing the transcribe quality checks by emitting final
tokens directly from handleMessage and returning them unchanged from stop().
Update SonioxStreaming so its final text flows through the same guardrails used
elsewhere in src/transcribe/** before being pushed to transcriptChunks or
emitted, including the artifact, suffix, mixed-script, and merge-expansion
validations. Then ensure stop() only returns already-validated text by reusing
the same validation path from handleMessage/processSonioxAudio rather than
exposing raw renderFinalTokenText output.
Source: Path instructions
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/daemon/recording-session.ts (1)
105-117: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftExpose a way to drain queued live typing before formatted retype.
pendingWritecan still be running when callers use the returnedwriterforretypeFormattedText(), so a queued suffix write may land after the formatted replacement and corrupt the final text. Return awaitForPendingWrites()/flush()handle or queue retype through the same promise before Soniox teardown/retype.🤖 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 `@src/daemon/recording-session.ts` around lines 105 - 117, The recording session flow allows `pendingWrite` work from live transcript events to still be in flight when `writer.retypeFormattedText()` is called, which can let queued suffix text overwrite the formatted replacement. Update `recording-session` so the returned object from the transcript handler path exposes a `waitForPendingWrites()` or `flush()` method, or ensure `retypeFormattedText()` is chained through the same `pendingWrite` promise. Use the `pendingWrite` chain and the returned `writer`/`detach` API to locate the fix, and make sure teardown or retype waits for all queued transcript writes to finish first.
🧹 Nitpick comments (2)
src/cli/index.ts (1)
311-317: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winReuse the daemon socket-path source of truth.
This command hardcodes the socket location instead of reusing the same path constant/helper the daemon listens on. If socket resolution changes in
src/daemon/ipc.ts,hyprvox soniox-togglewill quietly target the wrong file.🤖 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 `@src/cli/index.ts` around lines 311 - 317, The socket path in the CLI is hardcoded instead of using the daemon’s shared source of truth, so it can drift from what the daemon actually listens on. Update the `soniox-toggle` command in `src/cli/index.ts` to reuse the same socket-path constant/helper used by `src/daemon/ipc.ts` (or extract one shared helper both sides import) so `join(homedir(), ".config", "hypr", "vox", "daemon.sock")` is no longer duplicated.tests/soniox-streaming.test.ts (1)
197-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact accumulated transcript after paragraph pauses.
These checks pass even if the second chunk is truncated internally. Add exact event/final-text assertions so the tests catch divergence between emitted live text and
stop().text.Also applies to: 261-261
🤖 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 `@tests/soniox-streaming.test.ts` around lines 197 - 198, The paragraph-pause test in soniox-streaming is only checking event count and whitespace, so it can miss truncation between the live transcript and the final stop() result. Strengthen the assertions around the streaming flow by checking the exact accumulated transcript emitted by the relevant event handler and comparing it to stop().text. Use the existing soniox streaming test cases and symbols like events, stop(), and the paragraph pause assertions to ensure the final text exactly matches the expected transcript in both affected cases.
🤖 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 `@docs/PRD-SONIOX-LIVE-DICTATION-IMPROVEMENTS.md`:
- Around line 98-167: The docs section contains the same “Implementation Status”
table repeated multiple times, which looks like a copy-paste duplication. In
docs/PRD-SONIOX-LIVE-DICTATION-IMPROVEMENTS.md, keep only one instance of the
table and remove the extra repeated blocks so the section appears once with the
existing items and statuses.
In `@src/cli/index.ts`:
- Around line 327-333: The success path in client.on("connect") exits too early,
which can cut off the Soniox toggle before the socket write has flushed. Update
the handler to wait for the write callback or the socket "drain"/"finish"
completion before calling client.destroy() and process.exit(0), and keep the
success log only after the action has been fully sent.
In `@src/daemon/ipc.ts`:
- Around line 163-165: Buffer incoming IPC data in the ipc.ts parsing flow
before emitting commands, since the current data.toString().split("\n") handling
in the command emission path can discard a JSON line split across socket chunks.
Update the parser in the IPC message handling around the msg.type === "action"
check so it accumulates partial frames until a full newline-delimited JSON
message is available, then emit the command from the completed message. Ensure
the logic preserves existing behavior for complete frames while handling split
frames for actions like soniox-toggle.
In `@src/output/live-dictation.ts`:
- Around line 118-127: The fallback path in retypeFormattedText leaves
committedText stale when selectLineAndType is unavailable, so update
committedText after typeText(formattedText) just like the DesktopTextTyper path.
Keep the change localized to LiveDictation.retypeFormattedText and ensure
getCommittedText and later delta typing always see the newly retyped
formattedText regardless of typer implementation.
In `@src/transcribe/soniox-streaming.ts`:
- Around line 294-330: formatSonioxWithLLM currently ignores fallbackApiKey, so
add fallback handling when the primary Groq request fails. In the catch block,
retry the same client.chat.completions.create flow using fallbackApiKey if it is
present and different from the primary key, then return the fallback result;
keep the existing raw-text fallback only if both attempts fail. Use the existing
formatSonioxWithLLM function and Groq client creation as the main touchpoints.
- Around line 205-208: The logging in the Soniox streaming flow is exposing
dictated content by including raw transcript fields such as token.text, delta,
and fullText. Update the debug logging inside the token-processing logic in
soniox-streaming.ts to avoid emitting any raw text and instead log only
non-sensitive diagnostics like lengths, booleans, or other metadata; apply the
same redaction to the related logging site that handles delta/fullText so no
transcript content is written to logs.
- Around line 212-242: The transcript accumulation in soniox-streaming’s final
emission path is inconsistent when the new text is not a true extension of
lastEmittedFullText, causing slice() to drop content and making live output
differ from stored final text. Update the logic around renderFinalTokenText,
lastEmittedFullText, and transcriptChunks so delta is only derived for actual
append-only updates, and use the accumulated transcriptChunks.join("") as the
source of truth for the emitted/stored transcript. Keep the paragraph prefix
handling in sync so both the LiveTranscriptEvent text and the final stored text
include the same content.
---
Outside diff comments:
In `@src/daemon/recording-session.ts`:
- Around line 105-117: The recording session flow allows `pendingWrite` work
from live transcript events to still be in flight when
`writer.retypeFormattedText()` is called, which can let queued suffix text
overwrite the formatted replacement. Update `recording-session` so the returned
object from the transcript handler path exposes a `waitForPendingWrites()` or
`flush()` method, or ensure `retypeFormattedText()` is chained through the same
`pendingWrite` promise. Use the `pendingWrite` chain and the returned
`writer`/`detach` API to locate the fix, and make sure teardown or retype waits
for all queued transcript writes to finish first.
---
Nitpick comments:
In `@src/cli/index.ts`:
- Around line 311-317: The socket path in the CLI is hardcoded instead of using
the daemon’s shared source of truth, so it can drift from what the daemon
actually listens on. Update the `soniox-toggle` command in `src/cli/index.ts` to
reuse the same socket-path constant/helper used by `src/daemon/ipc.ts` (or
extract one shared helper both sides import) so `join(homedir(), ".config",
"hypr", "vox", "daemon.sock")` is no longer duplicated.
In `@tests/soniox-streaming.test.ts`:
- Around line 197-198: The paragraph-pause test in soniox-streaming is only
checking event count and whitespace, so it can miss truncation between the live
transcript and the final stop() result. Strengthen the assertions around the
streaming flow by checking the exact accumulated transcript emitted by the
relevant event handler and comparing it to stop().text. Use the existing soniox
streaming test cases and symbols like events, stop(), and the paragraph pause
assertions to ensure the final text exactly matches the expected transcript in
both affected cases.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 983bef7e-d3a0-43c2-af75-a3de92a1deea
📒 Files selected for processing (15)
CONTEXT.mddocs/CONFIGURATION.mddocs/PRD-SONIOX-LIVE-DICTATION-IMPROVEMENTS.mddocs/STT_FLOW.mdsrc/cli/index.tssrc/config/schema.tssrc/daemon/ipc.tssrc/daemon/recording-session.tssrc/daemon/service.tssrc/output/live-dictation.tssrc/shared/ipc-types.tssrc/transcribe/deepgram-streaming.tssrc/transcribe/soniox-streaming.tstests/hotkey-bindings.test.tstests/soniox-streaming.test.ts
✅ Files skipped from review due to trivial changes (1)
- CONTEXT.md
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/STT_FLOW.md
- src/transcribe/deepgram-streaming.ts
- tests/hotkey-bindings.test.ts
- src/config/schema.ts
- docs/CONFIGURATION.md
- src/daemon/service.ts
Comment on lines
+98
to
+167
| ## Implementation Status | ||
|
|
||
| | Item | Status | | ||
| |------|--------| | ||
| | Token spacing (join with space, collapse doubles) | Done | | ||
| | Per-token debug logging | Done | | ||
| | Structured perf entries | Done | | ||
| | `paragraphPauseMs` config schema | Done | | ||
| | Paragraph break insertion (double-space prefix) | Done | | ||
| | LLM formatting pass (Groq/Llama 3.3) | Done | | ||
| | `retypeFormatted` config + Home+Shift+End retype | Done | | ||
| | Tests (9 Soniox-specific, 245 total) | Done | | ||
| | Documentation updates | Done | | ||
|
|
||
| ## Implementation Status | ||
|
|
||
| | Item | Status | | ||
| |------|--------| | ||
| | Token spacing (join with space, collapse doubles) | Done | | ||
| | Per-token debug logging | Done | | ||
| | Structured perf entries | Done | | ||
| | `paragraphPauseMs` config schema | Done | | ||
| | Paragraph break insertion (double-space prefix) | Done | | ||
| | LLM formatting pass (Groq/Llama 3.3) | Done | | ||
| | `retypeFormatted` config + Home+Shift+End retype | Done | | ||
| | Tests (9 Soniox-specific, 245 total) | Done | | ||
| | Documentation updates | Done | | ||
|
|
||
| ## Implementation Status | ||
|
|
||
| | Item | Status | | ||
| |------|--------| | ||
| | Token spacing (join with space, collapse doubles) | Done | | ||
| | Per-token debug logging | Done | | ||
| | Structured perf entries | Done | | ||
| | `paragraphPauseMs` config schema | Done | | ||
| | Paragraph break insertion (double-space prefix) | Done | | ||
| | LLM formatting pass (Groq/Llama 3.3) | Done | | ||
| | `retypeFormatted` config + Home+Shift+End retype | Done | | ||
| | Tests (9 Soniox-specific, 245 total) | Done | | ||
| | Documentation updates | Done | | ||
|
|
||
| ## Implementation Status | ||
|
|
||
| | Item | Status | | ||
| |------|--------| | ||
| | Token spacing (join with space, collapse doubles) | Done | | ||
| | Per-token debug logging | Done | | ||
| | Structured perf entries | Done | | ||
| | `paragraphPauseMs` config schema | Done | | ||
| | Paragraph break insertion (double-space prefix) | Done | | ||
| | LLM formatting pass (Groq/Llama 3.3) | Done | | ||
| | `retypeFormatted` config + Home+Shift+End retype | Done | | ||
| | Tests (9 Soniox-specific, 245 total) | Done | | ||
| | Documentation updates | Done | | ||
|
|
||
| ## Implementation Status | ||
|
|
||
| | Item | Status | | ||
| |------|--------| | ||
| | Token spacing (join with space, collapse doubles) | Done | | ||
| | Per-token debug logging | Done | | ||
| | Structured perf entries | Done | | ||
| | `paragraphPauseMs` config schema | Done | | ||
| | Paragraph break insertion (double-space prefix) | Done | | ||
| | LLM formatting pass (Groq/Llama 3.3) | Done | | ||
| | `retypeFormatted` config + Home+Shift+End retype | Done | | ||
| | Tests (9 Soniox-specific, 245 total) | Done | | ||
| | Documentation updates | Done | | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove duplicate "Implementation Status" sections.
The "Implementation Status" table is repeated 5 times consecutively (lines 98–111, 112–125, 126–139, 140–153, 154–167). This appears to be a copy-paste error. Keep one copy and remove the rest.
🤖 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 `@docs/PRD-SONIOX-LIVE-DICTATION-IMPROVEMENTS.md` around lines 98 - 167, The
docs section contains the same “Implementation Status” table repeated multiple
times, which looks like a copy-paste duplication. In
docs/PRD-SONIOX-LIVE-DICTATION-IMPROVEMENTS.md, keep only one instance of the
table and remove the extra repeated blocks so the section appears once with the
existing items and statuses.
Comment on lines
+327
to
+333
| client.on("connect", () => { | ||
| client.write( | ||
| JSON.stringify({ type: "action", action: "soniox-toggle" }) + "\n", | ||
| ); | ||
| console.log(colors.green("✅") + " Soniox toggle sent to daemon"); | ||
| client.destroy(); | ||
| process.exit(0); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wait for the action write to flush before exiting.
Lines 328-333 report success, destroy() the socket, and process.exit(0) immediately. That can terminate the process before the newline-delimited action is fully written, so the CLI may claim success while the daemon never receives the toggle.
Suggested fix
client.on("connect", () => {
- client.write(
- JSON.stringify({ type: "action", action: "soniox-toggle" }) + "\n",
- );
- console.log(colors.green("✅") + " Soniox toggle sent to daemon");
- client.destroy();
- process.exit(0);
+ client.end(
+ JSON.stringify({ type: "action", action: "soniox-toggle" }) + "\n",
+ () => {
+ console.log(colors.green("✅") + " Soniox toggle sent to daemon");
+ process.exit(0);
+ },
+ );
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| client.on("connect", () => { | |
| client.write( | |
| JSON.stringify({ type: "action", action: "soniox-toggle" }) + "\n", | |
| ); | |
| console.log(colors.green("✅") + " Soniox toggle sent to daemon"); | |
| client.destroy(); | |
| process.exit(0); | |
| client.on("connect", () => { | |
| client.end( | |
| JSON.stringify({ type: "action", action: "soniox-toggle" }) + "\n", | |
| () => { | |
| console.log(colors.green("✅") + " Soniox toggle sent to daemon"); | |
| process.exit(0); | |
| }, | |
| ); | |
| }); |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@src/cli/index.ts` around lines 327 - 333, The success path in
client.on("connect") exits too early, which can cut off the Soniox toggle before
the socket write has flushed. Update the handler to wait for the write callback
or the socket "drain"/"finish" completion before calling client.destroy() and
process.exit(0), and keep the success log only after the action has been fully
sent.
Comment on lines
+163
to
+165
| if (msg.type === "action" && typeof msg.action === "string") { | ||
| this.emit("command", msg.action); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Buffer IPC frames before emitting commands.
Lines 163-165 rely on the existing data.toString().split("\n") parsing path, which drops a command if the JSON line arrives across multiple socket chunks. That makes the new soniox-toggle IPC action a silent no-op in a valid stream-framing case.
Suggested fix
private handleClientConnection(socket: Socket): void {
const clientId = ++this.clientIdCounter;
this.clients.set(clientId, socket);
+ let pending = "";
logger.debug({ clientId }, "IPC client connected");
this.emit("clientConnected", clientId);
@@
socket.on("data", (data) => {
- try {
- const lines = data
- .toString()
- .split("\n")
- .filter((l) => l.trim());
- for (const line of lines) {
- const msg = JSON.parse(line);
- logger.debug({ clientId, msg }, "Received message from client");
- if (msg.type === "action" && typeof msg.action === "string") {
- this.emit("command", msg.action);
- }
- }
- } catch {
- // Malformed JSON - ignore
- }
+ pending += data.toString();
+ const lines = pending.split("\n");
+ pending = lines.pop() ?? "";
+
+ for (const line of lines) {
+ if (!line.trim()) continue;
+ try {
+ const msg = JSON.parse(line);
+ logger.debug({ clientId, msg }, "Received message from client");
+ if (msg.type === "action" && msg.action === "soniox-toggle") {
+ this.emit("command", msg.action);
+ }
+ } catch {
+ // Malformed JSON - ignore
+ }
+ }
});🤖 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 `@src/daemon/ipc.ts` around lines 163 - 165, Buffer incoming IPC data in the
ipc.ts parsing flow before emitting commands, since the current
data.toString().split("\n") handling in the command emission path can discard a
JSON line split across socket chunks. Update the parser in the IPC message
handling around the msg.type === "action" check so it accumulates partial frames
until a full newline-delimited JSON message is available, then emit the command
from the completed message. Ensure the logic preserves existing behavior for
complete frames while handling split frames for actions like soniox-toggle.
Comment on lines
+118
to
+127
| public async retypeFormattedText(formattedText: string): Promise<void> { | ||
| const typer = this.typer as DesktopTextTyper; | ||
| if (typeof typer.selectLineAndType !== "function") { | ||
| await this.typer.typeText(formattedText); | ||
| return; | ||
| } | ||
|
|
||
| await typer.selectLineAndType(formattedText); | ||
| this.committedText = formattedText; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep committedText in sync in the fallback retype path.
Line 121 types formattedText for non-DesktopTextTyper implementations but returns without updating committedText, so getCommittedText() and later delta typing can observe stale state.
Proposed fix
if (typeof typer.selectLineAndType !== "function") {
await this.typer.typeText(formattedText);
+ this.committedText = formattedText;
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public async retypeFormattedText(formattedText: string): Promise<void> { | |
| const typer = this.typer as DesktopTextTyper; | |
| if (typeof typer.selectLineAndType !== "function") { | |
| await this.typer.typeText(formattedText); | |
| return; | |
| } | |
| await typer.selectLineAndType(formattedText); | |
| this.committedText = formattedText; | |
| } | |
| public async retypeFormattedText(formattedText: string): Promise<void> { | |
| const typer = this.typer as DesktopTextTyper; | |
| if (typeof typer.selectLineAndType !== "function") { | |
| await this.typer.typeText(formattedText); | |
| this.committedText = formattedText; | |
| return; | |
| } | |
| await typer.selectLineAndType(formattedText); | |
| this.committedText = formattedText; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn, spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@src/output/live-dictation.ts` around lines 118 - 127, The fallback path in
retypeFormattedText leaves committedText stale when selectLineAndType is
unavailable, so update committedText after typeText(formattedText) just like the
DesktopTextTyper path. Keep the change localized to
LiveDictation.retypeFormattedText and ensure getCommittedText and later delta
typing always see the newly retyped formattedText regardless of typer
implementation.
Comment on lines
+205
to
+208
| for (const token of tokens) { | ||
| logger.debug( | ||
| { text: token.text, isFinal: token.is_final }, | ||
| "Soniox token received", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not log raw dictation text.
token.text, delta, and fullText can contain dictated PII; logging lengths/flags preserves diagnostics without storing transcript content.
Proposed fix
logger.debug(
- { text: token.text, isFinal: token.is_final },
+ { tokenLength: token.text?.length ?? 0, isFinal: token.is_final },
"Soniox token received",
);-logger.debug({ delta, fullText }, "Soniox transcript delta emitted");
+logger.debug(
+ { deltaLength: delta.length, fullTextLength: fullText.length },
+ "Soniox transcript delta emitted",
+);Also applies to: 235-235
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 205-208: Avoid logging sensitive data
Context: logger.debug(
{ text: token.text, isFinal: token.is_final },
"Soniox token received",
)
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🤖 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 `@src/transcribe/soniox-streaming.ts` around lines 205 - 208, The logging in
the Soniox streaming flow is exposing dictated content by including raw
transcript fields such as token.text, delta, and fullText. Update the debug
logging inside the token-processing logic in soniox-streaming.ts to avoid
emitting any raw text and instead log only non-sensitive diagnostics like
lengths, booleans, or other metadata; apply the same redaction to the related
logging site that handles delta/fullText so no transcript content is written to
logs.
Source: Linters/SAST tools
Comment on lines
+212
to
+242
| const fullText = renderFinalTokenText(tokens); | ||
| const delta = fullText.slice(this.lastEmittedFullText.length); | ||
| this.lastEmittedFullText = fullText; | ||
|
|
||
| if (!delta) return; | ||
|
|
||
| const now = Date.now(); | ||
| const gapMs = this.lastMessageTimestamp > 0 | ||
| ? now - this.lastMessageTimestamp | ||
| : 0; | ||
| this.lastMessageTimestamp = now; | ||
|
|
||
| let prefix = ""; | ||
| if (this.hasReceivedMessage && gapMs >= this.paragraphPauseMs) { | ||
| prefix = " "; | ||
| this.paragraphBreakCount++; | ||
| logger.debug( | ||
| { gapMs, paragraphPauseMs: this.paragraphPauseMs }, | ||
| "Soniox paragraph break inserted", | ||
| ); | ||
| } | ||
| this.hasReceivedMessage = true; | ||
|
|
||
| logger.debug({ delta, fullText }, "Soniox transcript delta emitted"); | ||
| this.transcriptChunks.push(prefix + delta); | ||
| const event: LiveTranscriptEvent = { | ||
| text: prefix + fullText, | ||
| isFinal: true, | ||
| speechFinal: true, | ||
| }; | ||
| this.emit("transcript", prefix + fullText, event); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Emit and store the accumulated transcript consistently.
When a later response is not an extension of lastEmittedFullText, slice() can drop most of the segment; with paragraph pauses, the live event emits prefix + fullText while stop() stores prefix + delta, so live typing and final text diverge. Compute delta only for true extensions and emit transcriptChunks.join("").
Proposed fix
const fullText = renderFinalTokenText(tokens);
-const delta = fullText.slice(this.lastEmittedFullText.length);
+const delta = fullText.startsWith(this.lastEmittedFullText)
+ ? fullText.slice(this.lastEmittedFullText.length)
+ : fullText;
this.lastEmittedFullText = fullText;
if (!delta) return;
@@
logger.debug({ delta, fullText }, "Soniox transcript delta emitted");
this.transcriptChunks.push(prefix + delta);
+const committedText = this.transcriptChunks.join("");
const event: LiveTranscriptEvent = {
- text: prefix + fullText,
+ text: committedText,
isFinal: true,
speechFinal: true,
};
-this.emit("transcript", prefix + fullText, event);
+this.emit("transcript", committedText, event);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const fullText = renderFinalTokenText(tokens); | |
| const delta = fullText.slice(this.lastEmittedFullText.length); | |
| this.lastEmittedFullText = fullText; | |
| if (!delta) return; | |
| const now = Date.now(); | |
| const gapMs = this.lastMessageTimestamp > 0 | |
| ? now - this.lastMessageTimestamp | |
| : 0; | |
| this.lastMessageTimestamp = now; | |
| let prefix = ""; | |
| if (this.hasReceivedMessage && gapMs >= this.paragraphPauseMs) { | |
| prefix = " "; | |
| this.paragraphBreakCount++; | |
| logger.debug( | |
| { gapMs, paragraphPauseMs: this.paragraphPauseMs }, | |
| "Soniox paragraph break inserted", | |
| ); | |
| } | |
| this.hasReceivedMessage = true; | |
| logger.debug({ delta, fullText }, "Soniox transcript delta emitted"); | |
| this.transcriptChunks.push(prefix + delta); | |
| const event: LiveTranscriptEvent = { | |
| text: prefix + fullText, | |
| isFinal: true, | |
| speechFinal: true, | |
| }; | |
| this.emit("transcript", prefix + fullText, event); | |
| const fullText = renderFinalTokenText(tokens); | |
| const delta = fullText.startsWith(this.lastEmittedFullText) | |
| ? fullText.slice(this.lastEmittedFullText.length) | |
| : fullText; | |
| this.lastEmittedFullText = fullText; | |
| if (!delta) return; | |
| const now = Date.now(); | |
| const gapMs = this.lastMessageTimestamp > 0 | |
| ? now - this.lastMessageTimestamp | |
| : 0; | |
| this.lastMessageTimestamp = now; | |
| let prefix = ""; | |
| if (this.hasReceivedMessage && gapMs >= this.paragraphPauseMs) { | |
| prefix = " "; | |
| this.paragraphBreakCount++; | |
| logger.debug( | |
| { gapMs, paragraphPauseMs: this.paragraphPauseMs }, | |
| "Soniox paragraph break inserted", | |
| ); | |
| } | |
| this.hasReceivedMessage = true; | |
| logger.debug({ delta, fullText }, "Soniox transcript delta emitted"); | |
| this.transcriptChunks.push(prefix + delta); | |
| const committedText = this.transcriptChunks.join(""); | |
| const event: LiveTranscriptEvent = { | |
| text: committedText, | |
| isFinal: true, | |
| speechFinal: true, | |
| }; | |
| this.emit("transcript", committedText, event); |
🤖 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 `@src/transcribe/soniox-streaming.ts` around lines 212 - 242, The transcript
accumulation in soniox-streaming’s final emission path is inconsistent when the
new text is not a true extension of lastEmittedFullText, causing slice() to drop
content and making live output differ from stored final text. Update the logic
around renderFinalTokenText, lastEmittedFullText, and transcriptChunks so delta
is only derived for actual append-only updates, and use the accumulated
transcriptChunks.join("") as the source of truth for the emitted/stored
transcript. Keep the paragraph prefix handling in sync so both the
LiveTranscriptEvent text and the final stored text include the same content.
Comment on lines
+294
to
+330
| export async function formatSonioxWithLLM( | ||
| text: string, | ||
| apiKey: string, | ||
| fallbackApiKey?: string, | ||
| ): Promise<{ formattedText: string; llmFormattingMs: number }> { | ||
| const startTime = Date.now(); | ||
| try { | ||
| const GroqModule = await import("groq-sdk"); | ||
| const Groq = GroqModule.default; | ||
| const client = new Groq({ apiKey }); | ||
|
|
||
| const completion = await client.chat.completions.create({ | ||
| model: "llama-3.3-70b-versatile", | ||
| messages: [ | ||
| { role: "system", content: LLM_FORMAT_SYSTEM_PROMPT }, | ||
| { role: "user", content: text }, | ||
| ], | ||
| temperature: 0, | ||
| max_tokens: Math.ceil(text.length / 3), | ||
| seed: 42, | ||
| }); | ||
|
|
||
| const formattedText = | ||
| completion.choices[0]?.message?.content?.trim() ?? text; | ||
| const llmFormattingMs = Date.now() - startTime; | ||
|
|
||
| logger.debug( | ||
| { originalLength: text.length, formattedLength: formattedText.length, llmFormattingMs }, | ||
| "Soniox LLM formatting complete", | ||
| ); | ||
|
|
||
| return { formattedText, llmFormattingMs }; | ||
| } catch (error) { | ||
| const llmFormattingMs = Date.now() - startTime; | ||
| logError("Soniox LLM formatting failed; using raw text", error); | ||
| return { formattedText: text, llmFormattingMs }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Use the supplied Groq fallback key.
fallbackApiKey is accepted but never read, so a configured fallback cannot recover formatting when the primary Groq key fails.
🤖 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 `@src/transcribe/soniox-streaming.ts` around lines 294 - 330,
formatSonioxWithLLM currently ignores fallbackApiKey, so add fallback handling
when the primary Groq request fails. In the catch block, retry the same
client.chat.completions.create flow using fallbackApiKey if it is present and
different from the primary key, then return the fallback result; keep the
existing raw-text fallback only if both attempts fail. Use the existing
formatSonioxWithLLM function and Groq client creation as the main touchpoints.
Soniox sends spaces as separate tokens, so join('') is correct.
Added double-space collapsing to handle edge cases.
Removed punctuation space regex (no longer needed).
Updated test expectations to match.
Soniox tokens include trailing spaces in each token text (e.g., 'world ' not 'world'). When joined, this produces unwanted spaces before punctuation like 'world ,' instead of 'world,'. Added regex to strip spaces before common punctuation characters. Added dedicated test case.
The wtype select command used \x1b[1;2C (Shift+Right, 1 char) instead of \x1b[1;2F (Shift+End, full line). This caused selectLineAndType to select only 1 character before retyping, producing garbled output. Added TDD tests for selectLineAndType on both Wayland and X11.
…yland Escape sequences like \x1b[1;2F are xterm-specific and may not work in Wayland terminal emulators. wtype supports native key press/release via -M/-m modifiers and -k keys, which works through the Wayland virtual keyboard protocol without relying on terminal escape sequence support. Changed select command from raw escape sequence to: wtype -k Home -M shift -k End -m shift Updated TDD test to match.
Soniox supports context.terms for custom vocabulary in the WebSocket config message. Now passes providerBoostWords from config to improve recognition of domain-specific terms (project names, commands, etc.). Added TDD test verifying the context.terms field in the config message.
…onfig; disable retype by default - Add languageHintsStrict (default true), contextGeneral (default software dev metadata), contextText (default technical context) to liveDictation.soniox - Change retypeFormatted default from true to false to avoid Wayland retype issues
…to WebSocket config - Add SonioxStreamingTranscriberOptions for languageHintsStrict, contextGeneral, contextText and pass them in the WebSocket config message - Fix incomplete unicode escape in punctuation regex (\u00 -> \u0021)
- Add 5 tests: language_hints_strict, context.general, context.text, combined context, empty context omission - Update hotkey-bindings fixture with new config fields - Document new config options in CONFIGURATION.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/CONFIGURATION.md (1)
333-343: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove duplicate table rows for
retypeFormattedandparagraphPauseMs.The options table contains duplicate entries:
retypeFormattedappears at line 335 and again at line 339 with identical descriptionsparagraphPauseMsappears at line 338 and again at line 340 with slightly different descriptionsConsolidate each to a single row with the correct, unified description.
🤖 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 `@docs/CONFIGURATION.md` around lines 333 - 343, Remove the duplicated configuration rows in the options table and keep only one entry each for retypeFormatted and soniox.paragraphPauseMs. Update the retained rows in docs/CONFIGURATION.md so the descriptions are unified and consistent, using the existing table entries for retypeFormatted and soniox.paragraphPauseMs as the symbols to locate the duplicates.
🤖 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.
Outside diff comments:
In `@docs/CONFIGURATION.md`:
- Around line 333-343: Remove the duplicated configuration rows in the options
table and keep only one entry each for retypeFormatted and
soniox.paragraphPauseMs. Update the retained rows in docs/CONFIGURATION.md so
the descriptions are unified and consistent, using the existing table entries
for retypeFormatted and soniox.paragraphPauseMs as the symbols to locate the
duplicates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a8d82a7f-4c5c-4333-88d8-79d5689a9f71
📒 Files selected for processing (5)
docs/CONFIGURATION.mdsrc/config/schema.tssrc/transcribe/soniox-streaming.tstests/hotkey-bindings.test.tstests/soniox-streaming.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/hotkey-bindings.test.ts
- src/config/schema.ts
- src/transcribe/soniox-streaming.ts
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
1 participant
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.
Summary
Validation
Notes
Summary by CodeRabbit
soniox-toggleCLI command and support for multiple hotkey bindings.SONIOX_API_KEYfallback and new PRD/issue breakdowns.