Skip to content

Studio: add Voice settings tab (dictation, dictionary, read aloud) - #7074

Merged
danielhanchen merged 23 commits into
mainfrom
studio-voice-settings
Jul 15, 2026
Merged

danielhanchen merged 23 commits into
mainfrom
studio-voice-settings

Conversation

@shimmyshimmer

Copy link
Copy Markdown
Member

What

Adds a Voice tab to Studio settings (placed just before About) covering microphone, dictation (STT) and read aloud (TTS).

Dictation

  • Microphone picker backed by device enumeration, with a fallback button to grant mic access so device names show up
  • STT engine select (browser speech service today, local models later) and recognition language (Auto plus 13 languages)
  • Inline "Test dictation" with a live transcript so settings can be checked on the spot

Dictation dictionary

  • Words or phrases dictation should recognize. Matching speech is rewritten to the exact stored spelling and casing, e.g. "un sloth" becomes "Unsloth". Applied in both dictation paths (main composer and compare chat)

Recent dictations

  • Last 20 final transcripts with timestamps, copy to clipboard and clear, so text can be recovered if it lands in the wrong place

Read aloud

  • Optional read aloud button on assistant responses (assistant-ui Speak primitives plus a speech adapter on the chat runtime)
  • Two TTS engines:
    • System voices: curated list. macOS novelty and legacy Eloquence voices are filtered out, remaining voices are limited to relevant languages, ranked by quality hints (Premium, Enhanced, Siri, Google, Natural) and capped at 20
    • Unsloth TTS model: synthesizes through the loaded audio model (e.g. Orpheus) via /api/inference/audio/generate and plays the returned WAV
  • Speed, pitch (system engine only), volume and a preview button

Implementation notes

  • New persisted store unsloth_voice_settings. All adapters read settings at call time, so changes apply to the next dictation or utterance without reloading the runtime
  • The saved microphone falls back to the system default if the device is unplugged
  • English strings added to i18n, tab label translated for ja, zh-CN and pt-BR (overlays pass the parity check)

Testing

  • tsc, biome on the new files and the i18n parity check all pass
  • Verified in the running app: tab renders with all rows, dictionary entries persist across a full reload, engine switching toggles the right rows, and preview with no audio model loaded surfaces a clear "No TTS model is loaded" error from the backend round trip. No console errors
  • Mic capture and audio playback need real hardware, so the final listen test was done manually with the Test dictation and Preview buttons

New Voice tab in Settings, placed just before About:

- Dictation: microphone picker, browser STT engine, recognition language,
  and an inline mic test with a live transcript
- Dictation dictionary: entries rewrite matching speech to their exact
  spelling and casing, applied in both dictation paths
- Recent dictations: last 20 final transcripts with copy and clear, so
  text can be recovered if it lands in the wrong place
- Read aloud: optional button on assistant responses with two engines,
  curated system voices (novelty and legacy voices filtered, quality
  ranked, capped at 20) or the TTS audio model loaded in Unsloth via
  /audio/generate (e.g. Orpheus), plus speed, pitch, volume and preview

Settings persist in localStorage (unsloth_voice_settings) and are read
at call time so changes apply without reloading the runtime. Adds en
keys plus the tab label for ja, zh-CN and pt-BR.
The STT engine dropdown only had one entry, so it added noise without
giving a real choice. The engine row can come back once local STT
models land. Also renames the TTS engine option Unsloth TTS model to
Load TTS model to make the action clearer.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces comprehensive voice settings and features to the Unsloth Studio frontend, including text-to-speech (TTS) read-aloud capabilities and speech-to-text (STT) dictation. Key additions include a new voice settings tab, state persistence via Zustand, custom dictation dictionaries, and integration with both browser-native speech engines and custom studio models. The review feedback identifies several critical improvements: resolving a jarring UX issue where empty dictionary inputs are prematurely deleted and unmounted during typing, fixing a browser compatibility check for 'OverconstrainedError' when fallback microphones are used, preventing potential crashes in insecure contexts where 'navigator.mediaDevices' is undefined, and addressing a memory leak by ensuring proper cleanup of 'Audio' elements holding large base64 data URLs.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread studio/frontend/src/features/chat/adapters/studio-web-speech-dictation-adapter.ts Outdated
Comment thread studio/frontend/src/features/settings/tabs/voice-tab.tsx Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review


P2 Badge Don't silently ignore the selected microphone

