Download history refactor pt.2 (#703)

Two-phase download history: downloads are now recorded in the DB at
queue time (not just at terminal time), eliminating the need to
reconstruct metadata in the terminal hook and removing the
`_is_graduated_request_download()` request-scan mess

tests/core/test_activity_routes_api.py

@@ -53,8 +53,10 @@ def _record_terminal_download(
     final_status: str = "complete",
     request_id: int | None = None,
     status_message: str | None = None,
+    download_path: str | None = None,
 ) -> None:
-    main_module.download_history_service.record_terminal(
+    svc = main_module.download_history_service
+    svc.record_download(
         task_id=task_id,
         user_id=user_id,
         username=username,
@@ -68,9 +70,12 @@ def _record_terminal_download(
         preview=None,
         content_type="ebook",
         origin=origin,
+    )
+    svc.finalize_download(
+        task_id=task_id,
         final_status=final_status,
         status_message=status_message,
-        download_path=None,
+        download_path=download_path,
     )
 
 
@@ -162,6 +167,36 @@ class TestActivityRoutes:
         assert history_after_clear.status_code == 200
         assert history_after_clear.json == []
 
+    def test_dismiss_preserves_terminal_snapshot_without_live_queue_merge(self, main_module, client):
+        user = _create_user(main_module, prefix="reader")
+        _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
+
+        task_id = "dismiss-preserve-task"
+        _record_terminal_download(
+            main_module,
+            task_id=task_id,
+            user_id=user["id"],
+            username=user["username"],
+            title="Recorded Title",
+            author="Recorded Author",
+            status_message="Complete",
+        )
+
+        with patch.object(main_module, "get_auth_mode", return_value="builtin"):
+            dismiss_response = client.post(
+                "/api/activity/dismiss",
+                json={"item_type": "download", "item_key": f"download:{task_id}"},
+            )
+            history_response = client.get("/api/activity/history?limit=10&offset=0")
+
+        assert dismiss_response.status_code == 200
+        assert history_response.status_code == 200
+        assert history_response.json[0]["item_key"] == f"download:{task_id}"
+        snapshot_download = history_response.json[0]["snapshot"]["download"]
+        assert snapshot_download["title"] == "Recorded Title"
+        assert snapshot_download["author"] == "Recorded Author"
+        assert snapshot_download["status_message"] is None
+
     def test_clear_history_deletes_dismissed_requests_from_snapshot(self, main_module, client):
         user = _create_user(main_module, prefix="reader")
         _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
@@ -248,27 +283,6 @@ class TestActivityRoutes:
             user_id=user["id"],
             username=user["username"],
             title="History Local Download",
-        )
-
-        row = main_module.download_history_service.get_by_task_id(task_id)
-        assert row is not None
-        assert main_module.download_history_service is not None
-        main_module.download_history_service.record_terminal(
-            task_id=task_id,
-            user_id=user["id"],
-            username=user["username"],
-            request_id=row.get("request_id"),
-            source=row.get("source") or "direct_download",
-            source_display_name=row.get("source_display_name"),
-            title=row.get("title") or "History Local Download",
-            author=row.get("author"),
-            format=row.get("format"),
-            size=row.get("size"),
-            preview=row.get("preview"),
-            content_type=row.get("content_type"),
-            origin=row.get("origin") or "direct",
-            final_status=row.get("final_status") or "complete",
-            status_message=row.get("status_message"),
             download_path=str(file_path),
         )
 
@@ -295,7 +309,7 @@ class TestActivityRoutes:
                 "provider_id": "legacy-fulfilled-1",
             },
             status="fulfilled",
-            delivery_state="unknown",
+            delivery_state="none",
         )
 
         with patch.object(main_module, "get_auth_mode", return_value="builtin"):
@@ -355,6 +369,51 @@ class TestActivityRoutes:
             to=f"user_{user['id']}",
         )
 
+    def test_dismiss_many_preserves_terminal_snapshots_without_live_queue_merge(self, main_module, client):
+        user = _create_user(main_module, prefix="reader")
+        _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
+
+        first_task_id = "dismiss-many-preserve-1"
+        second_task_id = "dismiss-many-preserve-2"
+        _record_terminal_download(
+            main_module,
+            task_id=first_task_id,
+            user_id=user["id"],
+            username=user["username"],
+            title="First Title",
+            author="First Author",
+        )
+        _record_terminal_download(
+            main_module,
+            task_id=second_task_id,
+            user_id=user["id"],
+            username=user["username"],
+            title="Second Title",
+            author="Second Author",
+        )
+
+        with patch.object(main_module, "get_auth_mode", return_value="builtin"):
+            dismiss_many_response = client.post(
+                "/api/activity/dismiss-many",
+                json={
+                    "items": [
+                        {"item_type": "download", "item_key": f"download:{first_task_id}"},
+                        {"item_type": "download", "item_key": f"download:{second_task_id}"},
+                    ]
+                },
+            )
+            history_response = client.get("/api/activity/history?limit=20&offset=0")
+
+        assert dismiss_many_response.status_code == 200
+        assert dismiss_many_response.json["count"] == 2
+        assert history_response.status_code == 200
+
+        rows_by_key = {row["item_key"]: row for row in history_response.json}
+        assert rows_by_key[f"download:{first_task_id}"]["snapshot"]["download"]["title"] == "First Title"
+        assert rows_by_key[f"download:{first_task_id}"]["snapshot"]["download"]["author"] == "First Author"
+        assert rows_by_key[f"download:{second_task_id}"]["snapshot"]["download"]["title"] == "Second Title"
+        assert rows_by_key[f"download:{second_task_id}"]["snapshot"]["download"]["author"] == "Second Author"
+
     def test_no_auth_dismiss_many_and_history_use_shared_identity(self, main_module):
         task_id = f"no-auth-{uuid.uuid4().hex[:10]}"
         item_key = f"download:{task_id}"
@@ -513,6 +572,77 @@ class TestActivityRoutes:
         assert "cross-user-expired-task" in response.json["status"]["complete"]
         assert response.json["status"]["complete"]["cross-user-expired-task"]["id"] == "cross-user-expired-task"
 
+    def test_snapshot_shows_stale_active_download_as_interrupted_error(self, main_module, client):
+        user = _create_user(main_module, prefix="reader")
+        _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
+
+        # Record a download at queue time (active status) but don't put it in the queue
+        main_module.download_history_service.record_download(
+            task_id="stale-active-task",
+            user_id=user["id"],
+            username=user["username"],
+            request_id=None,
+            source="direct_download",
+            source_display_name="Direct Download",
+            title="Stale Active Task",
+            author="Stale Author",
+            format="epub",
+            size="1 MB",
+            preview=None,
+            content_type="ebook",
+            origin="direct",
+        )
+
+        with patch.object(main_module, "get_auth_mode", return_value="builtin"):
+            with patch.object(main_module.backend, "queue_status", return_value=_sample_status_payload()):
+                response = client.get("/api/activity/snapshot")
+
+        assert response.status_code == 200
+        assert "stale-active-task" in response.json["status"]["error"]
+        assert response.json["status"]["error"]["stale-active-task"]["status_message"] == "Interrupted"
+
+    def test_snapshot_active_download_with_queue_entry_shows_in_correct_bucket(self, main_module, client):
+        user = _create_user(main_module, prefix="reader")
+        _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
+
+        # Record a download at queue time
+        main_module.download_history_service.record_download(
+            task_id="active-downloading-task",
+            user_id=user["id"],
+            username=user["username"],
+            request_id=None,
+            source="direct_download",
+            source_display_name="Direct Download",
+            title="Active Downloading Task",
+            author="Active Author",
+            format="epub",
+            size="2 MB",
+            preview=None,
+            content_type="ebook",
+            origin="direct",
+        )
+
+        # Simulate it being active in the queue
+        active_status = _sample_status_payload()
+        active_status["downloading"] = {
+            "active-downloading-task": {
+                "id": "active-downloading-task",
+                "title": "Active Downloading Task",
+                "author": "Active Author",
+                "source": "direct_download",
+                "progress": 0.5,
+                "status_message": "Downloading 50%",
+            }
+        }
+
+        with patch.object(main_module, "get_auth_mode", return_value="builtin"):
+            with patch.object(main_module.backend, "queue_status", return_value=active_status):
+                response = client.get("/api/activity/snapshot")
+
+        assert response.status_code == 200
+        assert "active-downloading-task" in response.json["status"]["downloading"]
+        assert response.json["status"]["downloading"]["active-downloading-task"]["progress"] == 0.5
+
     def test_snapshot_clears_stale_download_dismissal_when_same_task_is_active(self, main_module, client):
         user = _create_user(main_module, prefix="reader")
         _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)

tests/core/test_activity_terminal_snapshots.py

