fix: schedule superseded-check cleanup (#261)

config/settings/base.py

@@ -551,6 +551,10 @@ def required_host_list(var: str) -> list[str]:
         "task": "wafer_space.projects.tasks_checks.checks_cleanup_stale_pending_tasks",
         "schedule": 60.0,
     },
+    "checks-cleanup-superseded": {
+        "task": "wafer_space.projects.tasks_checks.checks_cleanup_superseded",
+        "schedule": 60.0,
+    },
     "checks-drc-update-requeue": {
         "task": "wafer_space.projects.tasks_checks.checks_drc_update_requeue",
         "schedule": 60.0,

docs/celery_tasks_reference.md

@@ -49,8 +49,8 @@ These tasks run periodically via Celery Beat to poll for checks needing action.
 | `checks_retry`                       | `none:ro:checks-orch` | 60s      | Create retry checks for ERROR state   |
 | `checks_cleanup_stale_files`         | `none:ro:checks-orch` | 60s      | Cancel checks on inactive files       |
 | `checks_cleanup_stale_pending_tasks` | `none:ro:checks-orch` | 60s      | Remove orphaned task tracking records |
+| `checks_cleanup_superseded`          | `none:ro:checks-orch` | 60s      | Cancel checks superseded by newer ones |
 | `checks_drc_update_requeue`          | `none:ro:checks-orch` | 60s      | Create DRC_UPDATE checks for outdated versions |
-| `checks_cleanup`                     | `none:ro:checks-orch` | -        | Combined cleanup operations           |
 
 ### Work Tasks
 

wafer_space/projects/models.py

@@ -2370,8 +2370,9 @@ def root_check(self) -> "ManufacturabilityCheck":
     def create_check_drc_update(self) -> "ManufacturabilityCheck":
         """Create a new pending check to re-run with latest precheck version.
 
-        If this check is still in progress, it will be automatically cancelled
-        by the existing superseded check cleanup logic.
+        If this check is still in progress, the newly created check supersedes
+        it; the older in-progress check is then cancelled by the scheduled
+        ``checks_cleanup_superseded`` task.
 
         Returns:
             The newly created ManufacturabilityCheck.

wafer_space/projects/tasks_checks.py

@@ -191,10 +191,10 @@ def wrapper(check_id: int, *args: Any, **kwargs: Any) -> T | dict[str, str]:
 __all__ = [
     "checks_analyzing",
     "checks_cancelling",
-    "checks_cleanup",
     "checks_cleanup_orphaned_docker",
     "checks_cleanup_stale_files",
     "checks_cleanup_stale_pending_tasks",
+    "checks_cleanup_superseded",
     "checks_create",
     "checks_dispatching",
     "checks_drc_update_requeue",
@@ -1948,11 +1948,21 @@ def checks_cleanup_stale_files() -> dict:
     return {"cancelled": cancelled}
 
 
-def _cancel_superseded_checks() -> int:
+@checks_task()
+def checks_cleanup_superseded() -> dict:
     """Cancel in-progress checks that have been superseded by newer checks.
 
+    A check is "superseded" when a newer check exists for the same project
+    file while the older one is still in progress. This happens via the manual
+    DRC-update requeue view (``check_drc_update_requeue``), which can create a
+    fresh check for a file whose latest check is still running. The older,
+    now-redundant in-progress check is marked for cancellation here.
+
+    The scheduled ``checks_drc_update_requeue`` beat task does not produce this
+    situation, because it only requeues FINISHED checks.
+
     Returns:
-        Number of checks marked for cancellation.
+        Dict with 'cancelled' count of checks marked for cancellation.
     """
     logger = logging.getLogger(__name__)
 
@@ -1962,10 +1972,16 @@ def _cancel_superseded_checks() -> int:
         created_at__gt=OuterRef("created_at"),
     )
 
-    # Find all superseded in-progress checks
-    superseded = ManufacturabilityCheck.objects.filter(
-        status__in=ManufacturabilityCheck.Status.in_progress(),
-    ).filter(Exists(newer_exists))
+    # Find all superseded in-progress checks. Checks already in CANCELLING
+    # are excluded: CANCELLING -> CANCELLING is not a valid transition, and
+    # the checks_cancelling task completes them.
+    superseded = (
+        ManufacturabilityCheck.objects.filter(
+            status__in=ManufacturabilityCheck.Status.in_progress(),
+        )
+        .exclude(status=ManufacturabilityCheck.Status.CANCELLING)
+        .filter(Exists(newer_exists))
+    )
 
     cancelled = 0
     for check in superseded:
@@ -1976,40 +1992,10 @@ def _cancel_superseded_checks() -> int:
                 check.id,
             )
             cancelled += 1
-        except Exception:
+        except InvalidStateTransitionError:
             logger.exception("Failed to cancel superseded check %s", check.id)
 
-    return cancelled
-
-
-@checks_task()
-def checks_cleanup() -> dict:
-    """Cleanup task that performs all periodic cleanup operations.
-
-    This task combines multiple cleanup operations:
-    - Cancel checks superseded by newer checks
-    - Cancel checks on inactive project files
-    - Remove orphaned pending task records
-
-    Returns:
-        Dict with counts of cleanup operations performed.
-    """
-    # Cancel superseded checks
-    superseded_cancelled = _cancel_superseded_checks()
-
-    # Cancel checks on stale files
-    stale_files_result = checks_cleanup_stale_files()
-    stale_files_cancelled = stale_files_result.get("cancelled", 0)
-
-    # Clean up orphaned pending tasks
-    pending_tasks_result = checks_cleanup_stale_pending_tasks()
-    pending_tasks_deleted = pending_tasks_result.get("deleted", 0)
-
-    return {
-        "superseded_cancelled": superseded_cancelled,
-        "stale_files_cancelled": stale_files_cancelled,
-        "pending_tasks_deleted": pending_tasks_deleted,
-    }
+    return {"cancelled": cancelled}
 
 
 @checks_task()

wafer_space/projects/tests/test_tasks.py

@@ -56,8 +56,8 @@
 from wafer_space.projects.tasks import do_starting
 from wafer_space.projects.tasks import download_project_file
 from wafer_space.projects.tasks_checks import _save_output_gds
-from wafer_space.projects.tasks_checks import checks_cleanup
 from wafer_space.projects.tasks_checks import checks_cleanup_stale_pending_tasks
+from wafer_space.projects.tasks_checks import checks_cleanup_superseded
 from wafer_space.projects.tasks_checks import checks_drc_update_requeue
 from wafer_space.projects.tasks_download import _apply_post_download_processing
 from wafer_space.projects.tasks_download import _initialize_hash_calculators
@@ -2608,7 +2608,7 @@ def test_returns_zero_when_no_pending_tasks(self) -> None:
 
 
 class TestCancelSupersededChecks:
-    """Tests for cancel_superseded_checks functionality."""
+    """Tests for the checks_cleanup_superseded task."""
 
     @pytest.mark.django_db
     def test_cancels_older_in_progress_check_when_newer_exists(self) -> None:
@@ -2625,7 +2625,7 @@ def test_cancels_older_in_progress_check_when_newer_exists(self) -> None:
             status=ManufacturabilityCheck.Status.PENDING,
         )
 
-        checks_cleanup()
+        checks_cleanup_superseded()
 
         old_check.refresh_from_db()
         assert old_check.status == ManufacturabilityCheck.Status.CANCELLING
@@ -2640,7 +2640,7 @@ def test_does_not_cancel_if_no_newer_check(self) -> None:
             status=ManufacturabilityCheck.Status.RUNNING,
         )
 
-        checks_cleanup()
+        checks_cleanup_superseded()
 
         check.refresh_from_db()
         assert check.status == ManufacturabilityCheck.Status.RUNNING
@@ -2660,11 +2660,36 @@ def test_does_not_cancel_finished_checks(self) -> None:
             status=ManufacturabilityCheck.Status.PENDING,
         )
 
-        checks_cleanup()
+        checks_cleanup_superseded()
 
         old_check.refresh_from_db()
         assert old_check.status == ManufacturabilityCheck.Status.FINISHED
 
+    @pytest.mark.django_db
+    def test_skips_checks_already_cancelling_without_error_logs(
+        self, caplog: pytest.LogCaptureFixture
+    ) -> None:
+        """Checks already in CANCELLING are skipped, not re-cancelled with errors."""
+        project_file = ProjectFileFactory()
+        old_check = ManufacturabilityCheckFactory(
+            project=project_file.project,
+            project_file=project_file,
+            status=ManufacturabilityCheck.Status.CANCELLING,
+        )
+        ManufacturabilityCheckFactory(
+            project=project_file.project,
+            project_file=project_file,
+            status=ManufacturabilityCheck.Status.PENDING,
+        )
+
+        with caplog.at_level(logging.ERROR, logger="wafer_space.projects.tasks_checks"):
+            result = checks_cleanup_superseded()
+
+        assert result == {"cancelled": 0}
+        assert not caplog.records
+        old_check.refresh_from_db()
+        assert old_check.status == ManufacturabilityCheck.Status.CANCELLING
+
 
 @pytest.mark.django_db
 class TestChecksDrcUpdateRequeue: