Skip to content

feat: Qwen CustomVoice engine + profile compatibility UX#373

Open
jamiepine wants to merge 2 commits intomainfrom
feat/custom-voice
Open

feat: Qwen CustomVoice engine + profile compatibility UX#373
jamiepine wants to merge 2 commits intomainfrom
feat/custom-voice

Conversation

@jamiepine
Copy link
Copy Markdown
Owner

@jamiepine jamiepine commented Mar 31, 2026

Summary

  • Add Qwen CustomVoice as a new preset engine with 9 built-in voices and instruct control
  • Gray out unsupported voice profiles instead of hiding them, with supported profiles sorted first
  • Clicking a grayed-out profile auto-switches the engine to a compatible one and scrolls to it
  • Fix engine/profile desync when navigating between tabs by initializing form state from the store
  • Store media paths relative to data dir for portability

Test plan

  • Select a preset engine (Kokoro/CustomVoice) — cloned profiles should appear grayed out
  • Click a grayed-out profile — engine should switch and profile becomes active
  • Navigate away to another tab and back — engine and profile selection should stay in sync
  • Verify info tip appears at bottom of profile list when unsupported profiles exist
  • Create and generate with Qwen CustomVoice preset voices

🤖 Generated with Claude Code


Note

Medium Risk
Moderate risk due to changes in generation form defaulting and auto-switching engine based on selected profiles, which can affect what model/engine is used for requests. Backend impact appears minimal (route import/update timestamp behavior) but should be verified with preset voice listing and profile update flows.

Overview
Adds support for the new preset engine qwen_custom_voice and updates generation/profile selection so the form engine auto-aligns with the selected profile (including falling back away from preset engines when selecting non-preset voices).

Updates the voice profile list UX to sort supported profiles first, gray out unsupported profiles instead of filtering them out, show an inline info hint when unsupported profiles exist, and auto-scroll the list to the currently selected profile after engine changes.

Initializes generation form engine from the global selectedEngine store to prevent engine/profile desync when navigating between views, and includes a small backend tweak in profiles.py (imports datetime, used for updated timestamps in effects updates).

Written by Cursor Bugbot for commit 7ebf57d. Configure here.

Summary by CodeRabbit

  • New Features

    • Voice profiles now organized by engine compatibility, with supported profiles listed first.
    • Unsupported profiles are visually disabled; an informational row explains selection limits.
    • Selected voice profile auto-scrolls into view; engine selection now defaults to your previously chosen engine.
  • Bug Fixes

    • Improved profile-to-generation synchronization to avoid incompatible engine selections when cloning or switching profiles.
    • Server now updates profile timestamps when effects are changed.

… engine on selection

- Show all voice profiles with unsupported ones grayed out (opacity) instead of hidden
- Clicking a grayed-out profile selects it and auto-switches the engine to a compatible one
- Sort supported profiles first, with info tip about compatibility at the bottom
- Scroll to selected profile after engine/sort changes with safe margin
- Fix engine desync on tab navigation by initializing form engine from store

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

📝 Walkthrough

Walkthrough

Updates engine and profile synchronization across frontend and backend. Changes engine derivation logic to support both default_engine and preset_engine with fallback remapping, adds disabled state for unsupported profiles in the UI, shows all profiles (marking unsupported ones disabled) with scroll-into-view for selection, sources default engine from the UI store, and updates backend profile timestamps when effects change.

Changes