In browsers that expose SpeechRecognition but do not implement the start(audioTrack) overload, passing the selected track is ignored or falls into the no-argument retry, so recognition listens to the browser/default microphone instead of the device the user picked while still holding the selected stream open. This makes the new microphone picker appear to work but capture from the wrong mic in those environments; detect support or surface that the selected device cannot be honored instead of falling back silently.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/frontend/src/features/settings/tabs/voice-tab.tsx Outdated
Comment thread studio/frontend/src/features/settings/tabs/voice-tab.tsx Outdated
Simulated the feature across Chromium, Firefox and WebKit plus node
level unit runs and backend contract checks. Fixes from the findings:

- Dictionary rewrite used a replacement string, so entries containing
  dollar patterns corrupted transcripts (A$$AP became A$AP, $& injected
  the match). Switched to the callback form of String.replace
- Persisted voice settings now validate types on hydration: non string
  micDeviceId, dictationLanguage and ttsVoiceURI, and non boolean
  ttsEnabled fall back to defaults instead of flowing into the UI
- Dictionary entries are trimmed, capped at 120 chars and re-sanitized
  on hydration
- The Test dictation panel now falls back to the default microphone
  when the saved device is unplugged, matching the composer adapter

Test coverage: 46 unit assertions (dictionary regex edge cases across
unicode, word boundaries and injection, voice curation for simulated
macOS, Windows and Linux voice inventories, corrupt storage merge),
13 backend contract checks against /audio/generate on an isolated
instance, and 60 browser assertions across the three engines covering
rendering, degradation without SpeechRecognition, curation in a real
DOM, dictionary persistence with unicode and dollar entries, the
no-model preview error path and corrupt localStorage recovery.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a029e3bd40

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/frontend/src/features/settings/tabs/voice-tab.tsx Outdated
Comment thread studio/frontend/src/features/settings/tabs/voice-tab.tsx Outdated
Comment thread studio/frontend/src/features/chat/shared-composer.tsx Outdated
Verified each review comment before acting. Confirmed and fixed:

- Editing a dictionary entry was broken in two ways: the store trimmed
  on every keystroke so spaces could not be typed, and clearing the
  field deleted the entry and unmounted the input mid edit. Updates now
  keep the raw value and a blur commit trims or removes the entry
- The unplugged mic fallback checked instanceof DOMException, but a
  cross browser probe showed Firefox and WebKit throw
  OverconstrainedError objects that are not DOMExceptions, so the
  fallback never fired there. Matching on the error name now
- When the browser ended a dictation test on its own (silence timeout),
  the mic stream stayed open. All recognition end paths now stop the
  tracks and save the transcript through a single finalize path
- The studio TTS audio element now releases its WAV data URL as soon as
  playback ends, fails or is cancelled
- Allow microphone now reports insecure contexts (no mediaDevices)
  accurately instead of claiming access was blocked
- Voice tab copy moved into i18n keys per src/i18n/AGENTS.md, so locale
  overlays can translate it; en is the baseline and parity passes
- unsloth_voice_settings added to the Reset all local preferences key
  list so voice preferences obey the reset
- Non default microphones note that the system default is used when the
  browser speech engine cannot bind a specific device, since browsers
  without the start(track) overload ignore the argument silently

Re-ran the full simulation set after the changes: 46 unit assertions,
13 backend contract checks and 60 browser assertions across Chromium,
Firefox and WebKit all pass, plus a dedicated browser probe for the
dictionary editing behavior.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2ccc1baa32

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/frontend/src/features/settings/tabs/voice-tab.tsx Outdated
Comment thread studio/frontend/src/features/settings/tabs/voice-tab.tsx
Comment thread studio/frontend/src/components/assistant-ui/thread.tsx
The Voice tab and its buttons used the hugeicons Mic02 glyph while the
chat composer uses a custom filled mic. Extract that composer icon into
a shared lib/mic-icon component, drop the duplicate inline copies in
thread.tsx and shared-composer.tsx, and use it for the Voice tab icon
and the tab's mic buttons so the microphone looks the same everywhere.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 830e333bbe

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Verified each new comment against the current code first. One item was
already fixed in the previous round (recording transcripts when the
browser ends a dictation test on its own). Confirmed and fixed:

- The microphone row showed a picker with generic names when browsers
  enumerate unlabeled devices before permission, leaving no way to
  grant access from the row. It now branches on whether labels are
  visible and shows Allow microphone otherwise
