From 3d922ec8465081d58d4707890925622715b2f626 Mon Sep 17 00:00:00 2001
From: Jamie Pine <ijamespine@me.com>
Date: Sat, 14 Mar 2026 07:47:06 -0700
Subject: [PATCH] Fix review findings: toggle logic, preset saving, version
 lookup, async audio ops

- Fix inverted effects toggle in FloatingGenerateBox
- Add Save button + API method for editing custom effect presets
- Return early on effects save failure in ProfileForm
- Fix no-op ternary in effectsStore
- Handle duplicate preset names with proper 400 response
- Use effects_chain is None instead of label for clean version lookup
- Move blocking audio ops to asyncio.to_thread in async endpoints
- Log warnings instead of silently swallowing parse errors
---
 .../components/EffectsTab/EffectsDetail.tsx   | 54 +++++++++++++++----
 .../Generation/FloatingGenerateBox.tsx        |  5 +-
 .../components/VoiceProfiles/ProfileForm.tsx  |  1 +
 app/src/lib/api/client.ts                     | 10 ++++
 app/src/lib/api/types.ts                      |  6 +++
 app/src/stores/effectsStore.ts                |  2 +-
 backend/effects.py                            | 12 ++++-
 backend/main.py                               | 31 +++++------
 backend/profiles.py                           |  5 +-
 9 files changed, 95 insertions(+), 31 deletions(-)

diff --git a/app/src/components/EffectsTab/EffectsDetail.tsx b/app/src/components/EffectsTab/EffectsDetail.tsx
index 73be3113..f877c914 100644
--- a/app/src/components/EffectsTab/EffectsDetail.tsx
+++ b/app/src/components/EffectsTab/EffectsDetail.tsx
@@ -142,6 +142,29 @@ export function EffectsDetail() {
     }
   }
 
+  async function handleSaveExisting() {
+    if (!selectedPresetId || !name.trim()) return;
+    setSaving(true);
+    try {
+      await apiClient.updateEffectPreset(selectedPresetId, {
+        name: name.trim(),
+        description: description.trim() || undefined,
+        effects_chain: workingChain,
+      });
+      queryClient.invalidateQueries({ queryKey: ['effect-presets'] });
+      queryClient.invalidateQueries({ queryKey: ['effect-preset', selectedPresetId] });
+      toast({ title: 'Preset updated' });
+    } catch (error) {
+      toast({
+        title: 'Failed to save',
+        description: error instanceof Error ? error.message : 'Unknown error',
+        variant: 'destructive',
+      });
+    } finally {
+      setSaving(false);
+    }
+  }
+
   async function handleSaveAsNew() {
     await handleSaveNew();
   }
