From 2c9d02af62ff978ad33b1e9f303547b26becb079 Mon Sep 17 00:00:00 2001 From: Daniel Knoodle Date: Mon, 20 Jul 2026 14:39:58 -0500 Subject: [PATCH] fix(models): stop reporting errored downloads as still downloading (#926) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TaskManager.error_download() intentionally keeps a failed task in the active list (status="error") so /tasks/active can surface the error and retry UI — but /models/status derived its "downloading" flag from the same unfiltered list. One failed download therefore showed the model as downloading:true / downloaded:false for the life of the process, masking the model's real cache state (even a fully valid on-disk cache) until an app restart. Likely behind endless-spinner reports like #181 and the restart-fixes-it pattern in #883. Add TaskManager.get_pending_downloads() (downloading/extracting only) and use it in /models/status; /tasks/active behavior is unchanged. Fixes #925 Claude-Session: https://claude.ai/code/session_011iwL9AyeAWgz2jpgcHxJpC Co-authored-by: Claude Fable 5 --- backend/routes/models.py | 5 +- .../test_model_status_pending_downloads.py | 51 +++++++++++++++++++ backend/utils/tasks.py | 13 +++++ 3 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 backend/tests/test_model_status_pending_downloads.py diff --git a/backend/routes/models.py b/backend/routes/models.py index 7cbb7b04..f6d56566 100644 --- a/backend/routes/models.py +++ b/backend/routes/models.py @@ -231,7 +231,10 @@ async def get_model_status(): backend_type = get_backend_type() task_manager = get_task_manager() - active_download_names = {task.model_name for task in task_manager.get_active_downloads()} + # Pending only — an errored task stays in the active list for the + # error/retry UI, but reporting it as "downloading" here would mask + # the model's real cache state until the app restarts (issue #925). + active_download_names = {task.model_name for task in task_manager.get_pending_downloads()} try: from huggingface_hub import scan_cache_dir diff --git a/backend/tests/test_model_status_pending_downloads.py b/backend/tests/test_model_status_pending_downloads.py new file mode 100644 index 00000000..ee69cfb5 --- /dev/null +++ b/backend/tests/test_model_status_pending_downloads.py @@ -0,0 +1,51 @@ +"""Errored downloads must not be reported as still downloading. + +A failed download intentionally stays in the TaskManager with +``status="error"`` so ``/tasks/active`` can surface the error and retry +UI — but ``/models/status`` derives its ``downloading`` flag from the +same list. Without a status filter, one failed download shows the model +as "downloading" forever and masks its real cache state until the app +restarts (issue #925, symptom reports like #181). +""" + +from backend.utils.tasks import TaskManager + + +def test_errored_download_is_not_pending(): + tm = TaskManager() + tm.start_download("whisper-turbo") + assert [t.model_name for t in tm.get_pending_downloads()] == ["whisper-turbo"] + + tm.error_download("whisper-turbo", "boom") + + assert tm.get_pending_downloads() == [] + # Still visible to /tasks/active for the error/retry UI. + active = tm.get_active_downloads() + assert [t.model_name for t in active] == ["whisper-turbo"] + assert active[0].status == "error" + assert active[0].error == "boom" + + +def test_retry_after_error_is_pending_again(): + tm = TaskManager() + tm.start_download("qwen3-4b") + tm.error_download("qwen3-4b", "boom") + tm.start_download("qwen3-4b") + assert [t.model_name for t in tm.get_pending_downloads()] == ["qwen3-4b"] + + +def test_completed_download_is_removed_everywhere(): + tm = TaskManager() + tm.start_download("whisper-turbo") + tm.complete_download("whisper-turbo") + assert tm.get_pending_downloads() == [] + assert tm.get_active_downloads() == [] + + +def test_cancel_dismisses_errored_download(): + tm = TaskManager() + tm.start_download("whisper-turbo") + tm.error_download("whisper-turbo", "boom") + assert tm.cancel_download("whisper-turbo") is True + assert tm.get_active_downloads() == [] + assert tm.get_pending_downloads() == [] diff --git a/backend/utils/tasks.py b/backend/utils/tasks.py index 8baf71c3..efec184e 100644 --- a/backend/utils/tasks.py +++ b/backend/utils/tasks.py @@ -67,6 +67,19 @@ class TaskManager: def get_active_downloads(self) -> List[DownloadTask]: """Get all active downloads.""" return list(self._active_downloads.values()) + + def get_pending_downloads(self) -> List[DownloadTask]: + """Get downloads that are still in flight. + + Excludes errored tasks, which stay in the active list so the + error/retry UI can show them but must not be reported as + "downloading" by /models/status. + """ + return [ + task + for task in self._active_downloads.values() + if task.status in ("downloading", "extracting") + ] def get_active_generations(self) -> List[GenerationTask]: """Get all active generations."""