--- phase: 07-redo-and-optimize-llm-integration plan: 04 subsystem: backend/tasks + backend/api tags: [celery, retry, classification, async, tdd] wave: 4 dependency_graph: requires: - 07-03 # load_provider_config + provider singletons provides: - D-09 # Celery exponential backoff (30/90/270s) - D-10 # Final classification_failed status writeback - D-11 # backend half: POST /classify re-queues Celery affects: - backend/tasks/document_tasks.py - backend/api/documents.py - backend/tests/test_document_tasks.py - backend/tests/test_documents.py tech_stack: added: [] patterns: - "_ClassificationError sentinel escapes asyncio.run() to trigger Celery retry in sync layer (Pitfall 3)" - "MaxRetriesExceededError caught in nested try/except inside except _ClassificationError block" - "push_request(retries=N) + patch.object(task, retry) pattern for testing bound Celery tasks" - "assert_called_once_with (not assert_awaited_once_with) for asyncio.run(AsyncMock(...)) calling convention" key_files: modified: - backend/tasks/document_tasks.py - backend/api/documents.py - backend/tests/test_document_tasks.py - backend/tests/test_documents.py decisions: - "MaxRetriesExceededError caught in nested try/except inside except _ClassificationError — not as sibling except — because raise self.retry() raises MaxRetriesExceededError during exception handling, which would propagate past a sibling except block" - "module-level extract_and_classify import retained in documents.py (already existed, already used in confirm and upload handlers) — no deferred import needed" - "test_reclassify_cross_user_returns_404 added as new IDOR test for classify endpoint (plan said 'regression of existing IDOR test' but none existed for classify; added new test per security mandate)" metrics: completed_date: "2026-06-04" tasks_completed: 2 tasks_total: 2 files_modified: 4 --- # Phase 07 Plan 04: Celery Retry Harness + POST /classify Re-queue Summary ## One-liner Celery exponential-backoff retry harness (30s/90s/270s) via `_ClassificationError` sentinel + MaxRetriesExceededError nested catch, and `POST /classify` converted from synchronous classification to Celery re-queue. ## Tasks Completed | Task | Name | Commit | Files | |------|------|--------|-------| | 1 | Celery retry harness + _ClassificationError + classification_failed writeback | e9ee5d4 | document_tasks.py, test_document_tasks.py | | 2 | POST /api/documents/{id}/classify → re-queue Celery | 63cd707 | documents.py, test_documents.py | ## What Was Built ### Task 1: Celery retry harness (D-09 / D-10) `backend/tasks/document_tasks.py` was refactored to implement the self-healing retry loop: - `class _ClassificationError(Exception)` — sentinel raised by `_run()` when `classifier.classify_document()` fails; escapes `asyncio.run()` and is caught by the outer sync task - `async def _mark_classification_failed(document_id)` — writes `doc.status = "classification_failed"` after all retries exhausted; called via `asyncio.run(...)` from the `MaxRetriesExceededError` handler - Task decorator changed to `@celery_app.task(name=..., bind=True, max_retries=3)` - Retry loop: `except _ClassificationError as exc` → `countdowns = [30, 90, 270]` → `raise self.retry(exc=exc, countdown=countdown)` - `MaxRetriesExceededError` caught in a **nested** `try/except` inside the `_ClassificationError` handler (not as a sibling `except`) because `raise self.retry()` raises `MaxRetriesExceededError` during exception handling context **Critical architectural decision:** The nested catch was required. If `MaxRetriesExceededError` were a sibling `except` block after `except _ClassificationError`, Celery's `self.retry()` call raising `MaxRetriesExceededError` inside the `_ClassificationError` handler would propagate past it without being caught. ### Task 2: POST /classify re-queue (D-11) `backend/api/documents.py` classify endpoint was refactored: - Removed: `await classifier.classify_document(session, doc_id, topic_names)` and the topics response - Added: `doc.status = "processing"`, `await session.commit()`, `extract_and_classify.delay(str(doc.id))` - Returns: `{"document_id": str(doc.id), "status": "processing"}` with HTTP 200 - Ownership check retained inline (no `_get_owned_doc` helper existed; plan said to replicate if absent) - `body: dict = {}` parameter removed (no longer needed since topics are written by the Celery task) ## Test Coverage ### Promoted from xfail: - `test_document_tasks.py::test_retry_backoff` — verifies countdowns [30, 90, 270] for retries 0/1/2 - `test_document_tasks.py::test_exhaustion_sets_failed_status` — verifies `_mark_classification_failed` called and return dict correct - `test_documents.py::test_reclassify_requeues_celery` — verifies Celery delay called, doc.status=processing, response correct ### New tests added: - `test_documents.py::test_reclassify_cross_user_returns_404` — IDOR test for classify endpoint (T-07-10) ### Test suite result: - Before Plan 04: 366 passed, 12 xfailed, 1 pre-existing failure - After Plan 04: 370 passed, 9 xfailed, 1 pre-existing failure (test_extract_docx missing module — unrelated) - Net: +4 promoted tests, +1 new IDOR test ## Deviations from Plan ### Auto-fixed Issues **1. [Rule 1 - Bug] MaxRetriesExceededError requires nested try/except, not sibling** - **Found during:** Task 1 implementation — tests confirmed the structure - **Issue:** The plan described `except MaxRetriesExceededError` as a sibling handler after `except _ClassificationError`. However, `raise self.retry(exc=exc, countdown=countdown)` is called inside the `except _ClassificationError` block. When `self.retry()` raises `MaxRetriesExceededError`, Python is already unwinding the `_ClassificationError` handler — a sibling `except MaxRetriesExceededError` would not catch it. - **Fix:** Used nested `try/except MaxRetriesExceededError` inside the `except _ClassificationError` block - **Files modified:** `backend/tasks/document_tasks.py` - **Commit:** e9ee5d4 **2. [Rule 2 - Missing Critical Functionality] Added IDOR test for classify endpoint** - **Found during:** Task 2 — plan referenced "regression of existing IDOR test" but no such test existed for POST /classify - **Issue:** T-07-10 (cross-user reclassify → 404) had no test coverage - **Fix:** Added `test_reclassify_cross_user_returns_404` - **Files modified:** `backend/tests/test_documents.py` - **Commit:** 63cd707 ### No architectural changes required. ## Threat Surface Scan No new network endpoints or trust boundaries introduced. The `POST /classify` endpoint already existed; this plan changed its implementation from synchronous to async-queued. The IDOR invariant (T-07-10) is enforced by the ownership check `doc.user_id != current_user.id → 404` — same pattern as all other document endpoints. No new entries needed in threat model. ## Known Stubs None — all classification path changes write real state to the DB via the Celery task. ## Self-Check: PASSED - `backend/tasks/document_tasks.py` contains `class _ClassificationError`, `bind=True`, `max_retries=3`, `async def _mark_classification_failed`, `countdowns = [30, 90, 270]`, `raise self.retry(exc=` - `backend/api/documents.py` POST classify contains `extract_and_classify.delay(` and `doc.status = "processing"`, does NOT contain `await classifier.classify_document(` - Commits e9ee5d4 and 63cd707 exist in git log - `pytest tests/test_document_tasks.py tests/test_documents.py::test_reclassify_requeues_celery` exits 0 - Full suite: 370 passed, 1 pre-existing failure (test_extract_docx — unrelated)