@@ -186,16 +209,27 @@ export function EffectsDetail() {
         </h2>
         <div className="flex items-center gap-2">
           {!isBuiltIn && !isCreatingNew && (
-            <Button
-              variant="ghost"
-              size="sm"
-              className="h-8 text-destructive hover:text-destructive gap-1.5"
-              onClick={handleDelete}
-              disabled={deleting}
-            >
-              <Trash2 className="h-3.5 w-3.5" />
-              {deleting ? 'Deleting...' : 'Delete'}
-            </Button>
+            <>
+              <Button
+                variant="ghost"
+                size="sm"
+                className="h-8 text-destructive hover:text-destructive gap-1.5"
+                onClick={handleDelete}
+                disabled={deleting}
+              >
+                <Trash2 className="h-3.5 w-3.5" />
+                {deleting ? 'Deleting...' : 'Delete'}
+              </Button>
+              <Button
+                size="sm"
+                className="h-8 gap-1.5"
+                onClick={handleSaveExisting}
+                disabled={saving || workingChain.length === 0}
+              >
+                <Save className="h-3.5 w-3.5" />
+                {saving ? 'Saving...' : 'Save'}
+              </Button>
+            </>
           )}
           {isCreatingNew && (
             <Button
diff --git a/app/src/components/Generation/FloatingGenerateBox.tsx b/app/src/components/Generation/FloatingGenerateBox.tsx
index 2a5560c3..e2fe6b9b 100644
--- a/app/src/components/Generation/FloatingGenerateBox.tsx
+++ b/app/src/components/Generation/FloatingGenerateBox.tsx
@@ -393,8 +393,9 @@ export function FloatingGenerateBox({
                           variant="ghost"
                           size="icon"
                           onClick={() => {
-                            setIsEffectsMode(!isEffectsMode);
-                            if (isEffectsMode) setIsInstructMode(false);
+                            const next = !isEffectsMode;
+                            setIsEffectsMode(next);
+                            if (next) setIsInstructMode(false);
                           }}
                           className={cn(
                             'h-10 w-10 rounded-full transition-all duration-200',
diff --git a/app/src/components/VoiceProfiles/ProfileForm.tsx b/app/src/components/VoiceProfiles/ProfileForm.tsx
index 3bb1903d..13edf6f4 100644
--- a/app/src/components/VoiceProfiles/ProfileForm.tsx
+++ b/app/src/components/VoiceProfiles/ProfileForm.tsx
@@ -456,6 +456,7 @@ export function ProfileForm() {
                 fxError instanceof Error ? fxError.message : 'Failed to save effects chain',
               variant: 'destructive',
             });
+            return;
           }
         }
 
diff --git a/app/src/lib/api/client.ts b/app/src/lib/api/client.ts
index 55f88410..fd3e1b50 100644
--- a/app/src/lib/api/client.ts
+++ b/app/src/lib/api/client.ts
@@ -606,6 +606,16 @@ class ApiClient {
     });
   }
 
+  async updateEffectPreset(
+    presetId: string,
+    data: { name?: string; description?: string; effects_chain?: EffectConfig[] },
+  ): Promise<EffectPresetResponse> {
+    return this.request<EffectPresetResponse>(`/effects/presets/${presetId}`, {
+      method: 'PUT',
+      body: JSON.stringify(data),
+    });
+  }
+
   async deleteEffectPreset(presetId: string): Promise<void> {
     await this.request<void>(`/effects/presets/${presetId}`, {
       method: 'DELETE',
diff --git a/app/src/lib/api/types.ts b/app/src/lib/api/types.ts
index fea80478..95dc0f68 100644
--- a/app/src/lib/api/types.ts
+++ b/app/src/lib/api/types.ts
@@ -296,6 +296,12 @@ export interface EffectPresetCreate {
   effects_chain: EffectConfig[];
 }
 
+export interface EffectPresetUpdate {
+  name?: string;
+  description?: string;
+  effects_chain?: EffectConfig[];
+}
+
 export interface AvailableEffectParam {
   default: number;
   min: number;
diff --git a/app/src/stores/effectsStore.ts b/app/src/stores/effectsStore.ts
index f036c0a7..086dd312 100644
--- a/app/src/stores/effectsStore.ts
+++ b/app/src/stores/effectsStore.ts
@@ -22,5 +22,5 @@ export const useEffectsStore = create<EffectsStore>((set) => ({
   setWorkingChain: (chain) => set({ workingChain: chain }),
 
   isCreatingNew: false,
-  setIsCreatingNew: (v) => set({ isCreatingNew: v, selectedPresetId: v ? null : null }),
+  setIsCreatingNew: (v) => set({ isCreatingNew: v, ...(v && { selectedPresetId: null }) }),
 }));
diff --git a/backend/effects.py b/backend/effects.py
index 868797e8..79130785 100644
--- a/backend/effects.py
+++ b/backend/effects.py
@@ -9,6 +9,7 @@ import uuid
 from typing import List, Optional
 
 from sqlalchemy.orm import Session
+from sqlalchemy.exc import IntegrityError
 
 from .database import EffectPreset as DBEffectPreset
 from .models import EffectPresetResponse, EffectPresetCreate, EffectPresetUpdate, EffectConfig
@@ -58,6 +59,11 @@ def create_preset(data: EffectPresetCreate, db: Session) -> EffectPresetResponse
     if error:
         raise ValueError(error)
 
+    # Check for duplicate name before insert
+    existing = db.query(DBEffectPreset).filter_by(name=data.name).first()
+    if existing:
+        raise ValueError(f"A preset named '{data.name}' already exists")
+
     preset = DBEffectPreset(
         id=str(uuid.uuid4()),
         name=data.name,
@@ -66,7 +72,11 @@ def create_preset(data: EffectPresetCreate, db: Session) -> EffectPresetResponse
         is_builtin=False,
     )
     db.add(preset)
-    db.commit()
+    try:
+        db.commit()
+    except IntegrityError:
+        db.rollback()
+        raise ValueError(f"A preset named '{data.name}' already exists")
     db.refresh(preset)
     return _preset_response(preset)
 
diff --git a/backend/main.py b/backend/main.py
index 139fda10..e269b20d 100644
--- a/backend/main.py
+++ b/backend/main.py
@@ -1297,7 +1297,7 @@ async def transcribe_audio(
     try:
         # Get audio duration
         from .utils.audio import load_audio
-        audio, sr = load_audio(tmp_path)
+        audio, sr = await asyncio.to_thread(load_audio, tmp_path)
         duration = len(audio) / sr
         
         # Transcribe
@@ -1585,21 +1585,21 @@ async def preview_effects(
     if error:
         raise HTTPException(status_code=400, detail=error)
 
-    # Find clean version
+    # Find the original unprocessed version (no effects applied)
     all_versions = versions_mod.list_versions(generation_id, db)
-    clean_version = next((v for v in all_versions if v.label == "clean"), None)
+    clean_version = next((v for v in all_versions if v.effects_chain is None), None)
     source_path = clean_version.audio_path if clean_version else gen.audio_path
     if not source_path or not Path(source_path).exists():
         raise HTTPException(status_code=404, detail="Source audio file not found")
 
-    # Process in memory
-    audio, sample_rate = load_audio(source_path)
-    processed = apply_effects(audio, sample_rate, chain_dicts)
+    # Process in memory (off the event loop)
+    audio, sample_rate = await asyncio.to_thread(load_audio, source_path)
+    processed = await asyncio.to_thread(apply_effects, audio, sample_rate, chain_dicts)
 
     # Write to in-memory buffer
     import soundfile as sf
     buf = io.BytesIO()
-    sf.write(buf, processed, sample_rate, format="WAV")
+    await asyncio.to_thread(sf.write, buf, processed, sample_rate, "WAV")
     buf.seek(0)
 
     return StreamingResponse(
@@ -1723,10 +1723,10 @@ async def apply_effects_to_generation(
     if error:
         raise HTTPException(status_code=400, detail=error)
 
-    # Find the clean version to apply effects to
+    # Find the original unprocessed version (no effects applied)
     all_versions = versions_mod.list_versions(generation_id, db)
     clean_version = next(
-        (v for v in all_versions if v.label == "clean"), None
+        (v for v in all_versions if v.effects_chain is None), None
     )
     if not clean_version:
         # Fallback: use the generation's audio_path directly
@@ -1737,14 +1737,14 @@ async def apply_effects_to_generation(
     if not source_path or not Path(source_path).exists():
         raise HTTPException(status_code=404, detail="Source audio file not found")
 
-    # Load, process, save
-    audio, sample_rate = load_audio(source_path)
-    processed_audio = apply_effects(audio, sample_rate, chain_dicts)
+    # Load, process, save (off the event loop)
+    audio, sample_rate = await asyncio.to_thread(load_audio, source_path)
+    processed_audio = await asyncio.to_thread(apply_effects, audio, sample_rate, chain_dicts)
 
     # Generate a unique filename
     version_id = str(uuid.uuid4())
     processed_path = config.get_generations_dir() / f"{generation_id}_{version_id[:8]}.wav"
-    save_audio(processed_audio, str(processed_path), sample_rate)
+    await asyncio.to_thread(save_audio, processed_audio, str(processed_path), sample_rate)
 
     # Auto-label
     label = data.label or f"version-{len(all_versions) + 1}"
@@ -1857,14 +1857,15 @@ async def update_profile_effects(
 def _profile_to_response(profile) -> models.VoiceProfileResponse:
     """Convert a DB profile to a VoiceProfileResponse with parsed effects_chain."""
     import json as _json
+    import logging
 
     effects_chain = None
     if profile.effects_chain:
         try:
             raw = _json.loads(profile.effects_chain)
             effects_chain = [models.EffectConfig(**e) for e in raw]
-        except Exception:
-            pass
+        except Exception as e:
+            logging.warning(f"Failed to parse effects_chain for profile {profile.id}: {e}")
 
     return models.VoiceProfileResponse(
         id=profile.id,
diff --git a/backend/profiles.py b/backend/profiles.py
index ddec0e84..69ebd086 100644
--- a/backend/profiles.py
+++ b/backend/profiles.py
@@ -36,8 +36,9 @@ def _profile_to_response(profile: DBVoiceProfile) -> VoiceProfileResponse:
         try:
             raw = _json.loads(profile.effects_chain)
             effects_chain = [EffectConfig(**e) for e in raw]
-        except Exception:
-            pass
+        except Exception as e:
+            import logging
+            logging.warning(f"Failed to parse effects_chain for profile {profile.id}: {e}")
     return VoiceProfileResponse(
         id=profile.id,
         name=profile.name,