- Compare chat dictation ignored the selected microphone. It now opens
  the chosen device with the same fallback rules as the main adapter,
  passes the track to recognition where supported and releases the
  stream when recognition ends
- Closing the Voice tab cancelled the shared speechSynthesis even when
  read aloud was playing a chat message. Cleanup now only cancels when
  the tab owns an active preview
- Double clicking Start test could race two recognizers and leak the
  first stream. A starting flag set before the getUserMedia await makes
  start reentrancy safe
- Turning off the read aloud setting mid playback removed the only stop
  control. The stop button now renders whenever a message is speaking
- When an engine lacks the start(track) overload, both dictation paths
  now release the selected device stream before retrying with the
  default microphone instead of holding it open
- Read aloud support no longer requires Web Speech synthesis: the
  Unsloth TTS engine only needs audio playback, so it stays available
  in WebViews without speechSynthesis, with a clear error if the system
  engine is chosen there

Not addressed here: cancelling in flight backend TTS generation on
stop. The route runs generation in a worker thread without a
cancellation path, which is shared pre existing behavior with audio
chat generation and belongs in a backend change.

All suites re-run green: 46 unit, 13 backend contract and 60 browser
matrix assertions across Chromium, Firefox and WebKit, plus probes for
the unlabeled device branch and the double click race.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc65256cd2

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/frontend/src/features/chat/shared-composer.tsx Outdated
Comment thread studio/frontend/src/features/settings/tabs/voice-tab.tsx Outdated
Comment thread studio/frontend/src/features/settings/tabs/voice-tab.tsx
Comment thread studio/frontend/src/components/assistant-ui/thread.tsx
Comment thread studio/frontend/src/features/chat/adapters/studio-web-speech-dictation-adapter.ts Outdated
@danielhanchen

Copy link
Copy Markdown
Member

I ran a full review of the Voice tab plus live cross-browser and cross-OS testing against a running Studio. Short version: the feature works well and is safe to merge, and I found (and have a fix for) one real cross-browser crash. Details below.

Walkthrough

Voice tab walkthrough

Voice tab rendering, engine switch (System voices vs Studio TTS model), the dictation dictionary persisting across a full reload, and the "no audio model loaded" preview error surfaced from the backend round trip.

Before / after

Before and after

Settings gains a Voice tab. No existing tab or flow changes.

Cross-browser

Cross browser

Driven with Playwright across Chromium (Chrome/Edge), Firefox, and WebKit (Safari). Dictation gates correctly: Firefox keeps SpeechRecognition behind a flag, so it shows "Not supported in this browser" while the rest of the tab still works. Read aloud stays available everywhere through the Audio fallback.

One real bug found (with a fix)

WebKit/Safari and Linux espeak/festival/flite report speechSynthesis voices whose voiceURI is an empty string and/or is not unique (live repro in WebKit: kal, slt, rms, awb, all with voiceURI ""). The read-aloud picker renders each voice as <Select.Item value={voiceURI}>, and Radix Select throws on an empty-string value and collides on duplicate values, which crashes the whole Voice tab through the error boundary. Chromium and Firefox headless return no voices, so they never hit it, which is why it was easy to miss.

Fix in curateSystemVoices (drop empty and duplicate voiceURIs before building the picker):

+  const seenVoiceURIs = new Set<string>();
   const kept = voices.filter((voice) => {
+    if (!voice.voiceURI || seenVoiceURIs.has(voice.voiceURI)) return false;
+    seenVoiceURIs.add(voice.voiceURI);
     if (LOW_QUALITY_VOICE_NAMES.has(voiceBaseName(voice))) return false;
     return wantedLangs.has(langBase(voice.lang));
   });

With the fix the picker renders a clean, unique list and the tab no longer crashes in any engine (the WebKit panel above is post-fix).

Read aloud and Studio TTS

Tests run

  • Pure-logic unit tests: 36/36. Dictation dictionary regex (unicode, regex metacharacters, $-expansion safety, word boundaries, whitespace tolerance, invalid patterns), value clamping/validation, and the voice dedup/empty filter.
  • Cross-browser (Chromium, Firefox, WebKit): 3/3 render and gate correctly.
  • Robustness sims against the real bundle: 20/20. Messy voices (the fix), corrupt persisted settings (garbage rate/volume/engine/dictionary get sanitized), legacy partial settings (merge with defaults), and mocked dictation end to end including the start(track) to start() fallback.
  • Happy-path E2E: 9/9. Engine switch, dictionary add and persist across a full reload, and the no-model preview error.
  • Cross-OS build (typecheck + i18n parity + vite build): green on ubuntu-latest, windows-latest, and macos-14.