Cohort / File(s) Summary
Engine Selection & Form
app/src/components/Generation/FloatingGenerateBox.tsx, app/src/lib/hooks/useGenerationForm.ts
Engine derivation updated: FloatingGenerateBox now uses selectedProfile.default_engine or selectedProfile.preset_engine, casts within expanded local EngineValue (includes qwen_custom_voice), and remaps incompatible preset engines (kokoro, qwen_custom_voice) to qwen for non-preset profiles. useGenerationForm now prefers selectedEngine from UI store for form default.
Profile Card UI
app/src/components/VoiceProfiles/ProfileCard.tsx
Added optional disabled?: boolean prop. When disabled, hover/shadow and selected ring styling are adjusted; selection toggle logic includes a timed re-set when disabled-selected to retrigger engine auto-switch.
Profile List Management
app/src/components/VoiceProfiles/ProfileList.tsx
Switched from filtering to marking unsupported profiles: renders all profiles, passes disabled={!isSupported(profile)} to cards, sorts supported first, shows info row if any unsupported exist, and adds scroll-into-view behavior for selected cards (with temporary scroll margin).
Backend Profile Updates
backend/routes/profiles.py
When profile effects are modified/cleared, set profile.updated_at = datetime.utcnow() before commit/refresh to persist timestamp changes.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 I hopped through profiles, soft and spry,

Engines chosen, presets waving by,
Disabled cards rest with gentle cheer,
Scrolls aligned so selections appear,
Timestamps tick — a burrowed little sigh.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: Qwen CustomVoice engine + profile compatibility UX' accurately reflects the main changes: addition of a new Qwen CustomVoice engine and UX improvements for profile compatibility (grayed-out unsupported profiles, auto-engine switching, and better synchronization).

