Skip to content

fix: personality preset crash in Design tab (closes #89) - #111

Merged
debpalash merged 1 commit into
mainfrom
fix/issue-89-personality-preset-instruct
May 20, 2026
Merged

fix: personality preset crash in Design tab (closes #89)#111
debpalash merged 1 commit into
mainfrom
fix/issue-89-personality-preset-instruct

Conversation

@debpalash

@debpalash debpalash commented May 20, 2026

Copy link
Copy Markdown
Owner

Summary

Selecting any personality preset on the Design tab (e.g. News Anchor) made the next Synthesize Audio call crash with HTTP 400 from the generate router — the user had to restart the app to recover. Affects all 6 shipped personalities.

This PR ships a minimal data fix to backend/core/personalities.py plus a regression test that pins the failing path.

Reproduction steps (per issue #89)

  1. Open the Design tab.
  2. Click any personality button — try News Anchor.
  3. Click Synthesize Audio.
  4. Observe error toast / generation failure.

Confirmed all 6 prose personality strings raise ValueError against the same code path the runtime uses:

narrator     -> ValueError (#89)
casual       -> ValueError (#89)
news_anchor  -> ValueError (#89)
storyteller  -> ValueError (#89)
corporate    -> ValueError (#89)
energetic    -> ValueError (#89)
6/6 old personalities crash the model

Root cause

backend/core/personalities.py shipped human-readable prose as each personality's instruct value, e.g.

"Speak clearly and professionally like a television news presenter"

OmniVoice's model.generate(instruct=...) runs every instruct string through _resolve_instruct (omnivoice/models/omnivoice.py:1351), which splits on commas and validates each item against a fixed taxonomy defined in omnivoice/utils/voice_design.py (gender, age, pitch, accent, dialect, whisper). Prose like "Speak clearly…" has zero tokens in that vocabulary, so the model raises:

ValueError: Unsupported instruct items found in
Speak clearly and professionally like a television news presenter:
  'Speak clearly and professionally like a television news presenter'
  -> ... (unsupported)

The frontend (CloneDesignTab.jsxapplyPersonality) writes the preset's instruct straight into the synth form (useTTS.js line 108–110 joins it with the comma-separated vdStates), so every personality triggered the crash.

Fix

Map each personality to a comma-separated bundle of valid taxonomy tokens that _resolve_instruct accepts. The original prose moves into a new description field so design intent isn't lost and future UI tooltips can use it. The frontend doesn't render description today (only name + icon), so this is purely additive.

Personality New instruct
narrator middle-aged, low pitch
casual young adult, moderate pitch
news_anchor middle-aged, moderate pitch, american accent
storyteller middle-aged, moderate pitch, british accent
corporate middle-aged, moderate pitch
energetic young adult, high pitch

Regression test

tests/backend/core/test_personalities.py runs every personality's instruct through the exact same _resolve_instruct the runtime calls. The test would have failed on all 6 personalities before this commit; after the fix it passes for all of them.

tests/backend/core/test_personalities.py::test_personality_registry_shape PASSED
tests/backend/core/test_personalities.py::test_get_personality_lookup PASSED
tests/backend/core/test_personalities.py::test_every_personality_instruct_is_accepted_by_resolve_instruct PASSED

Also confirmed the wider tests/backend/core/ slice still passes (25 passed).

Cross-platform / data compatibility

  • Cross-platform: pure-Python data — identical on macOS, Windows, Linux. No platform-specific code.
  • Backward-compat: voice_profiles.instruct column is untouched. No DB migration needed; existing omnivoice_data/ works unchanged.
  • Engine compatibility: no engine code touched. Existing IndexTTS / CosyVoice / etc. installs are unaffected.

Test plan

  • Unit: pytest tests/backend/core/test_personalities.py -v (3/3 pass)
  • Regression: all 6 old prose strings provably fail; all 6 new taxonomy strings provably succeed against _resolve_instruct
  • No other backend core tests regress (pytest tests/backend/core/ — 25/25 pass)
  • Manual smoke (deferred to maintainer): click each personality in the Design tab, click Synthesize, expect audio playback instead of an error toast

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation

    • Enhanced personality presets with description fields and restructured instruction parameters for clarity.
  • Tests

    • Added comprehensive test suite to validate personality registry structure, data integrity, and voice synthesis compatibility.

Review Change Stack

Selecting any personality preset in the Design tab (e.g. "News Anchor")
made the next Synthesize call fail with a 400 ValueError that took the
generation pipeline down — the user had to restart the app.

Root cause
==========
backend/core/personalities.py shipped human-readable prose as each
preset's `instruct` value, e.g.:

    "Speak clearly and professionally like a television news presenter"

OmniVoice's model.generate(instruct=...) runs every instruct string
through _resolve_instruct (omnivoice/models/omnivoice.py:1351), which
splits on commas and validates each item against a fixed taxonomy in
omnivoice/utils/voice_design.py (gender, age, pitch, accent, dialect,
"whisper"). Prose like "Speak clearly..." has zero tokens in that
vocabulary, so the model raises:

    ValueError: Unsupported instruct items found in
    Speak clearly and professionally like a television news presenter:
      'Speak clearly and professionally like a television news presenter'
      -> ... (unsupported)

The frontend (CloneDesignTab.jsx applyPersonality) writes the preset's
`instruct` straight into the synth form, so every one of the six
personalities triggered the crash — verified all six raise ValueError.

Fix
===
Map each personality to a comma-separated bundle of valid taxonomy
tokens that _resolve_instruct accepts. Kept the original prose as a new
`description` field for any future UI tooltips and so the design intent
isn't lost.

Verified personalities now round-trip cleanly:

    narrator     -> "middle-aged, low pitch"
    casual       -> "young adult, moderate pitch"
    news_anchor  -> "middle-aged, moderate pitch, american accent"
    storyteller  -> "middle-aged, moderate pitch, british accent"
    corporate    -> "middle-aged, moderate pitch"
    energetic    -> "young adult, high pitch"

Regression test
===============
tests/backend/core/test_personalities.py exercises the exact failing
path: every personality is fed through the same _resolve_instruct that
the runtime calls. The test would have failed on every shipped
personality before this commit.

Cross-platform / data compatibility
===================================
Pure-Python data change — same on macOS / Windows / Linux. No DB
schema or omnivoice_data/ migration: voice_profiles.instruct is
untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR updates the personality registry in backend/core/personalities.py to use comma-separated taxonomy tokens in instruct fields instead of prose, adds a description field to each personality, and introduces comprehensive regression tests to validate the new structure and prevent synthesis crashes from invalid personality configurations.

Changes

Personality Data Structure and Validation

Layer / File(s) Summary
Personality data structure and documentation
backend/core/personalities.py
Module docstring updated to explain taxonomy-token validation and prior prose-based failures. The PERSONALITIES constant reworked so each preset entry includes id, name, instruct (as comma-separated token lists), description, and icon.
Personality registry and validation tests
tests/backend/core/test_personalities.py
New test module with conditional torch/omnivoice dependency imports. Tests verify registry shape (non-empty, required fields, unique ids), get_personality() lookup behavior, and that all personality instruct values pass _resolve_instruct() validation without errors.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 The personalities now speak in tokens true,
No prose to confuse the voice that's new,
Descriptions added, tests in place,
Crash-free synthesis keeps up the pace! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main fix: resolving a personality preset crash in the Design tab by correcting instruct data to use valid taxonomy tokens instead of prose.
Description check ✅ Passed The description is comprehensive and follows the template structure with Summary, Changes (via Root cause/Fix sections), Type (Bug fix implied), Testing, and detailed regression test coverage. Includes reproduction steps, root cause analysis, and fix explanation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-89-personality-preset-instruct

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai 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.

🧹 Nitpick comments (1)
tests/backend/core/test_personalities.py (1)

59-63: ⚡ Quick win

Assert description in the schema contract test.

The registry test should validate the new description field too, otherwise this PR’s schema addition can regress silently.

Patch suggestion
-        for key in ("id", "name", "instruct", "icon"):
+        for key in ("id", "name", "instruct", "description", "icon"):
             assert key in p, f"personality {p.get('name')!r} missing {key!r}"
             assert isinstance(p[key], str), (
                 f"personality {p.get('name')!r} field {key!r} must be a string"
             )
🤖 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/backend/core/test_personalities.py` around lines 59 - 63, The test loop
validating personality schema omits the new "description" field; update the
check that iterates keys ("id","name","instruct","icon") to also include
"description" and add the same type assertion for p["description"] (i.e., assert
the key exists on p and assert isinstance(p["description"], str)) so the
registry test covers the new schema field; locate the loop using variable p in
the test_personalities test and modify it accordingly.
🤖 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.

Nitpick comments:
In `@tests/backend/core/test_personalities.py`:
- Around line 59-63: The test loop validating personality schema omits the new
"description" field; update the check that iterates keys
("id","name","instruct","icon") to also include "description" and add the same
type assertion for p["description"] (i.e., assert the key exists on p and assert
isinstance(p["description"], str)) so the registry test covers the new schema
field; locate the loop using variable p in the test_personalities test and
modify it accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 495c2f5b-fa5c-44d0-a583-432ab573f765

📥 Commits

Reviewing files that changed from the base of the PR and between 1edd35c and 41a96bc.

📒 Files selected for processing (2)
  • backend/core/personalities.py
  • tests/backend/core/test_personalities.py

@debpalash
debpalash merged commit 0588a2a into main May 20, 2026
8 checks passed
@debpalash
debpalash deleted the fix/issue-89-personality-preset-instruct branch May 20, 2026 08:31
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.

1 participant