fix(clone): voice-design panel no longer crashes on a partial vd_states shape (#983) (#995)

Crash: DesignMethodPanel's optLabel() called val.replace(...) on an
undefined category value — a regression from f678e33, which swapped a
safe plain template literal for an i18n lookup that assumes vdStates[key]
is always a string. Both occurrences (the label kicker and the chip
list) are now guarded, falling back to 'Auto' the same way the rest of
the component treats an unset category.

Root cause: vdStates could actually go partial in real usage. Selecting
a design profile (useProfiles.js) or restoring legacy localStorage state
(useAppData.js) applied the backend/stored vd_states object as-is, with
no check that all 6 CATEGORIES keys were present — so an older client,
hand-edited payload, or partial API write reproduced the crash on
selection. Both call sites now run the restored object through
mergeDescribedAttrs() (voiceInstruct.js), the existing completion helper
already used for the "describe your voice" path, which fills any
missing/unknown category with 'Auto'. useAppData.js also gained the
typeof === 'object' guard useProfiles.js already had.

Closes the class at the source: POST /profiles now completes vd_states
against CATEGORY_ORDER (core/describe_voice.py, the same list the
frontend's CATEGORIES mirrors) before persisting, so a design profile
can never be *saved* with an incomplete shape regardless of which
client wrote it — updated two existing tests whose fixtures asserted
the old (partial) persisted shape.

Regression tests: DesignMethodPanel render test with a partial vdStates
input, a mergeDescribedAttrs unit test for the exact partial shape from
the issue, and a backend test asserting POST /profiles fills all 6 keys.

Co-authored-by: mergetest <test@local>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Palash Debnath
2026-07-08 04:34:14 +05:30
committed by GitHub
co-authored by mergetest Claude Sonnet 5
parent 549fa4009f
commit 8520b84b68
8 changed files with 157 additions and 7 deletions
+11
View File
@@ -73,6 +73,17 @@ async def create_profile(
raise ValueError("not an object") raise ValueError("not an object")
except ValueError: except ValueError:
raise HTTPException(status_code=422, detail="vd_states must be a JSON object") raise HTTPException(status_code=422, detail="vd_states must be a JSON object")
# Root-cause close for #983: a design profile must never be PERSISTED
# with a partial vd_states shape, regardless of which client (older
# frontend build, hand-edited payload, third-party API caller) created
# it — a missing category key crashes DesignMethodPanel's render on
# every future client that selects this profile. CATEGORY_ORDER is the
# same single source of truth the frontend's CATEGORIES keys mirror
# (core/describe_voice.py), so this can't drift from the picker.
from core.describe_voice import CATEGORY_ORDER
for _cat in CATEGORY_ORDER:
parsed.setdefault(_cat, "Auto")
vd_states = _json.dumps(parsed)
# An all-Auto design (every category left on "Auto") yields an empty # An all-Auto design (every category left on "Auto") yields an empty
# instruct — that's still a valid, saveable voice: synthesis falls back # instruct — that's still a valid, saveable voice: synthesis falls back
# to neutral instruct-only conditioning (see generation.py design path). # to neutral instruct-only conditioning (see generation.py design path).
@@ -147,6 +147,13 @@ export default function DesignMethodPanel({
{Object.entries(CATEGORIES).map(([key, options]) => { {Object.entries(CATEGORIES).map(([key, options]) => {
const many = options.length > 6; const many = options.length > 6;
const optLabel = (val) => { const optLabel = (val) => {
// #983: a profile/localStorage-restored vdStates can carry a
// partial shape (missing category keys) — val is then undefined
// here even though the 'Auto' check above only catches the
// literal sentinel. Guard before .replace() rather than crash;
// 'Auto' matches how the rest of the component (the ternary
// above, the chip/select fallbacks) treats an unset category.
if (typeof val !== 'string' || !val) return 'Auto';
const tKey = `clone.opt_${val.replace(/[ -]/g, '_')}`; const tKey = `clone.opt_${val.replace(/[ -]/g, '_')}`;
const tl = t(tKey); const tl = t(tKey);
return tl !== tKey ? tl : val; return tl !== tKey ? tl : val;
@@ -180,9 +187,13 @@ export default function DesignMethodPanel({
aria-label={t(`clone.cat_${key}`)} aria-label={t(`clone.cat_${key}`)}
> >
{options.map((opt, i) => { {options.map((opt, i) => {
const optTKey = `clone.opt_${opt.replace(/[ -]/g, '_')}`; // `opt` is always a hardcoded CATEGORIES string today,
// never undefined — guarded anyway for consistency with
// the identical pattern above (#983).
const safeOpt = typeof opt === 'string' && opt ? opt : 'Auto';
const optTKey = `clone.opt_${safeOpt.replace(/[ -]/g, '_')}`;
const optTl = t(optTKey); const optTl = t(optTKey);
const optLabel = optTl !== optTKey ? optTl : opt; const optLabel = optTl !== optTKey ? optTl : safeOpt;
const checked = vdStates[key] === opt; const checked = vdStates[key] === opt;
// Roving tabindex: the checked chip is the group's // Roving tabindex: the checked chip is the group's
// single tab stop (first chip if nothing matches). // single tab stop (first chip if nothing matches).
@@ -0,0 +1,68 @@
import React from 'react';
import { describe, it, expect, vi } from 'vitest';
import { render, screen } from '@testing-library/react';
import DesignMethodPanel from './DesignMethodPanel';
// #983: "Cannot read properties of undefined (reading 'replace')" — the
// identity panel crashed whenever vdStates was missing one of the 6
// CATEGORIES keys (a design profile saved by an older/foreign client, or a
// stale localStorage shape). The label helper called `val.replace(...)` on
// an undefined category value. This regression-tests the render guard added
// to DesignMethodPanel.jsx directly, independent of the upstream data-shape
// fixes in useProfiles.js / useAppData.js / profiles.py.
// A minimal i18next-compatible mock: returns the defaultValue if given, else
// echoes the key back (mirrors i18next's behavior for a missing translation,
// which is what `optLabel`'s `tl !== tKey` check relies on).
const t = (key, opts) => opts?.defaultValue ?? key;
function setup(vdStates, props = {}) {
return render(
<DesignMethodPanel
t={t}
describeText=""
onDescribeChange={vi.fn()}
describeMatchedAny={false}
describeUnmatched={[]}
chipPersonalities={[]}
activePersonality={null}
applyPersonality={vi.fn()}
applyPreset={vi.fn()}
identityOpen={true}
setIdentityOpen={vi.fn()}
identityRecipe="test recipe"
vdStates={vdStates}
setVdStates={vi.fn()}
onChipKeyDown={vi.fn()}
showSaveProfile={false}
setShowSaveProfile={vi.fn()}
profileName=""
setProfileName={vi.fn()}
handleSaveDesignProfile={vi.fn()}
instruct=""
language="Auto"
{...props}
/>,
);
}
describe('DesignMethodPanel — #983 partial vdStates crash', () => {
it('does not throw when vdStates is missing 5 of the 6 CATEGORIES keys', () => {
// Only Gender is set — Age, Pitch, Style, EnglishAccent, ChineseDialect
// are all undefined, exercising both the chip-based and <select>-based
// ("many" options) render paths.
expect(() => setup({ Gender: 'male' })).not.toThrow();
});
it('does not throw when vdStates is a fully empty object', () => {
expect(() => setup({})).not.toThrow();
});
it('still renders category labels and the identity recipe with a partial shape', () => {
const { container } = setup({ Gender: 'male' });
expect(screen.getByText('test recipe')).toBeInTheDocument();
// The label text sits alongside a sibling <span> kicker, so assert via
// textContent rather than getByText's exact-node matching.
expect(container.textContent).toContain('clone.cat_Gender');
});
});
+7 -1
View File
@@ -8,6 +8,7 @@ import { listExportHistory } from '../api/exports';
import { modelStatus as apiModelStatus } from '../api/system'; import { modelStatus as apiModelStatus } from '../api/system';
import { useModelStatus } from '../api/hooks'; import { useModelStatus } from '../api/hooks';
import useRealtimeEvents from './useRealtimeEvents'; import useRealtimeEvents from './useRealtimeEvents';
import { mergeDescribedAttrs } from '../utils/voiceInstruct';
/** /**
* Encapsulates all data-loading effects, localStorage persistence, * Encapsulates all data-loading effects, localStorage persistence,
@@ -174,7 +175,12 @@ export default function useAppData() {
setDefineMethod('design'); setDefineMethod('design');
} else if (saved.mode) setMode(saved.mode); } else if (saved.mode) setMode(saved.mode);
if (saved.defineMethod) setDefineMethod(saved.defineMethod); if (saved.defineMethod) setDefineMethod(saved.defineMethod);
if (saved.vdStates) setVdStates(saved.vdStates); // #983: legacy localStorage state had no shape validation at all — a
// partial/corrupt saved.vdStates crashed DesignMethodPanel on restore.
// Mirror useProfiles.js's guard: require a plain object, then complete
// it to the full CATEGORIES shape (missing/unknown keys → 'Auto').
if (saved.vdStates && typeof saved.vdStates === 'object')
setVdStates(mergeDescribedAttrs(saved.vdStates));
if (saved.language) setLanguage(saved.language); if (saved.language) setLanguage(saved.language);
if (saved.isSidebarCollapsed !== undefined) setIsSidebarCollapsed(saved.isSidebarCollapsed); if (saved.isSidebarCollapsed !== undefined) setIsSidebarCollapsed(saved.isSidebarCollapsed);
if (saved.sidebarTab) setSidebarTab(saved.sidebarTab); if (saved.sidebarTab) setSidebarTab(saved.sidebarTab);
+7 -2
View File
@@ -11,7 +11,7 @@ import { generateSpeech, audioUrlWithCacheBust } from '../api/generate';
import { apiFetch } from '../api/client'; import { apiFetch } from '../api/client';
import { playBlobAudio } from '../utils/media'; import { playBlobAudio } from '../utils/media';
import { PRESETS } from '../utils/constants'; import { PRESETS } from '../utils/constants';
import { instructToFormValue } from '../utils/voiceInstruct'; import { instructToFormValue, mergeDescribedAttrs } from '../utils/voiceInstruct';
import { askConfirm } from '../utils/dialog'; import { askConfirm } from '../utils/dialog';
import { toast } from 'react-hot-toast'; import { toast } from 'react-hot-toast';
import { recordValueMoment } from '../utils/donationMoments'; import { recordValueMoment } from '../utils/donationMoments';
@@ -96,7 +96,12 @@ export default function useProfiles({ loadHistory, loadProfiles }) {
if (profile.kind === 'design' && profile.vd_states) { if (profile.kind === 'design' && profile.vd_states) {
try { try {
const parsed = JSON.parse(profile.vd_states); const parsed = JSON.parse(profile.vd_states);
if (parsed && typeof parsed === 'object') setVdStates(parsed); // #983: a profile saved by an older/foreign client (or hand-edited)
// can carry a partial shape — mergeDescribedAttrs (already used for
// the "describe your voice" restore path) guarantees every
// CATEGORIES key is present, defaulting missing/unknown ones to
// 'Auto', so DesignMethodPanel never sees an undefined category.
if (parsed && typeof parsed === 'object') setVdStates(mergeDescribedAttrs(parsed));
} catch { } catch {
/* malformed stored state — sliders keep their current values */ /* malformed stored state — sliders keep their current values */
} }
+16
View File
@@ -61,3 +61,19 @@ test('empty / missing attrs yields all-Auto', () => {
for (const cat of ALL_CATS) assert.equal(out[cat], 'Auto'); for (const cat of ALL_CATS) assert.equal(out[cat], 'Auto');
} }
}); });
test('#983 — a partial vdStates shape (as restored from a saved profile or '
+ 'localStorage) is completed to all 6 CATEGORIES keys', () => {
// Mirrors the exact partial shape from issue #983: only Gender survives
// (e.g. a design profile saved by an older client, or a hand-edited
// payload), the other 5 category keys are simply absent from the object.
// useProfiles.js/useAppData.js now run any restored vd_states through this
// helper before calling setVdStates, so DesignMethodPanel's render never
// sees an undefined category value.
const out = mergeDescribedAttrs({ Gender: 'male' });
assert.deepEqual(Object.keys(out).sort(), [...ALL_CATS].sort());
assert.equal(out.Gender, 'male');
for (const cat of ALL_CATS) {
if (cat !== 'Gender') assert.equal(out[cat], 'Auto');
}
});
+5 -1
View File
@@ -81,7 +81,11 @@ def test_design_save_creates_row_when_model_unavailable(iso, monkeypatch):
assert row["kind"] == "design" assert row["kind"] == "design"
# Sample is pending — no rendered identity wav was forced at save time. # Sample is pending — no rendered identity wav was forced at save time.
assert not row["ref_audio_path"] assert not row["ref_audio_path"]
assert json.loads(row["vd_states"]) == _VD # #983: vd_states is completed to all 6 known categories before persisting
# (missing ones default to 'Auto') — _VD only sets 3, so the stored value
# is a superset of it, not an exact match.
stored = json.loads(row["vd_states"])
assert stored == {**_VD, "Style": "Auto", "EnglishAccent": "Auto", "ChineseDialect": "Auto"}
def test_all_auto_design_is_saveable(iso, monkeypatch): def test_all_auto_design_is_saveable(iso, monkeypatch):
+30 -1
View File
@@ -170,12 +170,41 @@ def test_design_create_renders_sample_and_stores_params(app_client, fake_render)
assert body["kind"] == "design" assert body["kind"] == "design"
profile = client.get(f"/profiles/{body['id']}").json() profile = client.get(f"/profiles/{body['id']}").json()
assert profile["kind"] == "design" assert profile["kind"] == "design"
assert json.loads(profile["vd_states"]) == _VD # #983: the server now completes vd_states to all 6 known categories
# (missing ones default to 'Auto') before persisting — _VD only sets 3,
# so the stored value is a superset of it, not an exact match.
stored = json.loads(profile["vd_states"])
assert stored == {**_VD, "Style": "Auto", "EnglishAccent": "Auto", "ChineseDialect": "Auto"}
assert profile["seed"] == 42 # deterministic identity sample assert profile["seed"] == 42 # deterministic identity sample
wav = os.path.join(cfg.VOICES_DIR, profile["ref_audio_path"]) wav = os.path.join(cfg.VOICES_DIR, profile["ref_audio_path"])
assert os.path.exists(wav) and os.path.getsize(wav) > 0 assert os.path.exists(wav) and os.path.getsize(wav) > 0
def test_design_normalizes_partial_vd_states_to_all_categories(app_client, fake_render):
"""#983: a design profile must never persist with a partial vd_states shape.
A client (older frontend build, hand-edited payload, third-party API
caller) that only sends a subset of the 6 known category keys used to be
saved as-is — selecting that profile later handed the frontend an
incomplete vdStates object, crashing DesignMethodPanel's render
("Cannot read properties of undefined (reading 'replace')"). The server
now fills every missing category with 'Auto' before persisting, so the
stored vd_states is always complete regardless of which client wrote it.
"""
client, _ = app_client
r = client.post(
"/profiles",
data={"name": "Partial", "kind": "design", "vd_states": json.dumps({"Gender": "male"})},
)
assert r.status_code == 200, r.text
profile = client.get(f"/profiles/{r.json()['id']}").json()
stored = json.loads(profile["vd_states"])
assert set(stored) == {"Gender", "Age", "Pitch", "Style", "EnglishAccent", "ChineseDialect"}
assert stored["Gender"] == "male"
for cat in ("Age", "Pitch", "Style", "EnglishAccent", "ChineseDialect"):
assert stored[cat] == "Auto"
# ── Migration 0005 ─────────────────────────────────────────────────────────── # ── Migration 0005 ───────────────────────────────────────────────────────────
def _run_alembic(direction: str, db_path: str, target: str = "head"): def _run_alembic(direction: str, db_path: str, target: str = "head"):