feat(tts): add provider personas#70748
Conversation
openclaw-barnacle
Bot
added
docs
barronlroth
force-pushed
the
feat/tts-personas-speech-core
branch
2 times, most recently
from
b7c4525 to
17a5dce
Compare
Greptile SummaryThis PR adds first-class TTS persona support to OpenClaw: persona config schema, runtime resolution, provider-specific synthesis preparation hooks (Gemini audio-profile-v1 wrapping, OpenAI instructions injection), CLI/chat command/gateway control surfaces, and status reporting. The overall design is sound — persona resolution is deterministic, the One minor telemetry accuracy issue: in the synthesis loops, skipped-provider attempt records hardcode Note also that Google TTS persona prompt fields ( Confidence Score: 5/5Safe to merge; all remaining findings are P2 style/telemetry concerns that do not affect synthesis correctness or security. No P0 or P1 findings. The one substantive issue (incorrect personaBinding label in skipped-attempt telemetry) is a cosmetic inaccuracy in status output and does not affect actual TTS behavior, provider selection, or data integrity. The persona resolution logic, fallback policy enforcement, provider hooks, and all control surfaces are correctly wired. extensions/speech-core/src/tts.ts — the skipped-provider personaBinding labeling in both synthesis loops. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/speech-core/src/tts.ts
Line: 1037-1044
Comment:
**`personaBinding` incorrectly hardcoded to `"missing"` on skip**
When a provider is skipped due to `no_provider_registered` or `unsupported_for_telephony`, the code sets `personaBinding: persona ? "missing" : "none"` unconditionally. But a persona could have a provider-specific binding configured for that provider even when it was skipped for an unrelated reason. The same issue appears in the telephony loop. This produces misleading telemetry — users inspecting `/tts status` attempt details would see `persona=...:missing` even when the persona binding was not the skip cause.
The real binding state is only knowable after calling `mergeProviderConfigWithPersona`, which doesn't run on the skip path. The simplest fix is to omit `personaBinding` entirely from skipped-provider attempt records, or to explicitly note the binding as `"unknown"`.
```suggestion
attempts.push({
provider,
outcome: "skipped",
reasonCode: resolvedProvider.reasonCode,
persona: persona?.id,
personaBinding: persona ? "unknown" : "none",
error: resolvedProvider.message,
});
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "TTS: add provider personas" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
barronlroth
force-pushed
the
feat/tts-personas-speech-core
branch
from
17a5dce to
d568af4
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d568af45e5
ℹ️ 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".
barronlroth
force-pushed
the
feat/tts-personas-speech-core
branch
2 times, most recently
from
db06b68 to
6b52167
Compare
obviyus
left a comment
obviyus
left a comment
There was a problem hiding this comment.
Thanks for the work here. Can you please make these changes before we land it?
- Drop the
docs/.generated/*.sha256changes from this PR. - Rebase on latest
mainto clear the dirty merge state. - Clarify
fallbackPolicy: "fail"semantics, ideally with a small test: should it fail the whole persona request, or only skip providers without persona bindings?
|
Maintainer deep review against current Current state:
Blockers before this can be reconsidered:
This is a substantial API/product feature, not a narrow repair. Once rebased and narrowed around the resolved semantics, it can be reviewed as the canonical #68323 implementation. |
barronlroth
force-pushed
the
feat/tts-personas-speech-core
branch
from
6b52167 to
50b713c
Compare
|
Fresh rebase/update pushed in Addressed the requested follow-ups:
Validation after the final rebase:
|
steipete
left a comment
steipete
left a comment
There was a problem hiding this comment.
Deep reviewed the refreshed head 50b713ca3. This is closer, but I still would not land it yet.
Blocking findings:
-
Rebase is needed again. Current
mainhas moved since the PR head; local merge proof shows conflicts indocs/.generated/config-baseline.sha256anddocs/.generated/plugin-sdk-api-baseline.sha256(git merge-tree $(git merge-base main refs/tmp/pr-70748) main refs/tmp/pr-70748). GitHub still reportsmergeable: UNKNOWNandreviewDecision: CHANGES_REQUESTED. Please rebase on currentmainand regenerate/check the baselines after the rebase. -
persona.rewriteis a public no-op config surface. The PR adds and exportsTtsPersonaRewriteConfiginsrc/config/types.tts.ts, accepts it inTtsPersonaRewriteSchema/TtsPersonaSchema, exports it through the plugin SDK, and even accepts it insrc/config/zod-schema.tts.test.ts, but runtime grep shows no implementation path consuming it (git grep -n "TtsPersonaRewrite\|rewrite:" refs/tmp/pr-70748 -- src extensions docs/tools/tts.md). This means users can setmessages.tts.personas.<id>.rewrite.enabled=trueand it silently does nothing. Please either implement the rewrite behavior with tests/docs, or remove therewriteschema/types/export until it exists.
Non-blocking note:
- Google TTS model naming is still awkward. Official Google Gemini TTS docs currently list Gemini 2.5 Flash/Pro Preview TTS as supported; the PR docs/example and catalog include
gemini-3.1-flash-tts-preview. My.profilelive Google TTS smoke on current main passed, so I am not treating this as a blocker, but the PR should avoid presenting an undocumented model as the recommended example unless we have an explicit source or live proof attached.
Validation I ran locally while reviewing:
pnpm docs:listOPENCLAW_LIVE_TEST=1 OPENCLAW_VITEST_MAX_WORKERS=1 pnpm test:live extensions/google/google.live.test.ts -t "synthesizes speech through the registered provider"-> passed- local merge-tree conflict check against current
main
Acknowledged - on it! |
barronlroth
force-pushed
the
feat/tts-personas-speech-core
branch
from
50b713c to
4a0a473
Compare
|
@steipete addressed your latest review on refreshed head
Validation after the final rebase:
One unrelated local gate note: |
barronlroth
force-pushed
the
feat/tts-personas-speech-core
branch
from
4a0a473 to
29bbb70
Compare
|
Follow-up CI fix pushed in Addressed the failing checks from the prior head:
Rebased again on latest Validation after the rebase:
|
barronlroth
force-pushed
the
feat/tts-personas-speech-core
branch
from
29bbb70 to
218bc56
Compare
|
Maintainer re-review after the per-agent TTS work landed on This PR is still solving a real remaining feature: provider personas are not covered by This is not ready to land yet:
Best path to land:
Keeping this open because #68323 remains a valid gap, but it needs the above cleanup before merge. |
obviyus
force-pushed
the
feat/tts-personas-speech-core
branch
2 times, most recently
from
c0e8037 to
e932fc5
Compare
|
Maintainer patch pushed in Addressed the re-review items: rebased on current Local proof:
|
obviyus
force-pushed
the
feat/tts-personas-speech-core
branch
from
e932fc5 to
ac8aa08
Compare
obviyus
force-pushed
the
feat/tts-personas-speech-core
branch
from
ac8aa08 to
ab2d31c
Compare
obviyus
left a comment
obviyus
left a comment
There was a problem hiding this comment.
Verified provider personas compose with per-agent TTS config instead of duplicating the agent merge path, including /tts persona status and composed provider override coverage.
Maintainer follow-up: rebased onto current main, removed the generated sha256 baseline churn, and moved the changelog entry into the active release block.
Local gate: pnpm config:schema:check, focused TTS command/status/speech-core tests, and pnpm build.
|
Landed on main. Thanks @barronlroth. |
The TTS doc had grown to 1008 lines with 11 separate flat 'X primary' config blocks, a 100-line dense 'Notes on fields' bullet list, and the new provider-personas feature (#70748) buried near the bottom. Restructure for readability and feature visibility: - Lead with a Steps-based 'Quick start' so first-time readers can enable TTS in 4 explicit steps. - Replace the 13-bullet provider list with a single 'Supported providers' table that names auth env vars and per-provider notes inline. Add a Warning callout for the Microsoft/edge legacy alias. - Collapse the 11 'X primary' config blocks into one Tabs component ('OpenAI + ElevenLabs', 'Google Gemini', 'Azure Speech', 'Microsoft (no key)', 'MiniMax', 'Inworld', 'xAI', 'Volcengine', 'Xiaomi MiMo', 'OpenRouter', 'Gradium', 'Local CLI') so users see one preset at a time and the page is scannable. - Promote 'Personas' to its own top-level section with two examples (minimal and the Alfred provider-neutral persona), and add a new 'How providers use persona prompts' AccordionGroup covering Google (promptTemplate audio-profile-v1, personaPrompt), OpenAI (instructions auto-mapping), and Other providers, plus a fallback policy table. - Note that agents.list[].tts.persona overrides global persona per-agent (covers the recent feat(tts) per-agent voice-override work). - Convert the 100-line 'Notes on fields' wall into a per-provider AccordionGroup using ParamField, so the field reference is scannable and field types/defaults are visually distinct. - Sentence-case headings, drop redundant body H1, fold the flow diagram inline with Auto-TTS behavior, and refresh the Output formats section to a table-first layout. - Schema fields (label/description/provider/fallbackPolicy/prompt with profile/scene/sampleContext/style/accent/pacing/constraints and providers map) verified against src/config/types.tts.ts; all defaults and env-var fallbacks preserved verbatim. Net diff: 585 insertions, 684 deletions across the same surface area.
3 participants
Summary
AI-assisted: Yes.
Testing degree: targeted local validation on the rebased head; broader full-gate runs existed earlier in the branch, but I did not rerun the entire
pnpm build && pnpm check && pnpm testsequence after the final rebase before opening this draft.codex review --base upstream/mainis installed locally but could not run in this environment because~/.codex/sessionsis permission-blocked.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
Regression Test Plan (if applicable)
N/A for the feature framing, but the PR adds targeted seam/integration coverage for persona resolution and provider-specific synthesis preparation.
extensions/speech-core/src/tts.test.ts,extensions/google/speech-provider.test.ts,extensions/openai/speech-provider.test.ts,src/auto-reply/reply/commands-tts.test.ts,src/gateway/server-methods/tts.test.ts,src/cli/capability-cli.test.ts,src/config/zod-schema.tts.test.ts,test/helpers/plugins/tts-contract-suites.tsUser-visible / Behavior Changes
messages.tts.personaandmessages.tts.personasconfig surfaces./tts persona [id|off]chat command.Diagram (if applicable)
Security Impact (required)
Yes/No): YesYes/No): NoYes/No): NoYes/No): NoYes/No): NoYes, explain risk + mitigation: this adds persona selection/read APIs and command surfaces only within the existing TTS feature area. Persona ids are schema-validated, provider mapping stays inside the provider/runtime seam, and there are no new secret or network paths.Repro + Verification
Environment
messages.tts.personas,messages.tts.persona, provider TTS configSteps
/tts persona, CLI, or gateway method.Expected
Actual
Evidence
Attach at least one:
Validation run on this branch included:
pnpm plugin-sdk:api:checkpnpm config:docs:checkpnpm tsgopnpm tsgo:testpnpm checkAdditional follow-up during local prep validated an unrelated gateway socket-teardown regression outside this PR and was intentionally left uncommitted.
Human Verification (required)
What you personally verified (not just CI), and how:
Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes/No): YesYes/No): YesYes/No): NoRisks and Mitigations
prepareSynthesishooks plus targeted provider contract tests.