✏️ 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 feat/custom-voice

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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/components/Generation/FloatingGenerateBox.tsx`:
- Around line 128-137: The code casts backend engine strings to EngineValue and
calls form.setValue('engine', ...) without validating them, which can produce an
invalid form state; update the logic in the useEffect around selectedProfile to
check that the resolved engine (from selectedProfile.default_engine or
.preset_engine) is one of the allowed EngineValue members before calling
form.setValue('engine', ...), and if it is not valid either skip setting the
form or map/fallback to a safe default and optionally log/debug; additionally
extract the engine enum/allowed values used here into a shared constant (used by
generationSchema and EngineModelSelector) so validation is centralized and you
can perform a simple includes(...) check against that shared list before setting
the form value.

In `@app/src/components/VoiceProfiles/ProfileList.tsx`:
- Around line 21-34: The effect schedules a requestAnimationFrame and a
setTimeout without cancelling them, which can cause stale DOM writes after
unmount or when selections change; modify the useEffect that references
selectedProfileId/selectedEngine and cardRefs to capture the RAF id and timeout
id returned by requestAnimationFrame and setTimeout, and return a cleanup that
calls cancelAnimationFrame(rafId) and clearTimeout(timeoutId); additionally,
before mutating el.style inside the RAF callback, re-check that
cardRefs.current.get(selectedProfileId) still exists and that the component is
still mounted (or that selectedProfileId matches) to avoid applying
scrollMarginTop to stale elements.
- Around line 52-55: The isSupported predicate currently marks every non-preset
profile as supported when a non-preset engine is selected; update isSupported
(used with allProfiles and selectedEngine) so that for non-preset engines it
returns true only for profiles with voice_type !== 'preset' AND where
profile.default_engine is either unset/empty or equals selectedEngine; preserve
the existing preset branch that checks p.voice_type === 'preset' &&
p.preset_engine === selectedEngine so both preset_engine and default_engine
constraints are handled correctly.

In `@app/src/lib/hooks/useGenerationForm.ts`:
- Line 67: The code currently force-casts selectedEngine into
GenerationFormValues['engine'] when building the form default (engine:
(selectedEngine as GenerationFormValues['engine']) || 'qwen'); replace that
assertion with a runtime guard: validate selectedEngine against the allowed
engine values (e.g., check it exists in the GenerationFormValues engine
union/enum or in an allowedEngines array/Object.values) and only use it if
valid, otherwise fall back to 'qwen'; add or reuse a helper like
isValidEngine(selectedEngine) and reference it where engine is set to avoid
unsafe casts and protect downstream logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 812469e2-6744-4500-ae5e-27341763449f

📥 Commits

Reviewing files that changed from the base of the PR and between 75abbb0 and 7ebf57d.

📒 Files selected for processing (5)
  • app/src/components/Generation/FloatingGenerateBox.tsx
  • app/src/components/VoiceProfiles/ProfileCard.tsx
  • app/src/components/VoiceProfiles/ProfileList.tsx
  • app/src/lib/hooks/useGenerationForm.ts
  • backend/routes/profiles.py

Comment on lines +128 to +137
type EngineValue = 'qwen' | 'luxtts' | 'chatterbox' | 'chatterbox_turbo' | 'tada' | 'kokoro' | 'qwen_custom_voice';
useEffect(() => {
if (selectedProfile?.language) {
form.setValue('language', selectedProfile.language as LanguageCode);
}
// Auto-switch engine if profile has a default
if (selectedProfile?.default_engine) {
form.setValue(
'engine',
selectedProfile.default_engine as
| 'qwen'
| 'luxtts'
| 'chatterbox'
| 'chatterbox_turbo'
| 'tada'
| 'kokoro',
);
// Auto-switch engine to match the profile
const engine = selectedProfile?.default_engine ?? selectedProfile?.preset_engine;
if (engine) {
form.setValue('engine', engine as EngineValue);
} else if (selectedProfile && selectedProfile.voice_type !== 'preset') {
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect how engine metadata is typed and consumed
fd -i 'types.ts' --exec rg -n 'default_engine|preset_engine' {}
rg -n --type=ts --type=tsx -C2 '\b(default_engine|preset_engine)\b'

Repository: jamiepine/voicebox

Length of output: 224


🏁 Script executed:

# Get the actual FloatingGenerateBox.tsx file content around lines 128-137
fd -i floatinggeneratebox -type f | head -5

Repository: jamiepine/voicebox

Length of output: 232


🏁 Script executed:

# Find the types files that matched in the initial search
fd -i 'types.ts' -type f

Repository: jamiepine/voicebox

Length of output: 232


🏁 Script executed:

# Search for EngineValue definitions
rg -n 'type EngineValue|EngineValue\s*='

Repository: jamiepine/voicebox

Length of output: 237


🏁 Script executed:

# Understand form setup and validation
rg -n 'form\.setValue.*engine' -C 3 -i

Repository: jamiepine/voicebox

Length of output: 4299


🏁 Script executed:

# Find GenerationFormValues definition
rg -n 'type GenerationFormValues|interface GenerationFormValues' -A 15

Repository: jamiepine/voicebox

Length of output: 1512


🏁 Script executed:

# Look for form initialization and schema validation
rg -n 'const form = useForm|useForm.*GenerationFormValues' -B 2 -A 10

Repository: jamiepine/voicebox

Length of output: 5975


🏁 Script executed:

# Check for zod/validation schema
rg -n 'generationSchema|engine.*z\.' -A 3

Repository: jamiepine/voicebox

Length of output: 1016


🏁 Script executed:

# Get full FloatingGenerateBox.tsx context around engine assignment
sed -n '120,150p' app/src/components/Generation/FloatingGenerateBox.tsx

Repository: jamiepine/voicebox

Length of output: 1346


🏁 Script executed:

# Get the full generationSchema definition
sed -n '15,33p' app/src/lib/hooks/useGenerationForm.ts

Repository: jamiepine/voicebox

Length of output: 541


Add runtime validation for backend engine values before setting form state.

Lines 134–136 cast backend engine strings directly to EngineValue without checking validity. If the backend returns an unexpected value, form.setValue('engine', ...) accepts it and the form silently becomes invalid.

The form schema defines engine as a Zod enum (generationSchema), but form.setValue() doesn't trigger validation—only form submission does. This creates a window where the form state contains an invalid engine without error signaling.

Add a guard to validate against known engines:

Suggested fix
  type EngineValue = 'qwen' | 'luxtts' | 'chatterbox' | 'chatterbox_turbo' | 'tada' | 'kokoro' | 'qwen_custom_voice';
+ const VALID_ENGINES = new Set<EngineValue>([
+   'qwen',
+   'luxtts',
+   'chatterbox',
+   'chatterbox_turbo',
+   'tada',
+   'kokoro',
+   'qwen_custom_voice',
+ ]);
  useEffect(() => {
    if (selectedProfile?.language) {
      form.setValue('language', selectedProfile.language as LanguageCode);
    }
    // Auto-switch engine to match the profile
    const engine = selectedProfile?.default_engine ?? selectedProfile?.preset_engine;
-   if (engine) {
+   if (engine && VALID_ENGINES.has(engine as EngineValue)) {
      form.setValue('engine', engine as EngineValue);

Also consider extracting the engine enum to a shared constant to avoid duplication across FloatingGenerateBox.tsx, generationSchema, and other components like EngineModelSelector.tsx.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/components/Generation/FloatingGenerateBox.tsx` around lines 128 -
137, The code casts backend engine strings to EngineValue and calls
form.setValue('engine', ...) without validating them, which can produce an
invalid form state; update the logic in the useEffect around selectedProfile to
check that the resolved engine (from selectedProfile.default_engine or
.preset_engine) is one of the allowed EngineValue members before calling
form.setValue('engine', ...), and if it is not valid either skip setting the
form or map/fallback to a safe default and optionally log/debug; additionally
extract the engine enum/allowed values used here into a shared constant (used by
generationSchema and EngineModelSelector) so validation is centralized and you
can perform a simple includes(...) check against that shared list before setting
the form value.

