[fuleinist]

Summary

When user attempts to change model storage location with no models downloaded, the migration API returns moved: 0 early. Previously the UI would still call setCustomModelsDir() and restart the server, causing unexpected behavior (hang/connection lost) as described in issue #426.

This fix checks migrationResult.moved === 0 and shows a helpful toast message instructing the user to download at least one model before changing the storage location, instead of proceeding with a no-op storage change.

Test plan

1. Open Voicebox with no models downloaded
2. Attempt to change model storage location via the UI
3. Confirm a toast appears: 'No models to migrate' with description 'Download at least one model before changing the storage location.'
4. Confirm storage location remains unchanged and server is not restarted

Related issue

Fixes #426

Summary by CodeRabbit

• Bug Fixes
  • Enhanced the model migration process to exit early when no models are available for relocation, skipping unnecessary processing steps. Users now receive a clear notification when a migration completes without moving any models, improving the user experience.

[coderabbitai]

📝 Walkthrough

Walkthrough

The migration flow in ModelManagement.tsx now captures the result of apiClient.migrateModels(newDir) and adds an early-exit condition: if no models were moved (migrationResult.moved === 0), the process stops, displays a toast notification, and skips SSE progress handling and server restart.

Changes

Cohort / File(s)	Summary
Model Migration Early-Exit Logic app/src/components/ServerSettings/ModelManagement.tsx	Added conditional check after initiating model migration; when zero models are moved, the flow exits early with a toast notification and skips downstream operations (SSE connection, server restart, cache invalidation).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A rabbit hops through storage paths so bright,
No models lost, the migration's set right—
When zero files move, we bow out with grace,
Toast the attempt, and return to safe place! 🍞✨

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: detecting when no models exist and warning the user rather than proceeding with a no-op storage change.
Linked Issues check	✅ Passed	The PR correctly addresses issue #426 by detecting zero-moved-models early and preventing no-op server restarts with clear user feedback via toast.
Out of Scope Changes check	✅ Passed	All changes in ModelManagement.tsx are directly related to fixing the storage migration flow for the zero-models case, with no unrelated modifications.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 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/ServerSettings/ModelManagement.tsx`:
- Around line 980-983: The code reads migrationResult.moved but
apiClient.migrateModels is typed as returning Promise<{ source: string;
destination: string }>, causing a type mismatch; update the migrateModels return
type in the API client to include the moved property (e.g., add moved: number or
moved?: number to the returned object type or shared response interface used by
migrateModels), adjust any related type alias or interface declarations used by
apiClient.migrateModels to reflect this field, and then rebuild/TypeScript-check
to ensure all call sites (including the check using migrationResult.moved)
compile correctly.

🪄 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: 6f9e0a9a-ad9a-4255-8d6e-5e16de071281

📥 Commits

Reviewing files that changed from the base of the PR and between 625e1ba and b9a31f6.

📒 Files selected for processing (1)

• app/src/components/ServerSettings/ModelManagement.tsx

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "1) Locate migrateModels definitions and callsites"
rg -n --type=ts --type=tsx -C2 '\bmigrateModels\s*\('

echo
echo "2) Check declared response type for apiClient.migrateModels"
rg -n --type=ts -C3 'async\s+migrateModels\s*\(destination:\s*string\)\s*:\s*Promise<\{[^}]*\}>' app/src/lib/api/client.ts

echo
echo "3) Find usage of `moved` to confirm contract expectations"
rg -n --type=ts --type=tsx -C2 '\bmoved\b'

Repository: jamiepine/voicebox

Length of output: 156

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== 1) Search for migrateModels definitions and callsites ==="
rg -n 'migrateModels\s*\(' --type ts -A5 -B2

echo
echo "=== 2) Check app/src/lib/api/client.ts for migrateModels ==="
if [ -f app/src/lib/api/client.ts ]; then
  cat -n app/src/lib/api/client.ts | head -100
fi

echo
echo "=== 3) Check ModelManagement.tsx around lines 980-983 ==="
if [ -f app/src/components/ServerSettings/ModelManagement.tsx ]; then
  sed -n '975,990p' app/src/components/ServerSettings/ModelManagement.tsx | cat -n
fi

echo
echo "=== 4) Find all uses of .moved property ==="
rg -n '\.moved' --type ts -B3 -A2

Repository: jamiepine/voicebox

Length of output: 6464

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== 1) Search for migration-related types ==="
rg -n -i 'migrate|migration' app/src/lib/api/types.ts -B2 -A5

echo
echo "=== 2) Search for MigrateModels or similar types ==="
rg -n 'type.*Migrate|interface.*Migrate' app/src/lib/api/

echo
echo "=== 3) Check if moved field exists anywhere ==="
rg -n 'moved\s*:' app/src/lib/api/

echo
echo "=== 4) Check context around the migration response usage ==="
sed -n '980,1010p' app/src/components/ServerSettings/ModelManagement.tsx | cat -n

Repository: jamiepine/voicebox

Length of output: 107

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== 1) Check if types.ts exists and list files in api ==="
ls -la app/src/lib/api/

echo
echo "=== 2) Broader search for 'moved' anywhere in the codebase ==="
rg -n 'moved' --type ts --max-count 10

echo
echo "=== 3) Full context around the migration code (wider range) ==="
sed -n '970,1010p' app/src/components/ServerSettings/ModelManagement.tsx | cat -n

echo
echo "=== 4) Search in server backend for migration response ==="
find . -name '*.py' -o -name '*.ts' | xargs grep -l 'models/migrate' 2>/dev/null | head -5

Repository: jamiepine/voicebox

Length of output: 4452

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== 1) Search for /models/migrate endpoint in backend ==="
rg -n 'models/migrate|def.*migrate' backend/routes/models.py -B5 -A20

echo
echo "=== 2) Check if there's a migration response type/model ==="
rg -n 'class.*Migrate|def.*migrate' backend/ -B2 -A10

echo
echo "=== 3) Look for the response structure with moved field ==="
rg -n '"moved"|\.moved' backend/ -B3 -A3

Repository: jamiepine/voicebox

Length of output: 11983

---

migrationResult.moved is not in the declared API type (build-break risk).

At Line 983, you access migrationResult.moved, but apiClient.migrateModels() is typed as Promise<{ source: string; destination: string }> in app/src/lib/api/client.ts:387. This is a TypeScript contract mismatch.

The backend does return moved when no models are found (see backend/routes/models.py:140), but the API client type doesn't declare it, causing a type error.

Update the API client response type to include the moved field:

Suggested fix

-async migrateModels(destination: string): Promise<{ source: string; destination: string }> {
+async migrateModels(destination: string): Promise<{ source: string; destination: string; moved: number; errors?: unknown[] }> {
   return this.request('/models/migrate', {
     method: 'POST',
     body: JSON.stringify({ destination }),
   });
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/components/ServerSettings/ModelManagement.tsx` around lines 980 -
983, The code reads migrationResult.moved but apiClient.migrateModels is typed
as returning Promise<{ source: string; destination: string }>, causing a type
mismatch; update the migrateModels return type in the API client to include the
moved property (e.g., add moved: number or moved?: number to the returned object
type or shared response interface used by migrateModels), adjust any related
type alias or interface declarations used by apiClient.migrateModels to reflect
this field, and then rebuild/TypeScript-check to ensure all call sites
(including the check using migrationResult.moved) compile correctly.