No new backend or hardware path: every change is frontend, and /api/inference/audio/generate already exists and is unchanged (it returns the identical contract on main and this branch). Backwards and forwards compatible too: the new persisted store validates/clamps every field so old or corrupt data degrades to safe defaults, and an older Studio ignores the new voice settings tab value and falls back to General.

Release a microphone opened after the component unmounts, and stop Compare
dictation on a permission or security failure instead of silently recording
from the default device, matching the main chat adapter.
@danielhanchen

Copy link
Copy Markdown
Member

Pushed two commits to this branch from a cross-browser and code review pass.

1. Voice tab crash on Safari/WebKit and Linux (studio-speech-synthesis-adapter.ts)

WebKit and Linux engines (espeak/festival/flite) report speechSynthesis voices whose voiceURI is an empty string and/or is not unique. The read-aloud picker renders each voice as <Select.Item value={voiceURI}>, and Radix Select throws on an empty-string value and collides on duplicates, which crashed the whole Voice tab through the error boundary. Reproduced live in WebKit (kal, slt, rms, awb, all with voiceURI ""). Chromium and Firefox return no voices headless, so they never hit it. curateSystemVoices now drops empty and duplicate URIs before building the picker.

2. Dictation mic lifecycle (voice-tab.tsx, shared-composer.tsx)

  • Unmount race: the Voice test and the Compare composer awaited getUserMedia() and then unconditionally stored the stream and started recognition. Closing Settings or leaving Compare while the permission prompt is open could start the mic on an unmounted component with no stop control. Both now check a disposedRef after the await and release the stream, matching the ended guard the main chat adapter already has.
  • Compare error handling: the Compare composer swallowed every getUserMedia failure and fell back to the default mic, so a NotAllowedError or NotReadableError on the selected device could silently record from a different one. It now stops on permission/security failures, matching the main adapter (which only falls back for missing-device errors).

Verified with tsc -b (typecheck passes). Biome reports only the repo's existing style rules; the new lines follow the surrounding style.

One open item I did not change (needs a call on intent): the dictation dictionary uses each entry as both the match pattern and the replacement, so a stored Unsloth normalizes the casing/whitespace of unsloth but cannot rewrite a two-word un sloth into Unsloth. If the intent is spoken-alias correction (the un sloth example), it needs a { spoken, replacement } model plus a small persisted migration and UI. If the intent is casing and whitespace normalization only, the current code is correct and the wording can be tightened. I can implement either once we decide.

…ttings

- Join final dictation chunks with a space so recorded transcripts do not merge words
- Ignore a stale recognizer onend so a quick stop then restart is not torn down
- Use previewingRef so a double click on TTS preview does not orphan the first request
- Keep the read-aloud stop control visible when a new run starts while a message is spoken
- Stop the dictionary remove button from deleting an adjacent entry on a blur then click race
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: be73912301

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 8546a09cc5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

- Only cancel the shared speechSynthesis for a system-voice preview, so stopping
  a Studio preview no longer stops an unrelated chat read-aloud
- Release the Studio preview audio and its WAV data URL on normal completion
- Iterate every finalized result in Compare dictation so batched phrases are kept
- Cap persisted recent dictations to the last 20 on hydration
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4cc15f3774

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread studio/frontend/src/features/settings/tabs/voice-tab.tsx
Comment thread studio/frontend/src/features/settings/tabs/voice-tab.tsx
… audio

- Copy recent dictations via the copyToClipboard helper so the execCommand
  fallback works in Safari and insecure http LAN contexts
- Release the Studio preview audio when play() rejects, not just on ended/error
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6eef67cb30

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +116 to +119
recentDictations: [
{ text: trimmed, at: Date.now() },
...state.recentDictations,
].slice(0, MAX_RECENT_DICTATIONS),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Cap recent dictation text before persisting