@@ -93,6 +93,7 @@ class TestTerminalSnapshotCapture:
             title="Requested Snapshot",
             user_id=user["id"],
             username=user["username"],
+            request_id=request_row["id"],
         )
         assert main_module.backend.book_queue.add(task) is True
 
@@ -193,6 +194,103 @@ class TestTerminalSnapshotCapture:
         finally:
             main_module.backend.book_queue.cancel_download(task_id)
 
+    def test_queue_hook_records_active_row_at_queue_time(self, main_module):
+        user = _create_user(main_module, prefix="snap-queue")
+        task_id = f"queue-{uuid.uuid4().hex[:8]}"
+        task = DownloadTask(
+            task_id=task_id,
+            source="direct_download",
+            title="Queue Time Snapshot",
+            author="Queue Author",
+            user_id=user["id"],
+            username=user["username"],
+        )
+        assert main_module.backend.book_queue.add(task) is True
+
+        try:
+            row = _read_download_history_row(main_module, task_id)
+            assert row is not None
+            assert row["final_status"] == "active"
+            assert row["user_id"] == user["id"]
+            assert row["task_id"] == task_id
+            assert row["origin"] == "direct"
+            assert row["title"] == "Queue Time Snapshot"
+            assert row["author"] == "Queue Author"
+            assert row["queued_at"] is not None
+        finally:
+            main_module.backend.book_queue.cancel_download(task_id)
+
+    def test_queue_hook_records_requested_origin_for_request_linked_task(self, main_module):
+        user = _create_user(main_module, prefix="snap-queue-req")
+        task_id = f"queue-req-{uuid.uuid4().hex[:8]}"
+        request_row = main_module.user_db.create_request(
+            user_id=user["id"],
+            content_type="ebook",
+            request_level="release",
+            policy_mode="request_release",
+            book_data={
+                "title": "Requested Queue",
+                "author": "Request Author",
+                "provider": "openlibrary",
+                "provider_id": "queue-req-1",
+            },
+            release_data={
+                "source": "prowlarr",
+                "source_id": task_id,
+                "title": "Requested Queue.epub",
+            },
+            status="fulfilled",
+            delivery_state="queued",
+        )
+        task = DownloadTask(
+            task_id=task_id,
+            source="prowlarr",
+            title="Requested Queue",
+            user_id=user["id"],
+            username=user["username"],
+            request_id=request_row["id"],
+        )
+        assert main_module.backend.book_queue.add(task) is True
+
+        try:
+            row = _read_download_history_row(main_module, task_id)
+            assert row is not None
+            assert row["final_status"] == "active"
+            assert row["origin"] == "requested"
+            assert row["request_id"] == request_row["id"]
+        finally:
+            main_module.backend.book_queue.cancel_download(task_id)
+
+    def test_finalize_updates_active_row_to_terminal(self, main_module):
+        user = _create_user(main_module, prefix="snap-finalize")
+        task_id = f"finalize-{uuid.uuid4().hex[:8]}"
+        task = DownloadTask(
+            task_id=task_id,
+            source="direct_download",
+            title="Finalize Snapshot",
+            user_id=user["id"],
+            username=user["username"],
+        )
+        assert main_module.backend.book_queue.add(task) is True
+
+        try:
+            # Verify active row exists
+            row = _read_download_history_row(main_module, task_id)
+            assert row is not None
+            assert row["final_status"] == "active"
+
+            # Transition to complete
+            main_module.backend.book_queue.update_status(task_id, QueueStatus.COMPLETE)
+
+            row = _read_download_history_row(main_module, task_id)
+            assert row is not None
+            assert row["final_status"] == "complete"
+            # Metadata from queue-time should be preserved
+            assert row["title"] == "Finalize Snapshot"
+            assert row["user_id"] == user["id"]
+        finally:
+            main_module.backend.book_queue.cancel_download(task_id)
+
     def test_cancelled_transition_does_not_trigger_notification(self, main_module):
         user = _create_user(main_module, prefix="snap-notify-cancel")
         task_id = f"notify-cancel-{uuid.uuid4().hex[:8]}"

tests/core/test_download_api_guardrails.py

@@ -535,7 +535,7 @@ class TestCancelDownloadEndpointGuardrails:
             db_user_id=user["id"],
             is_admin=False,
         )
-        main_module.user_db.create_request(
+        request_row = main_module.user_db.create_request(
             user_id=user["id"],
             content_type="ebook",
             request_level="release",
@@ -560,6 +560,7 @@ class TestCancelDownloadEndpointGuardrails:
             title="Requested Book",
             user_id=user["id"],
             username=user["username"],
+            request_id=request_row["id"],
         )
 
         with patch.object(main_module, "get_auth_mode", return_value="builtin"):
@@ -721,7 +722,7 @@ class TestRetryDownloadEndpointGuardrails:
             db_user_id=user["id"],
             is_admin=False,
         )
-        main_module.user_db.create_request(
+        request_row = main_module.user_db.create_request(
             user_id=user["id"],
             content_type="ebook",
             request_level="release",
@@ -746,6 +747,7 @@ class TestRetryDownloadEndpointGuardrails:
             title="Requested Book",
             user_id=user["id"],
             username=user["username"],
+            request_id=request_row["id"],
         )
 
         with patch.object(main_module, "get_auth_mode", return_value="builtin"):

tests/core/test_request_routes_api.py

@@ -922,7 +922,7 @@ class TestRequestRoutes:
                     fulfil_resp = client.post(f"/api/admin/requests/{request_id}/fulfil", json={})
 
         assert fulfil_resp.status_code == 400
-        assert "release_data is required to fulfil book-level requests" in fulfil_resp.json["error"]
+        assert "release_data is required to fulfil requests" in fulfil_resp.json["error"]
 
     def test_admin_fulfil_book_level_request_manual_approval(self, main_module, client):
         user = _create_user(main_module, prefix="reader")

tests/core/test_requests_service.py

@@ -307,7 +307,7 @@ def test_fulfil_request_requires_release_data_for_book_level(user_db):
         book_data=_book_data(),
     )
 
-    with pytest.raises(RequestServiceError, match="release_data is required to fulfil book-level requests"):
+    with pytest.raises(RequestServiceError, match="release_data is required to fulfil requests"):
         fulfil_request(
             user_db,
             request_id=created["id"],

tests/core/test_user_db.py

@@ -818,9 +818,6 @@ class TestDownloadRequests:
             book_data=self._book_data(),
         )
 
-        with pytest.raises(ValueError, match="request_level=release requires non-null release_data"):
-            user_db.update_request(created["id"], request_level="release")
-
         updated = user_db.update_request(
             created["id"],
             request_level="release",
@@ -1015,36 +1012,6 @@ class TestDownloadRequests:
         bob_rows = user_db.list_dismissed_requests(user_id=bob["id"])
         assert [row["id"] for row in bob_rows] == [second["id"]]
 
-    def test_clear_request_dismissals_scopes_by_user(self, user_db):
-        alice = user_db.create_user(username="alice")
-        bob = user_db.create_user(username="bob")
-        alice_request = user_db.create_request(
-            user_id=alice["id"],
-            content_type="ebook",
-            request_level="book",
-            policy_mode="request_book",
-            book_data=self._book_data(),
-        )
-        bob_request = user_db.create_request(
-            user_id=bob["id"],
-            content_type="ebook",
-            request_level="book",
-            policy_mode="request_book",
-            book_data=self._book_data(),
-        )
-
-        user_db.update_request(alice_request["id"], dismissed_at="2026-01-01T10:00:00+00:00")
-        user_db.update_request(bob_request["id"], dismissed_at="2026-01-01T11:00:00+00:00")
-
-        cleared_alice = user_db.clear_request_dismissals(user_id=alice["id"])
-        assert cleared_alice == 1
-        assert user_db.get_request(alice_request["id"])["dismissed_at"] is None
-        assert user_db.get_request(bob_request["id"])["dismissed_at"] is not None
-
-        cleared_all = user_db.clear_request_dismissals(user_id=None)
-        assert cleared_all == 1
-        assert user_db.get_request(bob_request["id"])["dismissed_at"] is None
-
     def test_delete_dismissed_requests_scopes_by_user_and_only_deletes_terminal(self, user_db):
         alice = user_db.create_user(username="alice")
         bob = user_db.create_user(username="bob")
@@ -1092,8 +1059,5 @@ class TestDownloadRequests:
         with pytest.raises(ValueError, match="user_id must be a positive integer"):
             user_db.list_dismissed_requests(user_id=0)
 
-        with pytest.raises(ValueError, match="user_id must be a positive integer"):
-            user_db.clear_request_dismissals(user_id=-1)
-
         with pytest.raises(ValueError, match="user_id must be a positive integer"):
             user_db.delete_dismissed_requests(user_id=0)