fix(downloads): accept download_id in history delete/retry endpoints, add unique index

py/routes/handlers/model_handlers.py

@@ -1787,14 +1787,20 @@ class ModelDownloadHandler:
 
     async def delete_download_history_item(self, request: web.Request) -> web.Response:
         try:
-            item_id = int(request.query.get("id", "0"))
-            if not item_id:
+            download_id = request.query.get("download_id")
+            id_str = request.query.get("id")
+            item_id = int(id_str) if id_str else None
+
+            if not download_id and not item_id:
                 return web.json_response(
-                    {"success": False, "error": "id is required"}, status=400
+                    {"success": False, "error": "id or download_id is required"},
+                    status=400,
                 )
 
             service = await DownloadQueueService.get_instance()
-            deleted = await service.delete_history_item(item_id)
+            deleted = await service.delete_history_item(
+                id=item_id, download_id=download_id
+            )
             return web.json_response({"success": deleted})
         except Exception as exc:
             self._logger.error(
@@ -1804,14 +1810,20 @@ class ModelDownloadHandler:
 
     async def retry_download_from_history(self, request: web.Request) -> web.Response:
         try:
-            item_id = int(request.query.get("id", "0"))
-            if not item_id:
+            download_id = request.query.get("download_id")
+            id_str = request.query.get("id")
+            item_id = int(id_str) if id_str else None
+
+            if not download_id and not item_id:
                 return web.json_response(
-                    {"success": False, "error": "id is required"}, status=400
+                    {"success": False, "error": "id or download_id is required"},
+                    status=400,
                 )
 
             service = await DownloadQueueService.get_instance()
-            item = await service.retry_from_history(item_id)
+            item = await service.retry_from_history(
+                item_id=item_id, download_id=download_id
+            )
             if item is None:
                 return web.json_response(
                     {"success": False, "error": "History item not found or not retryable"},

py/services/download_queue_service.py

@@ -74,6 +74,8 @@ class DownloadQueueService:
         );
         CREATE INDEX IF NOT EXISTS idx_dh_completed ON download_history(completed_at DESC);
         CREATE INDEX IF NOT EXISTS idx_dh_status ON download_history(status);
+        CREATE UNIQUE INDEX IF NOT EXISTS idx_dh_download_id
+            ON download_history(download_id) WHERE download_id IS NOT NULL;
     """
 
     @classmethod
@@ -390,7 +392,7 @@ class DownloadQueueService:
             )
             conn.execute(
                 """
-                INSERT INTO download_history (
+                INSERT OR IGNORE INTO download_history (
                     download_id, model_id, model_version_id, model_name,
                     version_name, thumbnail_url, status, error, file_path,
                     bytes_downloaded, total_bytes, completed_at
@@ -547,17 +549,27 @@ class DownloadQueueService:
             "offset": offset,
         }
 
-    async def delete_history_item(self, id: int) -> bool:
-        """Delete a single history entry by its *id*.
+    async def delete_history_item(
+        self, id: Optional[int] = None, download_id: Optional[str] = None
+    ) -> bool:
+        """Delete a single history entry by *download_id* (preferred) or *id*.
 
         Returns ``True`` if a row was deleted.
         """
         async with self._lock:
             conn = self._get_conn()
-            cursor = conn.execute(
-                "DELETE FROM download_history WHERE id = ?",
-                (id,),
-            )
+            if download_id:
+                cursor = conn.execute(
+                    "DELETE FROM download_history WHERE download_id = ?",
+                    (download_id,),
+                )
+            elif id is not None:
+                cursor = conn.execute(
+                    "DELETE FROM download_history WHERE id = ?",
+                    (id,),
+                )
+            else:
+                return False
             conn.commit()
         return cursor.rowcount > 0
 
@@ -614,21 +626,34 @@ class DownloadQueueService:
     # Retry
     # ------------------------------------------------------------------
 
-    async def retry_from_history(self, item_id: int) -> Optional[dict[str, Any]]:
+    async def retry_from_history(
+        self,
+        item_id: Optional[int] = None,
+        download_id: Optional[str] = None,
+    ) -> Optional[dict[str, Any]]:
         """Re-queue a failed or canceled download from history.
 
-        Looks up the history record by its primary key.  If the status is
-        ``failed`` or ``canceled`` a new queue entry is created with the
-        same model metadata and a fresh download id, and the original
-        history entry is **deleted** to prevent exponential growth when
-        the retried item is later canceled or fails again and re-retried.
+        Looks up the history record by *download_id* (preferred) or
+        *item_id*.  If the status is ``failed`` or ``canceled`` a new
+        queue entry is created with the same model metadata and a fresh
+        download id, and the original history entry is **deleted** to
+        prevent exponential growth when the retried item is later
+        canceled or fails again and re-retried.
         """
         async with self._lock:
             conn = self._get_conn()
-            row = conn.execute(
-                "SELECT * FROM download_history WHERE id = ?",
-                (item_id,),
-            ).fetchone()
+            if download_id:
+                row = conn.execute(
+                    "SELECT * FROM download_history WHERE download_id = ?",
+                    (download_id,),
+                ).fetchone()
+            elif item_id is not None:
+                row = conn.execute(
+                    "SELECT * FROM download_history WHERE id = ?",
+                    (item_id,),
+                ).fetchone()
+            else:
+                return None
             if row is None:
                 return None
             status = str(row["status"])
@@ -660,7 +685,7 @@ class DownloadQueueService:
             )
             conn.execute(
                 "DELETE FROM download_history WHERE id = ?",
-                (item_id,),
+                (row["id"],),
             )
             conn.commit()
             queued = conn.execute(

tests/services/test_download_queue_service.py

@@ -0,0 +1,193 @@
+"""Unit tests for DownloadQueueService history operations.
+
+Covers the new ``download_id``-based code paths in
+``delete_history_item`` and ``retry_from_history``, plus backward
+compatibility with ``id``.
+"""
+
+from pathlib import Path
+
+import pytest
+
+from py.services.download_queue_service import DownloadQueueService
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+def _make_service(tmp_path: Path) -> DownloadQueueService:
+    """Create a DownloadQueueService backed by a temporary database."""
+    return DownloadQueueService(db_path=str(tmp_path / "queue.sqlite"))
+
+
+async def _seed(
+    svc: DownloadQueueService,
+    download_id: str,
+    status: str = "failed",
+) -> tuple[int, str]:
+    """Insert a history row and return (autoincrement id, download_id)."""
+    row_id = await svc.add_to_history(
+        download_id=download_id,
+        model_id=1,
+        model_version_id=100,
+        model_name="TestModel",
+        version_name="v1",
+        status=status,
+    )
+    return row_id, download_id
+
+
+# ---------------------------------------------------------------------------
+# delete_history_item
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_delete_by_download_id(tmp_path: Path) -> None:
+    """delete_history_item(download_id=...) removes the correct row."""
+    svc = _make_service(tmp_path)
+    rid, did = await _seed(svc, "dl-aaa")
+
+    deleted = await svc.delete_history_item(download_id=did)
+    assert deleted is True
+
+    # Verify gone from history
+    history = await svc.get_history()
+    assert len(history["items"]) == 0
+
+
+@pytest.mark.asyncio
+async def test_delete_by_id_legacy(tmp_path: Path) -> None:
+    """delete_history_item(id=...) still works (backward compat)."""
+    svc = _make_service(tmp_path)
+    rid, _did = await _seed(svc, "dl-bbb")
+
+    deleted = await svc.delete_history_item(id=rid)
+    assert deleted is True
+
+    history = await svc.get_history()
+    assert len(history["items"]) == 0
+
+
+@pytest.mark.asyncio
+async def test_delete_no_params_returns_false(tmp_path: Path) -> None:
+    """Calling delete_history_item with no params returns False."""
+    svc = _make_service(tmp_path)
+    await _seed(svc, "dl-ccc")
+
+    deleted = await svc.delete_history_item()
+    assert deleted is False
+
+    # Row is still there
+    history = await svc.get_history()
+    assert len(history["items"]) == 1
+
+
+@pytest.mark.asyncio
+async def test_delete_download_id_precedence(tmp_path: Path) -> None:
+    """When both id and download_id are given, download_id is used."""
+    svc = _make_service(tmp_path)
+    # Insert two rows
+    rid_a, did_a = await _seed(svc, "dl-aaa")
+    rid_b, did_b = await _seed(svc, "dl-bbb")
+
+    # Delete by download_id while also passing the *wrong* id
+    deleted = await svc.delete_history_item(id=rid_b, download_id=did_a)
+    assert deleted is True
+
+    history = await svc.get_history()
+    ids_left = [it["id"] for it in history["items"]]
+    assert rid_a not in ids_left  # dl-aaa was deleted
+    assert rid_b in ids_left     # dl-bbb (wrong id) was ignored
+
+
+# ---------------------------------------------------------------------------
+# retry_from_history
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_retry_by_download_id(tmp_path: Path) -> None:
+    """retry_from_history(download_id=...) re-queues and deletes history."""
+    svc = _make_service(tmp_path)
+    rid, did = await _seed(svc, "dl-fail", status="failed")
+
+    item = await svc.retry_from_history(download_id=did)
+    assert item is not None
+    assert item["status"] == "queued"
+
+    # History row must be deleted (the bug fix)
+    history = await svc.get_history()
+    ids_in_history = [it["id"] for it in history["items"]]
+    assert rid not in ids_in_history
+
+    # Queue must contain the new item
+    queue = await svc.get_queue()
+    assert len(queue) == 1
+
+
+@pytest.mark.asyncio
+async def test_retry_by_download_id_canceled(tmp_path: Path) -> None:
+    """retry_from_history works for 'canceled' status too."""
+    svc = _make_service(tmp_path)
+    rid, did = await _seed(svc, "dl-cancel", status="canceled")
+
+    item = await svc.retry_from_history(download_id=did)
+    assert item is not None
+    assert item["status"] == "queued"
+
+    history = await svc.get_history()
+    assert len(history["items"]) == 0
+
+
+@pytest.mark.asyncio
+async def test_retry_by_id_legacy(tmp_path: Path) -> None:
+    """retry_from_history(item_id=...) still works (backward compat)."""
+    svc = _make_service(tmp_path)
+    rid, _did = await _seed(svc, "dl-legacy", status="failed")
+
+    item = await svc.retry_from_history(item_id=rid)
+    assert item is not None
+    assert item["status"] == "queued"
+
+    history = await svc.get_history()
+    assert len(history["items"]) == 0
+
+
+@pytest.mark.asyncio
+async def test_retry_no_params_returns_none(tmp_path: Path) -> None:
+    """Calling retry_from_history with no params returns None."""
+    svc = _make_service(tmp_path)
+    await _seed(svc, "dl-none", status="failed")
+
+    item = await svc.retry_from_history()
+    assert item is None
+
+    # History untouched
+    history = await svc.get_history()
+    assert len(history["items"]) == 1
+
+
+@pytest.mark.asyncio
+async def test_retry_non_retryable_status(tmp_path: Path) -> None:
+    """retry_from_history returns None for 'completed' status."""
+    svc = _make_service(tmp_path)
+    _rid, did = await _seed(svc, "dl-ok", status="completed")
+
+    item = await svc.retry_from_history(download_id=did)
+    assert item is None
+
+    # History untouched
+    history = await svc.get_history()
+    assert len(history["items"]) == 1
+
+
+@pytest.mark.asyncio
+async def test_retry_unknown_download_id(tmp_path: Path) -> None:
+    """retry_from_history returns None for a non-existent download_id."""
+    svc = _make_service(tmp_path)
+    await _seed(svc, "dl-real", status="failed")
+
+    item = await svc.retry_from_history(download_id="dl-nope")
+    assert item is None