When a user dictates a long passage, this stores the full transcript in the persisted unsloth_voice_settings localStorage entry and only limits the number of entries. A few long dictations can exceed the browser quota; Zustand's persist setItem throws synchronously, and callers such as dictation cleanup don't catch it, which can leave the voice test or dictation stop path in a broken state. Please truncate recent entries or catch quota failures before saving them.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in aefc743. Each recent dictation is now truncated to a max length on save and on hydration (matching the dictionary cap), so the persisted blob stays bounded and long transcripts cannot push localStorage over quota from the dictation cleanup path.

Recent dictations only limited entry count, so a long transcript stored the
full text in the persisted voice settings and a few could exceed the
localStorage quota, throwing synchronously from the uncaught dictation cleanup
path. Truncate each entry on save and on hydration, matching the dictionary cap.
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: aefc743f71

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Resolve the settings-dialog import conflict (keep the Voice tab alongside the
new settings search) and integrate the Voice tab with search: index its
searchable labels in SETTINGS_SEARCH_INDEX and render its icon component in
search results.
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a3e67efb45

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


void (async () => {
try {
const url = await generateStudioTtsAudio(text, controller.signal);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Cancel backend work when stopping Studio TTS

When the Studio TTS engine is used and the user presses Stop before /audio/generate returns, this aborts only the client fetch; the backend route still runs the TTS call inside asyncio.to_thread (studio/backend/routes/inference.py:5255-5256) with no cancellation hook. The UI immediately marks read-aloud/preview as cancelled while the server can keep the GPU busy generating audio, blocking later inference until that work finishes.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The client already aborts the /audio/generate fetch on Stop via AbortController, so this is a backend concern rather than an adapter one. The route runs the model call inside asyncio.to_thread, which cannot be interrupted from outside without cooperative cancellation inside the inference path, and that applies to all inference rather than this Voice settings feature. Out of scope for this PR; the client side already does the right thing.

);
}
try {
recognition.start(audioTrack);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Gate selected-mic path on start(audioTrack) support

When a user selects a non-default microphone in browsers that expose SpeechRecognition but not the experimental start(audioTrack) overload (for example Safari/iOS and Chrome on Android), this call does not reliably throw; the extra argument can be ignored, so recognition listens to the browser default while the selected-device stream remains open. The new microphone picker then appears to honor the saved device but transcribes from a different mic in those environments.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no reliable way to feature-detect the start(audioTrack) overload (both start() and start(track) report the same arity), so we attempt it and fall back to start() on throw. In browsers that silently ignore the track argument the selected mic degrades to the default device and the stream is released at session end. No clean detection-based fix exists, so this stays as a documented platform limitation.

- Deleting a message now stops read-aloud when the spoken message is among
  those removed (including a user prompt's cascaded assistant replies), read at
  click time and guarded so a playback end between render and click cannot
  abort the delete.
- Voice preview now reports playback failures instead of silently resetting
  the button, matching the read-aloud path.
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bea5d9d577

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plans/review_correctness.md Outdated
@@ -0,0 +1,47 @@
# PR #7074 correctness review (fork: correctness) — head a3e67ef

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Remove committed review notes

These new files look like transient PR review notes rather than product documentation: they reference PR #7074, fork heads, past review verdicts, and prior directives. Keeping them in the repo creates stale internal process artifacts that future contributors may mistake for current guidance or validated checks, so if they were only used during review, please drop the plans/ files before merging.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 099eb42. These were internal review scratch notes accidentally swept in by git add -A; they are gone from the PR diff.

);
}
try {
recognition.start(audioTrack);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Gate selected-mic dictation on track support

In browsers where SpeechRecognition exists but the start(audioTrack) overload is unsupported (for example Safari/iOS per Can I Use), extra arguments are ignored rather than throwing, so this call succeeds while recognition still listens to the browser default microphone and the selected getUserMedia stream stays open. That makes a saved non-default mic silently ineffective in those browsers; gate per-device selection/startup on actual track-overload support or fall back before opening the selected stream.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in prior rounds: there is no reliable way to feature-detect the start(audioTrack) overload, so we attempt it and fall back to start() on throw, releasing the selected stream. On engines that ignore the argument the mic degrades to the default device, which is a documented platform limitation with no clean detection-based fix.

!StudioSpeechSynthesisAdapter.systemVoicesSupported()
) {
const session = speakWithStudioModel(text, handleEnd, () => {
if (res.status.type !== "ended") res.status = { type: "running" };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Notify speech subscribers when Studio playback starts

When the Studio TTS engine is used, playback moves from starting to running asynchronously after audio generation, but this callback only mutates res.status and never notifies subscribers. Since assistant-ui's speech adapter contract relies on subscribe updates for async status changes (as shown in its custom adapter example), UI state that reads message.speech.status stays stuck at starting until the utterance ends; notify the subscribers after setting running.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 099eb42. The Studio path now notifies subscribers after the async starting -> running transition, so message.speech.status no longer stays stuck at starting.

const pattern = trimmed.split(/\s+/).map(escapeRegExp).join("\\s+");
try {
const regex = new RegExp(
`(?<![\\p{L}\\p{N}])${pattern}(?![\\p{L}\\p{N}])`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Avoid lookbehind for dictation dictionary

On Safari/iOS versions that support Web Speech dictation but predate RegExp lookbehind support (Safari <16.4), constructing this pattern throws and the catch below silently skips every dictionary entry. In that environment users can add dictionary spellings and dictate text, but none of the corrections apply; use a non-lookbehind boundary check so the feature works wherever dictation does.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 099eb42. The dictionary now captures the leading boundary instead of a lookbehind, so corrections apply on engines that support dictation but not lookbehind (Safari < 16.4). Verified equivalent matching behavior across word-boundary, whitespace-tolerant, adjacent, and CJK-adjacent cases.

- Remove plans/review_*.md scratch files accidentally committed earlier.
- Studio read-aloud now notifies speech subscribers on the async
  starting -> running transition so status does not stay stuck at starting.
- Dictionary correction captures the leading boundary instead of a lookbehind
  so it works on engines with dictation but no lookbehind (Safari < 16.4).
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 099eb429fb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +250 to +252
micDeviceId && micDeviceId !== "default"
? { ...baseAudio, deviceId: { exact: micDeviceId } }
: baseAudio,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detect unsupported track-based dictation before using mic IDs

When a user selects a non-default microphone on browsers that only implement the no-argument SpeechRecognition.start(), the extra audioTrack argument used later is ignored rather than throwing, so this deviceId stream can open successfully while the recognizer still listens to the system default and the fallback path never releases it. The new microphone picker therefore appears to honor the selected device but can transcribe from a different mic in those browsers; gate the selector or explicitly detect start(MediaStreamTrack) support before persisting/using a device ID.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same platform limitation as the earlier rounds: there is no reliable way to feature-detect start(MediaStreamTrack) support (both start() and start(track) report arity 0, and no capability flag exists), so it cannot be detected before persisting a device ID. Gating the selector would remove the mic picker this PR intentionally ships with graceful degradation (selected device where supported, default elsewhere; the stream is released at session end). Leaving as a documented limitation.

@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 099eb429fb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

);
}
try {
recognition.start(audioTrack);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detect unsupported speech-recognition audio tracks

When a user selects a non-default microphone in a browser whose SpeechRecognition.start() does not support the audioTrack parameter (for example Safari; Can I Use also shows older Chromium versions unsupported), the extra argument is ignored rather than throwing, so this catch path never runs. The code keeps the selected stream open but recognition listens to the browser's default mic, making the new microphone picker silently use the wrong input; please explicitly detect/handle unsupported start(audioTrack) and fall back or disable per-device selection in that case.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same platform limitation covered in the prior rounds. Web Speech captures audio internally, and there is no reliable way to detect whether the experimental start(MediaStreamTrack) overload actually binds recognition to the passed track (no capability flag; arity is identical). We attempt it and fall back to start() on throw; where the argument is silently ignored the mic degrades to the default device and the preflight stream is released at session end. Honoring or hiding the selected device reliably would require an STT engine that consumes a captured PCM stream, which is a larger design change owned by the feature author. Leaving as a documented limitation.

@danielhanchen

Copy link
Copy Markdown
Member

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces comprehensive voice capabilities to Unsloth Studio, including a new Voice settings tab, a text-to-speech synthesis adapter supporting both system voices and a backend TTS model, and an enhanced dictation adapter with custom microphone selection and dictionary-based corrections. The feedback highlights a keyboard navigation race condition in the dictionary entry list where deleting an item via keyboard can target the wrong entry due to index-shifting on blur. Additionally, it is recommended to set the playbackRate on Audio instances inside a loadedmetadata event listener to ensure speech speed settings are reliably applied across all browsers.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +610 to +613
// Keep the click from blurring an empty input first, which would
// commit-splice this row and make onClick delete the next one.
onMouseDown={(e) => e.preventDefault()}
onClick={() => removeDictionaryEntry(index)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There is a keyboard navigation race condition here. When a user tabs from the input to the delete button and presses Enter or Space, onMouseDown does not fire, so e.preventDefault() is bypassed. The input's onBlur fires first, calling commitDictionaryEntry(index). If the input was empty, this immediately splices the array and shifts subsequent items. The focused delete button now corresponds to the next item, and the subsequent keypress event will delete that next item instead of the intended one. Consider avoiding immediate deletion on blur, or using unique IDs for dictionary entries instead of array indices to prevent index-shifting race conditions.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6becba1. Tabbing to the row's Remove button no longer commit-splices the emptied row, so keyboard activation deletes the intended entry instead of the next one.

Comment on lines +206 to +208
audio = new Audio(url);
audio.playbackRate = ttsRate;
audio.volume = ttsVolume;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Setting playbackRate on an Audio instance before the media metadata has loaded can be ignored or reset to 1.0 by some browsers (such as Safari and Chrome) once the source loads. To ensure the speed setting is reliably applied, set it inside a loadedmetadata event listener.

      audio = new Audio(url);
      audio.volume = ttsVolume;
      audio.addEventListener("loadedmetadata", () => {
        if (audio) audio.playbackRate = ttsRate;
      });

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1fd25e5. playbackRate is now reapplied on loadedmetadata, so browsers that reset it on load still honor the selected speed.

Comment on lines +450 to +452
const audio = new Audio(url);
audio.playbackRate = ttsRate;
audio.volume = ttsVolume;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Setting playbackRate on an Audio instance before the media metadata has loaded can be ignored or reset to 1.0 by some browsers (such as Safari and Chrome) once the source loads. To ensure the speed setting is reliably applied, set it inside a loadedmetadata event listener.

        const audio = new Audio(url);
        audio.volume = ttsVolume;
        audio.addEventListener("loadedmetadata", () => {
          audio.playbackRate = ttsRate;
        });

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1fd25e5. Same loadedmetadata reapply for the voice preview path.

Tabbing to a just-emptied row's Remove button blurred the input and
commit-spliced the empty row, so with index-keyed rows the button's keyboard
activation deleted the next entry. Skip the commit when focus moves to that
row's Remove button; the existing mouse guard is kept.
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 6becba1382

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Some browsers reset an Audio element's playbackRate to 1 once the source
loads, so the selected speed could be dropped for read-aloud and voice
preview. Reapply it on loadedmetadata in both paths.
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1fd25e5be5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

);
}
try {
recognition.start(audioTrack);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detect unsupported start(audioTrack) before using selected mic

On browsers that expose SpeechRecognition but not the experimental start(MediaStreamTrack) overload, extra arguments to start() are ignored rather than throwing, so this catch-based fallback never runs. In that environment, choosing a non-default microphone opens the selected stream but recognition still captures the browser default mic until the session ends, making the new microphone picker silently ineffective.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same platform limitation as the prior rounds: start(MediaStreamTrack) support cannot be reliably feature-detected (identical arity, no capability flag). We attempt it and fall back to start() on throw; where the argument is silently ignored the mic degrades to the default device and the preflight stream is released at session end. Documented limitation.

const trimmed = entry.trim();
if (!trimmed) continue;
// Whitespace-tolerant pattern so "jane doe" still matches.
const pattern = trimmed.split(/\s+/).map(escapeRegExp).join("\\s+");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Support split dictation for single-word entries

When a user adds a spelling such as Unsloth, speech recognizers can produce the split phrase un sloth; this pattern is built only from whitespace already present in the stored entry, so Unsloth matches unsloth but never un sloth, and storing un sloth would rewrite to that exact split text instead. That leaves the dictation dictionary unable to correct split product or brand names, which is a core use case for this setting.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid observation, but deferred by design: the dictionary stores the exact written spelling only, and auto-matching split-word variants like 'un sloth' would over-match. A separate spoken-alias field is the right solution and is left as a follow-up (the doc comment was narrowed to reflect this scope).

@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 1fd25e5be5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danielhanchen
danielhanchen merged commit 9de8488 into main Jul 15, 2026
40 of 41 checks passed
@danielhanchen
danielhanchen deleted the studio-voice-settings branch July 15, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants