[codex] PR_26171_061 text2speech engine audio feature parity#18
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4541d6371
ℹ️ 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".
| return { ok: false, message: `${TEXT_TO_SPEECH_DISPLAY_NAME} ${sourceLabel}[${index}] missing: ${missing.join(", ")}.` }; | ||
| } | ||
| } | ||
| return { ok: true }; |
There was a problem hiding this comment.
Validate payload fields before accepting JSON
When Import JSON, samplePresetPath, or Project Workspace state reaches this return, the payload has only been checked for required property names. Schema-invalid values such as rate: "fast", out-of-range numbers, invalid enums, or extra persisted fields still get accepted and can then be copied/exported or written back as valid Text to Speech V2 data, even though the schema rejects them. Please validate the full text2speech-V2 schema, or normalize/reject invalid fields, before returning success.
Useful? React with 👍 / 👎.
| await loadQueue(); | ||
| refreshVoices({ selectedVoice: selectedItem()?.voice || elements.voice?.value || "" }); | ||
| writeStatus(`${TEXT_TO_SPEECH_DISPLAY_NAME} ready. SpeechSynthesis is available.`); |
There was a problem hiding this comment.
Preserve load failures during startup
When loadQueue() logs a failure for an invalid samplePresetPath or missing/invalid Project Workspace toolState, it returns without signaling failure, so startup continues here and overwrites the visible failure with “ready”. In those launch contexts users see a ready empty tool instead of the broken preset/workspace handoff error. Have loadQueue() return success/failure and skip the ready status path when loading failed.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| function workspaceManagerUrl() { | ||
| const url = new URL("../workspace-manager-v2/index.html", window.location.href); |
There was a problem hiding this comment.
Return to an existing workspace route
When the tool is launched with the Project Workspace params, clicking “Return to Project Workspace” sends users to ../workspace-manager-v2/index.html, but the active toolbox tree does not contain toolbox/workspace-manager-v2/ (I checked the active toolbox directories; only archived copies exist). This makes the restored return action navigate to a missing page instead of back to the workspace.
Useful? React with 👍 / 👎.
| if (elements.pitch) elements.pitch.value = String(item.pitch); | ||
| if (elements.volume) elements.volume.value = String(item.volume); | ||
| syncRangeOutputs(); | ||
| refreshVoices({ selectedVoice: item.voice }); |
There was a problem hiding this comment.
Hydrate each item's saved language
When selecting an imported/workspace/sample queue item whose language is not the current dropdown value, this refresh uses the old dropdown language because selectItem() never assigns item.language to elements.language first. The voice list is filtered for the wrong language, and the next edit or Speak sync overwrites the saved item with that wrong language/voice selection. Set the language control from the item before refreshing voices.
Useful? React with 👍 / 👎.
| if (state.queue.length === 0) { | ||
| state.queue = [createTextToSpeechQueueItem({ | ||
| ...TEXT_TO_SPEECH_DEFAULTS, | ||
| existingItems: [], | ||
| name: "Narrator welcome", | ||
| text: elements.text?.value || "Welcome to the arena, hero." | ||
| })]; |
There was a problem hiding this comment.
Do not fabricate payload items on empty launches
For a standalone launch with no samplePresetPath, an empty imported/workspace payload, or an empty sample root array, this creates a real Narrator welcome queue item. That means Copy/Export or any later workspace dirty writeback can persist a speech line the user never loaded or added, instead of preserving a valid empty Text to Speech V2 payload.
Useful? React with 👍 / 👎.
| if (elements.rate) elements.rate.value = String(item.rate); | ||
| if (elements.pitch) elements.pitch.value = String(item.pitch); | ||
| if (elements.volume) elements.volume.value = String(item.volume); |
There was a problem hiding this comment.
Reset slider override state on item selection
After a user manually changes a slider on one speech item, state.sliderOverrides remains true when another queue item is selected. These lines hydrate the new item's numeric values but leave the old override flags in place, so changing Voice Age or SSML-like preset on the newly selected item will unexpectedly refuse to reshape the inherited overridden sliders. Reset the override flags when applying a different item, as the old control did.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e898aae3cc
ℹ️ 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".
| setTextContent(root, "[data-tts-engine-status]", "Browser SpeechSynthesis is available for local preview."); | ||
| refreshVoices(); | ||
| engine.onVoicesChanged(() => { | ||
| refreshVoices(); |
There was a problem hiding this comment.
Preserve saved voice after async voice loading
When a workspace/sample item is loaded before SpeechSynthesis has populated voices, selectItem() cannot apply item.voice and leaves the voice select empty. This voiceschanged refresh then repopulates without the selected queue item's saved voice, so refreshVoices() chooses the first matching voice instead; the next edit or Speak syncs that wrong voice back into the item. Pass selectedItem()?.voice into this refresh so saved voices are hydrated when browser voices arrive asynchronously.
Useful? React with 👍 / 👎.
| pitch: boundedNumber(options.pitch, TEXT_TO_SPEECH_RANGE_DEFAULTS.pitch), | ||
| rate: boundedNumber(options.rate, TEXT_TO_SPEECH_RANGE_DEFAULTS.rate), | ||
| ssmlLikePreset: options.ssmlLikePreset || TEXT_TO_SPEECH_DEFAULTS.ssmlLikePreset, | ||
| text: String(text || "").trim() || "New speech line.", |
There was a problem hiding this comment.
Do not replace cleared text with a placeholder
When a user clears a selected speech line, the textarea input handler syncs through this helper and stores "New speech line." in state.queue/workspace session instead of the blank value the user sees. That makes Output Summary, Copy/Export, or workspace Save persist a fabricated sentence while Speak remains disabled because the control itself is empty; reserve this fallback for explicit Add/default creation paths, not edits of an existing item.
Useful? React with 👍 / 👎.
| return params.get("launch") === "workspace" | ||
| && params.get("fromTool") === "workspace-manager-v2" | ||
| && Boolean(params.get("hostContextId")); |
There was a problem hiding this comment.
Fail workspace-intent launches without context
When the URL still has launch=workspace&fromTool=workspace-manager-v2 but the hostContextId parameter is missing or stripped, this returns false and the page falls into standalone mode, showing JSON actions and creating/editing a local draft with no workspace writeback. Treat that workspace intent as a handoff failure instead, so users do not lose the Project Workspace context silently.
Useful? React with 👍 / 👎.
| if (elements.speak) elements.speak.disabled = !(supported && hasText && hasVoice && selectedItem()); | ||
| if (elements.pause) elements.pause.disabled = !(engine.canPause() && selectedItem()); | ||
| if (elements.resume) elements.resume.disabled = !(engine.canResume() && selectedItem()); | ||
| if (elements.stop) elements.stop.disabled = !(supported && selectedItem()); |
There was a problem hiding this comment.
Keep Stop available after deleting the selected item
If a user starts speech and then deletes the last/selected sentence while it is still playing, selectedItem() becomes null and this disables Stop even though TextToSpeechEngine may still have speech queued in the browser. Because Stop cancels global SpeechSynthesis playback rather than editing a queue item, gate it on engine support instead of requiring a selected item so users can still halt audio after queue edits.
Useful? React with 👍 / 👎.
Summary
old_text2speech-V2functionality sample.src/engine/audio/.toolbox/text-to-speech/to consume the engine module with Theme V2 and external JavaScript only.Validation
node --check src\engine\audio\TextToSpeechEngine.jsnode --check toolbox\text-to-speech\text2speech.jsnode --test tests\tools\Text2SpeechShell.test.mjsnpx playwright test tests/playwright/tools/TextToSpeechFunctional.spec.mjsnpm run test:workspace-v2(legacy command name; user-facing language is Project Workspace)git diff --checkNotes
tools/text2speech/path.