Comment on lines +21 to +34
useEffect(() => {
if (!selectedProfileId) return;
// Wait a frame for the DOM to update after re-sort
requestAnimationFrame(() => {
const el = cardRefs.current.get(selectedProfileId);
if (!el) return;

// Temporarily apply scroll-margin so it doesn't land flush at the top
el.style.scrollMarginTop = '180px';
el.scrollIntoView({ behavior: 'smooth', block: 'nearest', inline: 'nearest' });
// Clean up after scroll completes
setTimeout(() => { el.style.scrollMarginTop = ''; }, 500);
});
}, [selectedProfileId, selectedEngine]);
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.

⚠️ Potential issue | 🟡 Minor

Add cleanup for deferred scroll operations.

Lines 24-33 schedule requestAnimationFrame and setTimeout without cancellation. Rapid profile/engine changes can trigger stale scroll writes after unmount or subsequent selections.

Proposed fix
 useEffect(() => {
   if (!selectedProfileId) return;
-  // Wait a frame for the DOM to update after re-sort
-  requestAnimationFrame(() => {
+  let timeoutId: ReturnType<typeof setTimeout> | null = null;
+  const rafId = requestAnimationFrame(() => {
     const el = cardRefs.current.get(selectedProfileId);
     if (!el) return;
@@
-    setTimeout(() => { el.style.scrollMarginTop = ''; }, 500);
+    timeoutId = setTimeout(() => {
+      el.style.scrollMarginTop = '';
+    }, 500);
   });
+  return () => {
+    cancelAnimationFrame(rafId);
+    if (timeoutId) clearTimeout(timeoutId);
+  };
 }, [selectedProfileId, selectedEngine]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
if (!selectedProfileId) return;
// Wait a frame for the DOM to update after re-sort
requestAnimationFrame(() => {
const el = cardRefs.current.get(selectedProfileId);
if (!el) return;
// Temporarily apply scroll-margin so it doesn't land flush at the top
el.style.scrollMarginTop = '180px';
el.scrollIntoView({ behavior: 'smooth', block: 'nearest', inline: 'nearest' });
// Clean up after scroll completes
setTimeout(() => { el.style.scrollMarginTop = ''; }, 500);
});
}, [selectedProfileId, selectedEngine]);
useEffect(() => {
if (!selectedProfileId) return;
let timeoutId: ReturnType<typeof setTimeout> | null = null;
const rafId = requestAnimationFrame(() => {
const el = cardRefs.current.get(selectedProfileId);
if (!el) return;
// Temporarily apply scroll-margin so it doesn't land flush at the top
el.style.scrollMarginTop = '180px';
el.scrollIntoView({ behavior: 'smooth', block: 'nearest', inline: 'nearest' });
// Clean up after scroll completes
timeoutId = setTimeout(() => {
el.style.scrollMarginTop = '';
}, 500);
});
return () => {
cancelAnimationFrame(rafId);
if (timeoutId) clearTimeout(timeoutId);
};
}, [selectedProfileId, selectedEngine]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/components/VoiceProfiles/ProfileList.tsx` around lines 21 - 34, The
effect schedules a requestAnimationFrame and a setTimeout without cancelling
them, which can cause stale DOM writes after unmount or when selections change;
modify the useEffect that references selectedProfileId/selectedEngine and
cardRefs to capture the RAF id and timeout id returned by requestAnimationFrame
and setTimeout, and return a cleanup that calls cancelAnimationFrame(rafId) and
clearTimeout(timeoutId); additionally, before mutating el.style inside the RAF
callback, re-check that cardRefs.current.get(selectedProfileId) still exists and
that the component is still mounted (or that selectedProfileId matches) to avoid
applying scrollMarginTop to stale elements.

Comment on lines +52 to +55
const isSupported = (p: (typeof allProfiles)[number]) =>
isPresetEngine
? p.voice_type === 'preset' && p.preset_engine === selectedEngine
: p.voice_type !== 'preset';
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.

⚠️ Potential issue | 🟠 Major

isSupported should account for default_engine on non-preset profiles.

Lines 52-55 currently treat every non-preset profile as supported whenever a non-preset engine is selected. Profiles with a specific default_engine can be incorrectly marked as supported.

Proposed fix
 const isSupported = (p: (typeof allProfiles)[number]) =>
-  isPresetEngine
-    ? p.voice_type === 'preset' && p.preset_engine === selectedEngine
-    : p.voice_type !== 'preset';
+  isPresetEngine
+    ? p.voice_type === 'preset' && p.preset_engine === selectedEngine
+    : p.voice_type !== 'preset' &&
+      (!p.default_engine || p.default_engine === selectedEngine);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const isSupported = (p: (typeof allProfiles)[number]) =>
isPresetEngine
? p.voice_type === 'preset' && p.preset_engine === selectedEngine
: p.voice_type !== 'preset';
const isSupported = (p: (typeof allProfiles)[number]) =>
isPresetEngine
? p.voice_type === 'preset' && p.preset_engine === selectedEngine
: p.voice_type !== 'preset' &&
(!p.default_engine || p.default_engine === selectedEngine);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/components/VoiceProfiles/ProfileList.tsx` around lines 52 - 55, The
isSupported predicate currently marks every non-preset profile as supported when
a non-preset engine is selected; update isSupported (used with allProfiles and
selectedEngine) so that for non-preset engines it returns true only for profiles
with voice_type !== 'preset' AND where profile.default_engine is either
unset/empty or equals selectedEngine; preserve the existing preset branch that
checks p.voice_type === 'preset' && p.preset_engine === selectedEngine so both
preset_engine and default_engine constraints are handled correctly.

modelSize: '1.7B',
instruct: '',
engine: 'qwen',
engine: (selectedEngine as GenerationFormValues['engine']) || 'qwen',
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify selectedEngine is not strongly constrained at store boundary
fd -i 'uiStore.ts' --exec sed -n '1,220p' {}
rg -n --type=ts --type=tsx -C2 '\bsetSelectedEngine\s*\('

Repository: jamiepine/voicebox

Length of output: 2481


🏁 Script executed:

# Check useGenerationForm.ts around line 67 to confirm the code
fd -type f -name 'useGenerationForm.ts' -exec sed -n '40,75p' {} +

Repository: jamiepine/voicebox

Length of output: 232


🏁 Script executed:

# Find GenerationFormValues type definition
rg -n 'type GenerationFormValues|interface GenerationFormValues' -A 15 --type ts

Repository: jamiepine/voicebox

Length of output: 1512


🏁 Script executed:

# Find all usages of setSelectedEngine to see what values are being set
rg -n 'setSelectedEngine' --type ts -B 2 -A 2

Repository: jamiepine/voicebox

Length of output: 1912


🏁 Script executed:

# Find generationSchema definition
rg -n 'generationSchema\s*=' -A 20 --type ts

Repository: jamiepine/voicebox

Length of output: 1493


🏁 Script executed:

# Read FloatingGenerateBox around setSelectedEngine usage to see watchedEngine source
sed -n '110,135p' app/src/components/Generation/FloatingGenerateBox.tsx

Repository: jamiepine/voicebox

Length of output: 1060


🏁 Script executed:

# Read useGenerationForm.ts around line 67
sed -n '60,75p' app/src/lib/hooks/useGenerationForm.ts

Repository: jamiepine/voicebox

Length of output: 477


Guard selectedEngine at runtime instead of asserting it.

Line 67 force-casts a plain string from store into GenerationFormValues['engine']. The store accepts any string without validation, and while the current code path (form.watch → setSelectedEngine) maintains valid enum values, the store boundary is unguarded. If the store gets persisted, synced from external sources, or called from new code paths, invalid values can bypass form validation and break downstream logic.

Proposed fix
+const ALLOWED_ENGINES = new Set<NonNullable<GenerationFormValues['engine']>>([
+  'qwen',
+  'qwen_custom_voice',
+  'luxtts',
+  'chatterbox',
+  'chatterbox_turbo',
+  'tada',
+  'kokoro',
+]);
+
+const initialEngine: NonNullable<GenerationFormValues['engine']> = ALLOWED_ENGINES.has(
+  selectedEngine as NonNullable<GenerationFormValues['engine']>,
+)
+  ? (selectedEngine as NonNullable<GenerationFormValues['engine']>)
+  : 'qwen';
+
 const form = useForm<GenerationFormValues>({
   resolver: zodResolver(generationSchema),
   defaultValues: {
@@
-    engine: (selectedEngine as GenerationFormValues['engine']) || 'qwen',
+    engine: initialEngine,
     ...options.defaultValues,
   },
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/lib/hooks/useGenerationForm.ts` at line 67, The code currently
force-casts selectedEngine into GenerationFormValues['engine'] when building the
form default (engine: (selectedEngine as GenerationFormValues['engine']) ||
'qwen'); replace that assertion with a runtime guard: validate selectedEngine
against the allowed engine values (e.g., check it exists in the
GenerationFormValues engine union/enum or in an allowedEngines
array/Object.values) and only use it if valid, otherwise fall back to 'qwen';
add or reuse a helper like isValidEngine(selectedEngine) and reference it where
engine is set to avoid unsafe casts and protect downstream logic.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

isSelected && 'ring-2 ring-accent shadow-md',
'cursor-pointer transition-all flex flex-col h-[162px]',
disabled ? 'opacity-40 hover:opacity-60' : 'hover:shadow-md',
isSelected && !disabled && 'ring-2 ring-accent shadow-md',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disabled+selected card deselects instead of re-engaging

Medium Severity

When a profile is both disabled and isSelected (e.g., user selects a cloned profile then manually switches the engine to Kokoro), the selection ring is hidden via isSelected && !disabled, making the card look unselected. However, handleSelect still uses the toggle isSelected ? null : profile.id, so clicking it deselects the profile instead of re-selecting it and triggering the engine auto-switch. The FloatingGenerateBox's EngineModelSelector doesn't pass selectedProfile, so users can freely switch to an incompatible engine, hitting this state.

Additional Locations (1)
Fix in Cursor Fix in Web

} else if (selectedProfile && selectedProfile.voice_type !== 'preset') {
// Cloned/designed profile with no default — ensure a compatible (non-preset) engine
const currentEngine = form.getValues('engine');
const presetEngines = new Set(['kokoro', 'qwen_custom_voice']);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated preset engine set across components

Low Severity

The set of preset engines (['kokoro', 'qwen_custom_voice']) is independently defined in both ProfileList.tsx (as PRESET_ENGINES) and inline inside a useEffect in FloatingGenerateBox.tsx (as presetEngines). A related complement set CLONING_ENGINES also exists in EngineModelSelector.tsx. Adding a new preset engine requires updating all three locations, creating a maintenance risk of inconsistency.

Additional Locations (1)
Fix in Cursor Fix in Web

- Add cleanup for requestAnimationFrame and setTimeout in scroll effect
  to prevent stale DOM writes on unmount or rapid selection changes
- Fix disabled+selected card click: bounce the selection to re-trigger
  the engine auto-switch instead of deselecting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
app/src/components/VoiceProfiles/ProfileCard.tsx (1)

44-49: Selection bounce pattern intentionally triggers engine auto-switch.

The pattern of clearing selectedProfileId then re-setting it via setTimeout(..., 0) forces dependent effects (like engine auto-switch in FloatingGenerateBox.tsx) to re-run. This is a deliberate workaround for the "disabled but already selected" edge case.

Note: This creates a brief intermediate state where selectedProfileId is null, which will be observed by any component subscribed to the store. Ensure downstream components handle this gracefully.

🔧 Alternative: Consider a dedicated "re-trigger" mechanism

Instead of bouncing through null, you could add a counter or timestamp to the store that increments when a re-trigger is needed:

+// In uiStore
+profileSelectionVersion: 0,
+bumpProfileSelectionVersion: () => set((s) => ({ profileSelectionVersion: s.profileSelectionVersion + 1 })),

// In ProfileCard
 if (disabled && isSelected) {
-  setSelectedProfileId(null);
-  setTimeout(() => setSelectedProfileId(profile.id), 0);
+  bumpProfileSelectionVersion();
   return;
 }

Then effects can depend on both selectedProfileId and profileSelectionVersion to re-run without the intermediate null state.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/components/VoiceProfiles/ProfileCard.tsx` around lines 44 - 49, The
current "selection bounce" in ProfileCard (clearing selectedProfileId via
setSelectedProfileId(null) then setSelectedProfileId(profile.id) in a
setTimeout) causes a transient null state observed by subscribers; replace this
with a dedicated re-trigger mechanism: add a profileSelectionVersion (or
timestamp) in the same store as selectedProfileId, update it from ProfileCard
instead of flipping to null (e.g., increment or set Date.now() when you need to
re-trigger), and update dependent effects in FloatingGenerateBox (and any other
subscribers reading selectedProfileId) to depend on both selectedProfileId and
profileSelectionVersion so the engine auto-switch still re-runs without creating
an intermediate null state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/src/components/VoiceProfiles/ProfileCard.tsx`:
- Around line 44-49: The current "selection bounce" in ProfileCard (clearing
selectedProfileId via setSelectedProfileId(null) then
setSelectedProfileId(profile.id) in a setTimeout) causes a transient null state
observed by subscribers; replace this with a dedicated re-trigger mechanism: add
a profileSelectionVersion (or timestamp) in the same store as selectedProfileId,
update it from ProfileCard instead of flipping to null (e.g., increment or set
Date.now() when you need to re-trigger), and update dependent effects in
FloatingGenerateBox (and any other subscribers reading selectedProfileId) to
depend on both selectedProfileId and profileSelectionVersion so the engine
auto-switch still re-runs without creating an intermediate null state.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fbac16c8-d799-4516-b606-400039fafc7b

📥 Commits

Reviewing files that changed from the base of the PR and between 75abbb0 and a10024f.

📒 Files selected for processing (5)
  • app/src/components/Generation/FloatingGenerateBox.tsx
  • app/src/components/VoiceProfiles/ProfileCard.tsx
  • app/src/components/VoiceProfiles/ProfileList.tsx
  • app/src/lib/hooks/useGenerationForm.ts
  • backend/routes/profiles.py

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