-
Notifications
You must be signed in to change notification settings - Fork 0
PR_26171_BETA_081-message-playback-through-tts-engine #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # PR_26171_BETA_081 Manual Validation Notes | ||
|
|
||
| ## Review | ||
| - Confirmed /api/messages/tts-profiles returns server-owned emotionSettings for playback. | ||
| - Confirmed Play Message queues each active Message Part through the TextToSpeechEngine-backed registry. | ||
| - Confirmed Play Part uses the selected part TTS Profile and matching Emotion Setting values. | ||
| - Confirmed Stop continues through the TextToSpeechEngine-backed registry stop path. | ||
| - Confirmed missing Emotion Settings produce a visible validation error instead of falling back silently. | ||
|
|
||
| ## Manual Browser Coverage | ||
| - Covered by targeted Playwright validation for Message Studio playback, TTS profile emotion settings, Play Message, Play Part, and Stop. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # PR_26171_BETA_081 Validation Report | ||
|
|
||
| ## Commands | ||
| - PASS: node --check toolbox/messages/messages.js | ||
| - PASS: node --check src/dev-runtime/messages/messages-sqlite-service.mjs | ||
| - PASS: npx playwright test tests/playwright/tools/MessagesTool.spec.mjs | ||
| - PASS: npx playwright test tests/playwright/tools/TextToSpeechFunctional.spec.mjs --reporter=list | ||
| - PASS: node --test tests/tools/Text2SpeechShell.test.mjs | ||
| - PASS: npm run test:workspace-v2 | ||
| - PASS: git diff --check | ||
|
|
||
| ## Targeted Results | ||
| - Message Studio Playwright tests: 2 passed. | ||
| - Text To Speech compatibility Playwright tests: 3 passed. | ||
| - Text2Speech Node contract tests: 4 passed. | ||
| - Project Workspace legacy validation: 5 passed. | ||
|
|
||
| ## Notes | ||
| - A parallel Playwright run caused an HTML reporter file-copy collision after tests passed; the TTS compatibility lane was rerun with the list reporter and passed cleanly. | ||
| - npm run test:workspace-v2 is a legacy command name; user-facing language remains Project Workspace. | ||
| - Standard generated validation-report churn was restored before staging this PR. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # PR_26171_BETA_081-message-playback-through-tts-engine | ||
|
|
||
| ## Team Ownership | ||
| - TEAM: BETA | ||
| - Ownership source: docs_build/dev/PROJECT_MULTI_PC.txt | ||
| - Scope confirmed: Audio, Messages, Text To Speech, and TTS are owned by Team BETA. | ||
|
|
||
| ## Summary | ||
| - Added server-owned Emotion Settings to Messages TTS profile API responses without changing database schema. | ||
| - Updated Message Studio playback readiness to require a matching Emotion Setting on the selected TTS Profile. | ||
| - Wired Play Part and Play Message playback values through the existing TextToSpeechEngine registry path using selected TTS Profile language/voice and Emotion Setting pitch/rate/volume/preset. | ||
| - Kept Stop routed through the existing TextToSpeechEngine stop path. | ||
|
|
||
| ## Scope Guard | ||
| - No database schema changes. | ||
| - No engine core changes. | ||
| - No silent playback fallback when a selected TTS Profile lacks the selected Emotion Setting. | ||
| - Theme V2 and external JS only. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| docs_build/dev/reports/codex_changed_files.txt | ||
| docs_build/dev/reports/codex_review.diff | ||
| docs_build/dev/reports/PR_26171_BETA_079-message-studio-parent-child-table-completion-manual-validation-notes.md | ||
| docs_build/dev/reports/PR_26171_BETA_079-message-studio-parent-child-table-completion-validation-report.md | ||
| docs_build/dev/reports/PR_26171_BETA_079-message-studio-parent-child-table-completion.md | ||
| docs_build/dev/reports/PR_26171_BETA_081-message-playback-through-tts-engine-manual-validation-notes.md | ||
| docs_build/dev/reports/PR_26171_BETA_081-message-playback-through-tts-engine-validation-report.md | ||
| docs_build/dev/reports/PR_26171_BETA_081-message-playback-through-tts-engine.md | ||
| src/dev-runtime/messages/messages-sqlite-service.mjs | ||
| tests/playwright/tools/MessagesTool.spec.mjs | ||
| toolbox/messages/messages.js |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,9 +12,16 @@ import { createMessageStudioTtsServiceRegistry } from "./message-tts-service-reg | |
|
|
||
| const NEW_ROW_KEY = "__new__"; | ||
| const DEFAULT_TTS_PROFILE_KEY = "__default-balanced-tts__"; | ||
| const DEFAULT_TTS_EMOTION_SETTINGS = Object.freeze([ | ||
| Object.freeze({ active: true, emotion: "calm", emotionLabel: "Calm", pitch: 1, rate: 1, ssmlLikePreset: "normal", volume: 1 }), | ||
| Object.freeze({ active: true, emotion: "urgent", emotionLabel: "Urgent", pitch: 1.08, rate: 1.15, ssmlLikePreset: "normal", volume: 1 }), | ||
| Object.freeze({ active: true, emotion: "whisper", emotionLabel: "Whisper", pitch: 0.95, rate: 0.9, ssmlLikePreset: "normal", volume: 0.55 }), | ||
| Object.freeze({ active: true, emotion: "angry", emotionLabel: "Angry", pitch: 0.98, rate: 1.1, ssmlLikePreset: "normal", volume: 1 }), | ||
| ]); | ||
| const DEFAULT_TTS_PROFILE = Object.freeze({ | ||
| active: true, | ||
| description: "Balanced local browser playback option until authored TTS profiles are available.", | ||
| emotionSettings: DEFAULT_TTS_EMOTION_SETTINGS, | ||
| key: DEFAULT_TTS_PROFILE_KEY, | ||
| language: "en-US", | ||
| name: "Default Balanced TTS Profile", | ||
|
|
@@ -264,6 +271,32 @@ function emotionProfileByKey(profileKey) { | |
| return state.emotionProfiles.find((profile) => profile.key === profileKey) || null; | ||
| } | ||
|
|
||
| function emotionSettingKey(value) { | ||
| return String(value || "") | ||
| .trim() | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9]+/g, "-") | ||
| .replace(/^-+|-+$/g, "") || "neutral"; | ||
| } | ||
|
|
||
| function selectedEmotionSettingForProfile(profile, emotionProfile) { | ||
| const settings = Array.isArray(profile?.emotionSettings) | ||
| ? profile.emotionSettings.filter((setting) => setting?.active !== false) | ||
| : []; | ||
| const selectedEmotionKey = emotionSettingKey(emotionProfile?.name); | ||
| const setting = settings.find((candidate) => ( | ||
| emotionSettingKey(candidate.emotion) === selectedEmotionKey | ||
| || emotionSettingKey(candidate.emotionLabel) === selectedEmotionKey | ||
| )); | ||
|
Comment on lines
+287
to
+290
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If two active Emotion Profiles have distinct names that normalize to the same slug, such as Useful? React with 👍 / 👎. |
||
| if (!setting) { | ||
| return { | ||
| message: `Selected TTS Profile "${profile?.name || "Unknown"}" does not include an Emotion Setting for "${emotionProfile?.name || "Unknown"}".`, | ||
| ok: false, | ||
| }; | ||
| } | ||
| return { ok: true, setting }; | ||
| } | ||
|
|
||
| function activeTtsProfileOptions() { | ||
| const activeProfiles = state.ttsProfiles.filter((profile) => profile.active); | ||
| return activeProfiles.length ? activeProfiles : [DEFAULT_TTS_PROFILE]; | ||
|
|
@@ -428,6 +461,10 @@ function speechTestReadiness() { | |
| if (!target.emotionProfile) { | ||
| return { message: "Selected item needs an Emotion before testing speech.", ok: false }; | ||
| } | ||
| const emotionSetting = selectedEmotionSettingForProfile(profile, target.emotionProfile); | ||
| if (!emotionSetting.ok) { | ||
| return { message: emotionSetting.message, ok: false }; | ||
|
Comment on lines
+464
to
+466
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a message or part has its own TTS profile selected in the row, Useful? React with 👍 / 👎. |
||
| } | ||
| if (!String(target.text || "").trim()) { | ||
| return { message: "Selected item needs message text before testing speech.", ok: false }; | ||
| } | ||
|
|
@@ -923,18 +960,23 @@ function speakTarget(service, target, profile) { | |
| if (!target.emotionProfile) { | ||
| return visiblePlaybackError("Selected message or part needs an Emotion before playback."); | ||
| } | ||
| const emotionSetting = selectedEmotionSettingForProfile(profile, target.emotionProfile); | ||
| if (!emotionSetting.ok) { | ||
| return visiblePlaybackError(emotionSetting.message); | ||
| } | ||
|
Comment on lines
+963
to
+966
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Play Message is used on a multi-part message and a later part lacks a matching Emotion Setting, earlier loop iterations have already called Useful? React with 👍 / 👎. |
||
| if (!String(target.text || "").trim()) { | ||
| return visiblePlaybackError("Selected message or part needs text before playback."); | ||
| } | ||
| return ttsServiceRegistry.speak(service.key, { | ||
| language: profile.language, | ||
| pitch: target.emotionProfile.pitch ?? profile.pitch ?? 1, | ||
| rate: target.emotionProfile.rate ?? profile.rate ?? 1, | ||
| pitch: emotionSetting.setting.pitch ?? profile.pitch ?? 1, | ||
| rate: emotionSetting.setting.rate ?? profile.rate ?? 1, | ||
| speechItemId: target.id, | ||
| speechItemName: target.name, | ||
| ssmlLikePreset: emotionSetting.setting.ssmlLikePreset || "normal", | ||
| text: target.text, | ||
| voice: profile.voiceName, | ||
| volume: target.emotionProfile.volume ?? profile.volume ?? 1, | ||
| volume: emotionSetting.setting.volume ?? profile.volume ?? 1, | ||
| }); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When there are no active API TTS profiles,
activeTtsProfileOptions()falls back toDEFAULT_TTS_PROFILE; however the fallback Emotion Settings added here only cover Calm/Urgent/Whisper/Angry while the seeded active emotions also include Excited, Sad, and Mysterious. In that no-active-profile fallback path, any message or part using one of the omitted seeded emotions now fails inspeakTarget()with a missing Emotion Setting error instead of playing with the emotion values as it did before. Include all seeded emotions or derive these fallback settings fromstate.emotionProfiles.Useful? React with 👍 / 👎.