Address review comments

Author: vanessavmac

ami/ml/models/pipeline.py

@@ -194,24 +194,23 @@ def process_images(
     task_logger.info(f"Sending {len(images)} images to Pipeline {pipeline}")
     urls = [source_image.public_url() for source_image in images if source_image.public_url()]
 
-    source_images: list[SourceImageRequest] = []
+    source_image_requests: list[SourceImageRequest] = []
     detection_requests: list[DetectionRequest] = []
 
     for source_image, url in zip(images, urls):
         if url:
-            source_images.append(
-                SourceImageRequest(
-                    id=str(source_image.pk),
-                    url=url,
-                )
+            source_image_request = SourceImageRequest(
+                id=str(source_image.pk),
+                url=url,
             )
-            # Only re-process detections created by the pipeline's detector
+            source_image_requests.append(source_image_request)
+            # Re-process all existing detections if they exist
             for detection in source_image.detections.all():
                 bbox = detection.get_bbox()
                 if bbox and detection.detection_algorithm:
                     detection_requests.append(
                         DetectionRequest(
-                            source_image=source_images[-1],
+                            source_image=source_image_request,
                             bbox=bbox,
                             crop_image_url=detection.url(),
                             algorithm=AlgorithmReference(
@@ -231,7 +230,7 @@ def process_images(
 
     request_data = PipelineRequest(
         pipeline=pipeline.slug,
-        source_images=source_images,
+        source_images=source_image_requests,
         config=config,
         detections=detection_requests,
     )
@@ -253,7 +252,8 @@ def process_images(
             pipeline=pipeline.slug,
             total_time=0,
             source_images=[
-                SourceImageResponse(id=source_image.id, url=source_image.url) for source_image in source_images
+                SourceImageResponse(id=source_image_request.id, url=source_image_request.url)
+                for source_image_request in source_image_requests
             ],
             detections=[],
             errors=msg,
@@ -992,7 +992,7 @@ def collect_images(
         )
 
     def choose_processing_service_for_pipeline(
-        self, job_id: int, pipeline_name: str, project_id: int
+        self, job_id: int | None, pipeline_name: str, project_id: int
     ) -> ProcessingService:
         # @TODO use the cached `last_checked_latency` and a max age to avoid checking every time
 

ami/ml/tests.py

@@ -207,10 +207,10 @@ def test_pipeline_reprocessing(self):
             "detection_algorithm",
             "detection_algorithm__category_map",
         )
+        initial_detection_ids = sorted([det.pk for det in detections])
         assert detections.count() > 0
-        initial_num_detections = detections.count()
 
-        # Reprocess the same images
+        # Reprocess the same images using a different pipeline
         pipeline = self.processing_service_instance.pipelines.all().get(slug="constant")
         pipeline_response = pipeline.process_images(self.test_images, project_id=self.project.pk)
         reprocessed_results = save_results(pipeline_response, return_created=True)
@@ -222,7 +222,15 @@ def test_pipeline_reprocessing(self):
             "detection_algorithm",
             "detection_algorithm__category_map",
         )
-        assert initial_num_detections == detections.count(), "Expected no new detections to be created."
+
+        # Check detections were re-processed, and not re-created
+        reprocessed_detection_ids = sorted([det.pk for det in detections])
+        assert initial_detection_ids == reprocessed_detection_ids, (
+            "Expected the same detections to be returned after reprocessing with a different pipeline, "
+            f"but found {initial_detection_ids} != {reprocessed_detection_ids}"
+        )
+
+        # The constant pipeline produces 1 classification per detection
         for detection in detections:
             assert (
                 detection.classifications.count() == 3