mirror of
https://github.com/jamiepine/voicebox.git
synced 2026-09-20 15:20:39 -07:00
fix: address PR review feedback for download cancel/error UI
- Fix transcribe_audio to use whisper-large-v3 mapping (not openai/whisper-large) - Propagate error field in progress-only fallback path for get_active_tasks - Use removed return value in cancel endpoint to vary response message - Add error rollback to handleCancel with toast on failure - Make isCancelling per-model instead of global - Fix inverted chevron icons in Problems panel - Move all clears under lock in clear_all_tasks - Simplify cancel_download to use dict.pop()
This commit is contained in:
@@ -157,16 +157,30 @@ export function ModelManagement() {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const handleCancel = (modelName: string) => {
|
const handleCancel = (modelName: string) => {
|
||||||
// Immediately hide the error and suppress downloading state in UI
|
// Snapshot previous state for rollback
|
||||||
|
const prevDismissed = dismissedErrors;
|
||||||
|
const prevLocalErrors = localErrors;
|
||||||
|
const prevDownloadingModel = downloadingModel;
|
||||||
|
const prevDownloadingDisplayName = downloadingDisplayName;
|
||||||
|
|
||||||
|
// Optimistically hide the error and suppress downloading state in UI
|
||||||
setDismissedErrors((prev) => new Set(prev).add(modelName));
|
setDismissedErrors((prev) => new Set(prev).add(modelName));
|
||||||
setLocalErrors((prev) => { const next = new Map(prev); next.delete(modelName); return next; });
|
setLocalErrors((prev) => { const next = new Map(prev); next.delete(modelName); return next; });
|
||||||
// Also clear local downloading state if this was our current download
|
|
||||||
if (downloadingModel === modelName) {
|
if (downloadingModel === modelName) {
|
||||||
setDownloadingModel(null);
|
setDownloadingModel(null);
|
||||||
setDownloadingDisplayName(null);
|
setDownloadingDisplayName(null);
|
||||||
}
|
}
|
||||||
// Fire-and-forget the backend cancel, then refetch to sync
|
|
||||||
cancelMutation.mutate(modelName);
|
cancelMutation.mutate(modelName, {
|
||||||
|
onError: () => {
|
||||||
|
// Rollback optimistic updates on failure
|
||||||
|
setDismissedErrors(prevDismissed);
|
||||||
|
setLocalErrors(prevLocalErrors);
|
||||||
|
setDownloadingModel(prevDownloadingModel);
|
||||||
|
setDownloadingDisplayName(prevDownloadingDisplayName);
|
||||||
|
toast({ title: 'Cancel failed', description: 'Could not cancel the download task.', variant: 'destructive' });
|
||||||
|
},
|
||||||
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
const clearAllMutation = useMutation({
|
const clearAllMutation = useMutation({
|
||||||
@@ -259,7 +273,7 @@ export function ModelManagement() {
|
|||||||
}}
|
}}
|
||||||
onCancel={() => handleCancel(model.model_name)}
|
onCancel={() => handleCancel(model.model_name)}
|
||||||
isDownloading={downloadingModel === model.model_name}
|
isDownloading={downloadingModel === model.model_name}
|
||||||
isCancelling={cancelMutation.isPending}
|
isCancelling={cancelMutation.isPending && cancelMutation.variables === model.model_name}
|
||||||
isDismissed={dismissedErrors.has(model.model_name)}
|
isDismissed={dismissedErrors.has(model.model_name)}
|
||||||
erroredDownload={erroredDownloads.get(model.model_name)}
|
erroredDownload={erroredDownloads.get(model.model_name)}
|
||||||
formatSize={formatSize}
|
formatSize={formatSize}
|
||||||
@@ -291,7 +305,7 @@ export function ModelManagement() {
|
|||||||
}}
|
}}
|
||||||
onCancel={() => handleCancel(model.model_name)}
|
onCancel={() => handleCancel(model.model_name)}
|
||||||
isDownloading={downloadingModel === model.model_name}
|
isDownloading={downloadingModel === model.model_name}
|
||||||
isCancelling={cancelMutation.isPending}
|
isCancelling={cancelMutation.isPending && cancelMutation.variables === model.model_name}
|
||||||
isDismissed={dismissedErrors.has(model.model_name)}
|
isDismissed={dismissedErrors.has(model.model_name)}
|
||||||
erroredDownload={erroredDownloads.get(model.model_name)}
|
erroredDownload={erroredDownloads.get(model.model_name)}
|
||||||
formatSize={formatSize}
|
formatSize={formatSize}
|
||||||
@@ -310,9 +324,9 @@ export function ModelManagement() {
|
|||||||
className="flex items-center gap-2 hover:text-foreground transition-colors"
|
className="flex items-center gap-2 hover:text-foreground transition-colors"
|
||||||
>
|
>
|
||||||
{consoleOpen ? (
|
{consoleOpen ? (
|
||||||
<ChevronDown className="h-3.5 w-3.5" />
|
|
||||||
) : (
|
|
||||||
<ChevronUp className="h-3.5 w-3.5" />
|
<ChevronUp className="h-3.5 w-3.5" />
|
||||||
|
) : (
|
||||||
|
<ChevronDown className="h-3.5 w-3.5" />
|
||||||
)}
|
)}
|
||||||
<span>Problems</span>
|
<span>Problems</span>
|
||||||
<Badge variant="destructive" className="text-[10px] h-4 px-1.5 rounded-full">
|
<Badge variant="destructive" className="text-[10px] h-4 px-1.5 rounded-full">
|
||||||
|
|||||||
+13
-4
@@ -932,7 +932,11 @@ async def transcribe_audio(
|
|||||||
|
|
||||||
# Check if Whisper model is downloaded (uses default size "base")
|
# Check if Whisper model is downloaded (uses default size "base")
|
||||||
model_size = whisper_model.model_size
|
model_size = whisper_model.model_size
|
||||||
model_name = f"openai/whisper-{model_size}"
|
# Map model sizes to HF repo IDs (whisper-large needs -v3 suffix)
|
||||||
|
whisper_hf_repos = {
|
||||||
|
"large": "openai/whisper-large-v3",
|
||||||
|
}
|
||||||
|
model_name = whisper_hf_repos.get(model_size, f"openai/whisper-{model_size}")
|
||||||
|
|
||||||
# Check if model is cached
|
# Check if model is cached
|
||||||
from huggingface_hub import constants as hf_constants
|
from huggingface_hub import constants as hf_constants
|
||||||
@@ -1595,11 +1599,15 @@ async def cancel_model_download(request: models.ModelDownloadRequest):
|
|||||||
removed = task_manager.cancel_download(request.model_name)
|
removed = task_manager.cancel_download(request.model_name)
|
||||||
|
|
||||||
# Also clear progress state so the model doesn't show as downloading
|
# Also clear progress state so the model doesn't show as downloading
|
||||||
|
progress_removed = False
|
||||||
with progress_manager._lock:
|
with progress_manager._lock:
|
||||||
if request.model_name in progress_manager._progress:
|
if request.model_name in progress_manager._progress:
|
||||||
del progress_manager._progress[request.model_name]
|
del progress_manager._progress[request.model_name]
|
||||||
|
progress_removed = True
|
||||||
|
|
||||||
return {"message": f"Download task for {request.model_name} cancelled"}
|
if removed or progress_removed:
|
||||||
|
return {"message": f"Download task for {request.model_name} cancelled"}
|
||||||
|
return {"message": f"No active task found for {request.model_name}"}
|
||||||
|
|
||||||
|
|
||||||
@app.post("/tasks/clear")
|
@app.post("/tasks/clear")
|
||||||
@@ -1613,8 +1621,8 @@ async def clear_all_tasks():
|
|||||||
|
|
||||||
with progress_manager._lock:
|
with progress_manager._lock:
|
||||||
progress_manager._progress.clear()
|
progress_manager._progress.clear()
|
||||||
progress_manager._last_notify_time.clear()
|
progress_manager._last_notify_time.clear()
|
||||||
progress_manager._last_notify_progress.clear()
|
progress_manager._last_notify_progress.clear()
|
||||||
|
|
||||||
return {"message": "All task state cleared"}
|
return {"message": "All task state cleared"}
|
||||||
|
|
||||||
@@ -1771,6 +1779,7 @@ async def get_active_tasks():
|
|||||||
model_name=model_name,
|
model_name=model_name,
|
||||||
status=progress.get("status", "downloading"),
|
status=progress.get("status", "downloading"),
|
||||||
started_at=started_at,
|
started_at=started_at,
|
||||||
|
error=progress.get("error"),
|
||||||
))
|
))
|
||||||
|
|
||||||
# Get active generations
|
# Get active generations
|
||||||
|
|||||||
@@ -74,10 +74,7 @@ class TaskManager:
|
|||||||
|
|
||||||
def cancel_download(self, model_name: str) -> bool:
|
def cancel_download(self, model_name: str) -> bool:
|
||||||
"""Cancel/dismiss a download task (removes it from active list)."""
|
"""Cancel/dismiss a download task (removes it from active list)."""
|
||||||
if model_name in self._active_downloads:
|
return self._active_downloads.pop(model_name, None) is not None
|
||||||
del self._active_downloads[model_name]
|
|
||||||
return True
|
|
||||||
return False
|
|
||||||
|
|
||||||
def is_download_active(self, model_name: str) -> bool:
|
def is_download_active(self, model_name: str) -> bool:
|
||||||
"""Check if a download is active."""
|
"""Check if a download is active."""
|
||||||
|
|||||||
Reference in New Issue
Block a user