From fb3d36225ad678abbd1f9179c96cfb67186b67fe Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Thu, 9 Jul 2026 12:06:54 -0700 Subject: [PATCH 01/16] fix(algorithms): include a project's post-processing algorithms in its algorithm list The project-scoped algorithm list only returned algorithms attached to an enabled pipeline config. Post-processing algorithms such as class masking are created standalone with no pipeline, so they were hidden from the project's algorithm filter even though they produce determinations in the project. The list now also includes any algorithm that produced classifications in the project, so an operator can filter occurrences by a masked result. The detail endpoint was already unscoped; this brings the list in line with it. Co-Authored-By: Claude --- ami/ml/tests.py | 36 ++++++++++++++++++++++++++++++++++-- ami/ml/views.py | 21 ++++++++++++++++----- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/ami/ml/tests.py b/ami/ml/tests.py index 9366eef47..c875d5059 100644 --- a/ami/ml/tests.py +++ b/ami/ml/tests.py @@ -2070,8 +2070,10 @@ def test_deployment_counts_refresh_after_save_results(self): class TestAlgorithmViewSetProjectFilter(APITestCase): """ - The algorithm list endpoint is scoped to algorithms belonging to - pipelines enabled for the active project. + The algorithm list endpoint is scoped to algorithms relevant to the active + project: those belonging to an enabled pipeline, plus any that produced + classifications in the project (e.g. post-processing algorithms like class + masking, which are created standalone with no pipeline). """ def setUp(self): @@ -2136,3 +2138,33 @@ def test_detail_endpoint_unscoped_even_with_project_id(self): response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["name"], "Algo Disabled") + + def _classify_in_project(self, algorithm, project): + """Give ``algorithm`` a terminal classification whose capture is in ``project``.""" + source_image = SourceImage.objects.create(project=project) + detection = Detection.objects.create(source_image=source_image) + return Classification.objects.create( + detection=detection, + algorithm=algorithm, + timestamp=datetime.datetime.now(datetime.timezone.utc), + ) + + def test_lists_post_processing_algorithm_with_classifications_in_project(self): + """A post-processing algorithm has no pipeline but produces determinations in + the project, so the list must include it — otherwise the user cannot filter + occurrences by the masked result.""" + masked_algo = Algorithm.objects.create(name="Class Masked Classifier", version=1) + self._classify_in_project(masked_algo, self.project) + + names = self._list_algorithm_names(project_id=self.project.pk) + self.assertIn("Class Masked Classifier", names) + self.assertIn("Algo Enabled", names, "Enabled-pipeline algorithms still appear") + self.assertNotIn("Algo Disabled", names, "A disabled pipeline with no classifications stays hidden") + + def test_classifications_in_other_project_do_not_leak(self): + """An algorithm whose classifications live in another project must not appear.""" + other_masked_algo = Algorithm.objects.create(name="Other Project Masked", version=1) + self._classify_in_project(other_masked_algo, self.other_project) + + names = self._list_algorithm_names(project_id=self.project.pk) + self.assertNotIn("Other Project Masked", names) diff --git a/ami/ml/views.py b/ami/ml/views.py index 7de502f4a..47a4c33b3 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -1,7 +1,7 @@ import logging from django.db import transaction -from django.db.models import Prefetch +from django.db.models import Prefetch, Q from django.db.models.query import QuerySet from django.utils.text import slugify from drf_spectacular.utils import extend_schema @@ -15,7 +15,7 @@ from ami.base.views import ProjectMixin from ami.main.api.schemas import project_id_doc_param from ami.main.api.views import DefaultViewSet -from ami.main.models import Project, SourceImage +from ami.main.models import Classification, Project, SourceImage from ami.ml.schemas import PipelineRegistrationResponse from .models.algorithm import Algorithm, AlgorithmCategoryMap @@ -56,14 +56,25 @@ class AlgorithmViewSet(DefaultViewSet, ProjectMixin): def get_queryset(self) -> QuerySet["Algorithm"]: qs: QuerySet["Algorithm"] = super().get_queryset() qs = qs.with_category_count() # type: ignore[union-attr] # Custom queryset method - # Only scope list by project. Detail stays unscoped so links from historical + # Only scope the list by project. Detail stays unscoped so links from historical # classifications whose pipeline is no longer enabled still resolve. if getattr(self, "action", None) == "list": project = self.get_active_project() if project: + # An algorithm is relevant to the project if it is configured via an + # enabled pipeline OR it produced classifications in the project. + # Post-processing algorithms (e.g. class masking) are created standalone + # with no pipeline, so the pipeline join alone would hide them even though + # they own determinations the user needs to filter occurrences by. + classified_in_project = Classification.objects.filter(detection__source_image__project=project).values( + "algorithm" + ) qs = qs.filter( - pipelines__project_pipeline_configs__project=project, - pipelines__project_pipeline_configs__enabled=True, + Q( + pipelines__project_pipeline_configs__project=project, + pipelines__project_pipeline_configs__enabled=True, + ) + | Q(pk__in=classified_in_project) ).distinct() return qs From c1a8c532e4345774a0bd052cbc0a0036055ceb6b Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Thu, 9 Jul 2026 12:06:56 -0700 Subject: [PATCH 02/16] fix(post-processing): make class masking idempotent across re-runs Class masking previously relied only on the terminal flag it flips on the source classification to avoid re-scoring it again. If a source became terminal again (after a dedup or re-classification pass), or a partially completed run was retried, the source could be masked a second time and gain a duplicate masked classification. The scope now also excludes sources that already have a masked child for the same masking algorithm, keyed on the applied_to lineage, so a source is masked at most once per masking algorithm. This makes finishing an interrupted run safe. Co-Authored-By: Claude --- ami/ml/post_processing/class_masking.py | 24 ++++++---- .../tests/test_class_masking.py | 47 +++++++++++++++++++ 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/ami/ml/post_processing/class_masking.py b/ami/ml/post_processing/class_masking.py index fc72b836a..160205db4 100644 --- a/ami/ml/post_processing/class_masking.py +++ b/ami/ml/post_processing/class_masking.py @@ -272,7 +272,7 @@ def _get_or_create_masking_algorithm( return algorithm def _scoped_classifications( - self, config: ClassMaskingConfig, source_algorithm: Algorithm + self, config: ClassMaskingConfig, source_algorithm: Algorithm, masking_algorithm: Algorithm ) -> tuple[QuerySet[Classification], str]: """Resolve the terminal classifications to re-score from the config's scope. @@ -284,13 +284,21 @@ def _scoped_classifications( arrays, which measured 208s versus 0.5s on a 55,530-row scope. Broadening either scope to match several collections or a whole project would fan out and need de-duplication again — restrict the columns first. See #1376. + + Sources already re-scored by ``masking_algorithm`` are excluded via the + ``applied_to`` lineage, so a source is masked at most once per masking + algorithm even if it becomes terminal again later. See #1368. """ - base = Classification.objects.filter( - terminal=True, - algorithm=source_algorithm, - scores__isnull=False, - logits__isnull=False, - ).select_related("detection", "detection__occurrence") + base = ( + Classification.objects.filter( + terminal=True, + algorithm=source_algorithm, + scores__isnull=False, + logits__isnull=False, + ) + .exclude(derived_classifications__algorithm=masking_algorithm) + .select_related("detection", "detection__occurrence") + ) if config.occurrence_id is not None: if not Occurrence.objects.filter(pk=config.occurrence_id).exists(): @@ -327,7 +335,7 @@ def run(self) -> None: masking_algorithm = self._get_or_create_masking_algorithm( source_algorithm, taxa_list, reweight=config.reweight ) - classifications, scope_desc = self._scoped_classifications(config, source_algorithm) + classifications, scope_desc = self._scoped_classifications(config, source_algorithm, masking_algorithm) self.logger.info(f"Applying class masking on {scope_desc} using taxa list {taxa_list.pk}") def _on_setup(total: int) -> None: diff --git a/ami/ml/post_processing/tests/test_class_masking.py b/ami/ml/post_processing/tests/test_class_masking.py index 0e9b28efa..2143971ab 100644 --- a/ami/ml/post_processing/tests/test_class_masking.py +++ b/ami/ml/post_processing/tests/test_class_masking.py @@ -253,6 +253,53 @@ def test_task_run_collection_scope_persists_masking_algorithm(self): occ.refresh_from_db() self.assertEqual(occ.determination, self.species_taxa[1], "Occurrence determination follows the masked result") + def test_rerun_does_not_duplicate_masked_classifications(self): + """Re-running the same mask must not create a second masked classification for + a source already re-scored, even if that source became terminal again in between. + + Idempotency is keyed on the ``applied_to`` lineage — a source is masked at most + once per masking algorithm — not on the terminal flag. This makes it safe to + finish a partially completed run (e.g. one the health-check reaper revoked) or + to re-run after a re-classification / dedup pass re-terminalized a source. + """ + logits = [0.5, 3.0, 3.5] # excluded index 2 is top; index 1 is the in-list winner + taxa_list = TaxaList.objects.create(name="Idempotency list") + taxa_list.taxa.set(self.species_taxa[:2]) + + det, _ = self._detection_with_occurrence() + original = self._create_classification_with_logits(det, self.species_taxa[2], _softmax(logits), logits) + + def run(): + ClassMaskingTask( + source_image_collection_id=self.collection.pk, + taxa_list_id=taxa_list.pk, + algorithm_id=self.algorithm.pk, + ).run() + + run() + masking_algo = Algorithm.objects.get( + key=f"{self.algorithm.key}_filtered_by_taxa_list_{taxa_list.pk}_reweighted" + ) + self.assertEqual( + Classification.objects.filter(algorithm=masking_algo, applied_to=original).count(), + 1, + "First run masks the source exactly once", + ) + + # Simulate the source becoming terminal again (a dedup or re-classification pass) + # while its masked child still exists. The terminal filter alone would re-select + # it; the applied_to guard must still skip it. + original.refresh_from_db() + original.terminal = True + original.save(update_fields=["terminal"]) + + run() + self.assertEqual( + Classification.objects.filter(algorithm=masking_algo, applied_to=original).count(), + 1, + "Re-run must not create a duplicate masked classification for an already-masked source", + ) + def test_reweight_modes_get_distinct_masking_algorithms(self): """The reweight mode is part of the masking algorithm's identity. From 0d0a6d134df0a43ca5d65de0d3a4dbba2a1a2077 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Tue, 21 Jul 2026 14:23:28 -0700 Subject: [PATCH 03/16] perf(algorithms): scope the project algorithm list to algorithms that ran List an algorithm for a project when it produced classifications there, and drop the enabled-pipeline branch entirely. A configured but never-run algorithm has no results to filter or inspect, so listing it only adds dead entries to the filter. Removing the branch also removes the OR across the pipeline join and the SELECT DISTINCT it forced. Measured against a local copy of production data (roughly 834k classifications), on one of the larger projects the paginated endpoint drops from 2206 ms to 556 ms, most of it in the pagination COUNT: 1722 ms to 275 ms. Both remaining shapes still sequentially scan the classification and detection tables, because no index reaches a project from a classification. The cost therefore tracks total table size rather than project size, and improving it further needs a denormalised project on Classification or a precomputed per-project list. Co-Authored-By: Claude --- ami/ml/tests.py | 61 ++++++++++++++++++++++++++++++------------------- ami/ml/views.py | 21 +++++++---------- 2 files changed, 45 insertions(+), 37 deletions(-) diff --git a/ami/ml/tests.py b/ami/ml/tests.py index c875d5059..8d1aece81 100644 --- a/ami/ml/tests.py +++ b/ami/ml/tests.py @@ -2070,10 +2070,14 @@ def test_deployment_counts_refresh_after_save_results(self): class TestAlgorithmViewSetProjectFilter(APITestCase): """ - The algorithm list endpoint is scoped to algorithms relevant to the active - project: those belonging to an enabled pipeline, plus any that produced - classifications in the project (e.g. post-processing algorithms like class - masking, which are created standalone with no pipeline). + The algorithm list endpoint is scoped to the algorithms that actually produced + classifications in the active project. + + Pipeline configuration does not grant an algorithm a place in the list. An + algorithm wired to an enabled pipeline but never run has no results to filter or + inspect, and post-processing algorithms such as class masking are created + standalone with no pipeline at all, so pipeline membership is neither necessary + nor sufficient. """ def setUp(self): @@ -2083,16 +2087,18 @@ def setUp(self): self.project = Project.objects.create(name="Algo Project A", create_defaults=False) self.other_project = Project.objects.create(name="Algo Project B", create_defaults=False) - # Project A: one enabled pipeline, one disabled pipeline - self.algo_enabled = Algorithm.objects.create(name="Algo Enabled", version=1) + # Project A: an algorithm that has run, and one configured via a disabled pipeline + self.algo_used = Algorithm.objects.create(name="Algo Used", version=1) self.algo_disabled = Algorithm.objects.create(name="Algo Disabled", version=1) - # Project B: a different pipeline/algorithm + # Project A: enabled pipeline, but the algorithm never produced anything + self.algo_configured_unused = Algorithm.objects.create(name="Algo Configured Unused", version=1) + # Project B: a different algorithm, also used self.algo_other_project = Algorithm.objects.create(name="Algo Other Project", version=1) - # Unrelated algorithm not attached to any pipeline + # Unrelated algorithm not attached to any pipeline and never run self.algo_orphan = Algorithm.objects.create(name="Algo Orphan", version=1) enabled_pipeline = Pipeline.objects.create(name="Enabled Pipeline") - enabled_pipeline.algorithms.add(self.algo_enabled) + enabled_pipeline.algorithms.add(self.algo_used, self.algo_configured_unused) ProjectPipelineConfig.objects.create(project=self.project, pipeline=enabled_pipeline, enabled=True) disabled_pipeline = Pipeline.objects.create(name="Disabled Pipeline") @@ -2103,8 +2109,21 @@ def setUp(self): other_pipeline.algorithms.add(self.algo_other_project) ProjectPipelineConfig.objects.create(project=self.other_project, pipeline=other_pipeline, enabled=True) + self._classify_in_project(self.algo_used, self.project) + self._classify_in_project(self.algo_other_project, self.other_project) + self.client.force_authenticate(user=self.user) + def _classify_in_project(self, algorithm, project): + """Give ``algorithm`` a classification whose capture belongs to ``project``.""" + source_image = SourceImage.objects.create(project=project) + detection = Detection.objects.create(source_image=source_image) + return Classification.objects.create( + detection=detection, + algorithm=algorithm, + timestamp=datetime.datetime.now(datetime.timezone.utc), + ) + def _list_algorithm_names(self, project_id=None): params = {"project_id": project_id} if project_id is not None else {} url = reverse_with_params("api:algorithm-list", params=params) @@ -2112,9 +2131,15 @@ def _list_algorithm_names(self, project_id=None): self.assertEqual(response.status_code, 200) return {row["name"] for row in response.json()["results"]} - def test_lists_only_enabled_pipeline_algorithms_for_project(self): + def test_lists_only_algorithms_used_in_project(self): + """Having run in the project is what puts an algorithm in the list. + + ``Algo Configured Unused`` shares an enabled pipeline with ``Algo Used`` and is + excluded purely because it never classified anything, which pins the positive and + negative sides of the rule against an otherwise identical pair. + """ names = self._list_algorithm_names(project_id=self.project.pk) - self.assertEqual(names, {"Algo Enabled"}) + self.assertEqual(names, {"Algo Used"}) def test_other_project_only_sees_its_own_algorithms(self): names = self._list_algorithm_names(project_id=self.other_project.pk) @@ -2123,7 +2148,7 @@ def test_other_project_only_sees_its_own_algorithms(self): def test_unscoped_request_returns_all_algorithms(self): """Without project_id, current behavior lists all algorithms (unchanged).""" names = self._list_algorithm_names() - self.assertIn("Algo Enabled", names) + self.assertIn("Algo Used", names) self.assertIn("Algo Disabled", names) self.assertIn("Algo Other Project", names) self.assertIn("Algo Orphan", names) @@ -2139,16 +2164,6 @@ def test_detail_endpoint_unscoped_even_with_project_id(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["name"], "Algo Disabled") - def _classify_in_project(self, algorithm, project): - """Give ``algorithm`` a terminal classification whose capture is in ``project``.""" - source_image = SourceImage.objects.create(project=project) - detection = Detection.objects.create(source_image=source_image) - return Classification.objects.create( - detection=detection, - algorithm=algorithm, - timestamp=datetime.datetime.now(datetime.timezone.utc), - ) - def test_lists_post_processing_algorithm_with_classifications_in_project(self): """A post-processing algorithm has no pipeline but produces determinations in the project, so the list must include it — otherwise the user cannot filter @@ -2158,8 +2173,6 @@ def test_lists_post_processing_algorithm_with_classifications_in_project(self): names = self._list_algorithm_names(project_id=self.project.pk) self.assertIn("Class Masked Classifier", names) - self.assertIn("Algo Enabled", names, "Enabled-pipeline algorithms still appear") - self.assertNotIn("Algo Disabled", names, "A disabled pipeline with no classifications stays hidden") def test_classifications_in_other_project_do_not_leak(self): """An algorithm whose classifications live in another project must not appear.""" diff --git a/ami/ml/views.py b/ami/ml/views.py index 47a4c33b3..e70768417 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -1,7 +1,7 @@ import logging from django.db import transaction -from django.db.models import Prefetch, Q +from django.db.models import Prefetch from django.db.models.query import QuerySet from django.utils.text import slugify from drf_spectacular.utils import extend_schema @@ -61,21 +61,16 @@ def get_queryset(self) -> QuerySet["Algorithm"]: if getattr(self, "action", None) == "list": project = self.get_active_project() if project: - # An algorithm is relevant to the project if it is configured via an - # enabled pipeline OR it produced classifications in the project. - # Post-processing algorithms (e.g. class masking) are created standalone - # with no pipeline, so the pipeline join alone would hide them even though - # they own determinations the user needs to filter occurrences by. + # An algorithm belongs to the project if it actually produced classifications + # there. Pipeline configuration is deliberately not consulted: a configured but + # never-run algorithm has no results to filter or inspect, and post-processing + # algorithms (e.g. class masking) are created standalone with no pipeline at all, + # so a pipeline join would both list algorithms with nothing behind them and hide + # ones that own live determinations. classified_in_project = Classification.objects.filter(detection__source_image__project=project).values( "algorithm" ) - qs = qs.filter( - Q( - pipelines__project_pipeline_configs__project=project, - pipelines__project_pipeline_configs__enabled=True, - ) - | Q(pk__in=classified_in_project) - ).distinct() + qs = qs.filter(pk__in=classified_in_project) return qs @extend_schema(parameters=[project_id_doc_param]) From a26b4559f002ca411ab43ca00e9474d6bda37908 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Tue, 21 Jul 2026 14:26:17 -0700 Subject: [PATCH 04/16] docs(post-processing): state what the masking idempotency guard actually covers The docstring credited the lineage guard with resuming interrupted runs. It does not: demoting a source and writing its masked child share one transaction, so a resumed run already skips completed sources on the terminal flag alone. The guard covers only the case the flag cannot, a source restored to terminal by a later dedup or re-classification pass. Also record that a taxa list is identified by primary key rather than by contents, so editing a list in place and re-running the same mask leaves already-masked sources scored against the earlier membership. Co-Authored-By: Claude --- ami/ml/post_processing/class_masking.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ami/ml/post_processing/class_masking.py b/ami/ml/post_processing/class_masking.py index 160205db4..282ed8583 100644 --- a/ami/ml/post_processing/class_masking.py +++ b/ami/ml/post_processing/class_masking.py @@ -287,7 +287,9 @@ def _scoped_classifications( Sources already re-scored by ``masking_algorithm`` are excluded via the ``applied_to`` lineage, so a source is masked at most once per masking - algorithm even if it becomes terminal again later. See #1368. + algorithm even if it becomes terminal again later. A taxa list is identified + by primary key, not contents — re-masking against an edited list needs a new + list. See #1368. """ base = ( Classification.objects.filter( From ddbf5a19eb441aef047f49e2c29650687e3b0a51 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Tue, 21 Jul 2026 14:33:18 -0700 Subject: [PATCH 05/16] refactor(algorithms): deduplicate the project algorithm subquery The subquery returned one row per classification, so a project with a million classifications fed a million rows into the IN clause to identify at most a few dozen algorithms. Selecting distinct algorithm ids says what the query means. Measured effect is small and close to run-to-run noise (median 610 ms -> 567 ms over four runs against a local copy of production data); the change is for clarity. Co-Authored-By: Claude --- ami/ml/views.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ami/ml/views.py b/ami/ml/views.py index e70768417..f1dedb72d 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -67,8 +67,10 @@ def get_queryset(self) -> QuerySet["Algorithm"]: # algorithms (e.g. class masking) are created standalone with no pipeline at all, # so a pipeline join would both list algorithms with nothing behind them and hide # ones that own live determinations. - classified_in_project = Classification.objects.filter(detection__source_image__project=project).values( - "algorithm" + classified_in_project = ( + Classification.objects.filter(detection__source_image__project=project) + .values_list("algorithm", flat=True) + .distinct() ) qs = qs.filter(pk__in=classified_in_project) return qs From 1d6e1a45b787796624dca9bdc9ceafb110d775e9 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Tue, 21 Jul 2026 14:42:33 -0700 Subject: [PATCH 06/16] fix(algorithms): keep pipeline-configured algorithms in the project list Scoping the list purely by classification authorship dropped every detector. Localizers set Detection.detection_algorithm and never write a Classification, so the detector behind every detection in a project disappeared from the project's algorithm list, which renders a task_type column precisely to show detectors next to classifiers. Restore the enabled-pipeline join as the primary rule and admit post-processing algorithms through a second query, restricted to algorithms with no pipeline so the classification lookup stays off the full table. Collect the two separately rather than OR-ing them: an OR across the pipeline join forces a SELECT DISTINCT whose COUNT costs more than both queries together. Measured against a local copy of production data, on the largest project the list returns in 46 ms, versus 8 ms for the pipeline-only rule on main and 595 ms for the authorship-only rule this replaces. The result is a superset of main: nothing main listed is lost, and the post-processing algorithms are added. An algorithm attached to no pipeline that only ever produced detections is still absent, as it is on main. Catching it would mean scanning the detection table. Co-Authored-By: Claude --- ami/ml/tests.py | 42 ++++++++++++++++++++++++++++-------------- ami/ml/views.py | 37 ++++++++++++++++++++++++------------- 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/ami/ml/tests.py b/ami/ml/tests.py index 8d1aece81..d49a8d36a 100644 --- a/ami/ml/tests.py +++ b/ami/ml/tests.py @@ -2070,14 +2070,13 @@ def test_deployment_counts_refresh_after_save_results(self): class TestAlgorithmViewSetProjectFilter(APITestCase): """ - The algorithm list endpoint is scoped to the algorithms that actually produced - classifications in the active project. - - Pipeline configuration does not grant an algorithm a place in the list. An - algorithm wired to an enabled pipeline but never run has no results to filter or - inspect, and post-processing algorithms such as class masking are created - standalone with no pipeline at all, so pipeline membership is neither necessary - nor sufficient. + The algorithm list endpoint is scoped to the algorithms relevant to the active + project, which they reach two ways. + + An enabled pipeline configures most of them. That covers detectors, which never + author a Classification and so could not be found by their results. Post-processing + algorithms such as class masking have no pipeline at all, so they are admitted by + having classified something in the project. """ def setUp(self): @@ -2131,15 +2130,30 @@ def _list_algorithm_names(self, project_id=None): self.assertEqual(response.status_code, 200) return {row["name"] for row in response.json()["results"]} - def test_lists_only_algorithms_used_in_project(self): - """Having run in the project is what puts an algorithm in the list. + def test_lists_enabled_pipeline_algorithms_for_project(self): + """An enabled pipeline admits its algorithms whether or not they have run. - ``Algo Configured Unused`` shares an enabled pipeline with ``Algo Used`` and is - excluded purely because it never classified anything, which pins the positive and - negative sides of the rule against an otherwise identical pair. + Running is not required because detectors never author a Classification, so a + results-only rule would drop the detector behind every detection in the project. """ names = self._list_algorithm_names(project_id=self.project.pk) - self.assertEqual(names, {"Algo Used"}) + self.assertEqual(names, {"Algo Used", "Algo Configured Unused"}) + + def test_detector_that_ran_is_listed_although_it_never_classified(self): + """Detectors set ``Detection.detection_algorithm`` and never write a + Classification, so they are reachable only through their pipeline. This pins the + regression where scoping the list purely by classification authorship dropped + every localizer from the project's algorithm list.""" + detector = Algorithm.objects.create(name="Algo Detector", version=1, task_type="localization") + pipeline = Pipeline.objects.get(name="Enabled Pipeline") + pipeline.algorithms.add(detector) + + source_image = SourceImage.objects.create(project=self.project) + Detection.objects.create(source_image=source_image, detection_algorithm=detector) + self.assertFalse(Classification.objects.filter(algorithm=detector).exists()) + + names = self._list_algorithm_names(project_id=self.project.pk) + self.assertIn("Algo Detector", names) def test_other_project_only_sees_its_own_algorithms(self): names = self._list_algorithm_names(project_id=self.other_project.pk) diff --git a/ami/ml/views.py b/ami/ml/views.py index f1dedb72d..d41213ad0 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -15,7 +15,7 @@ from ami.base.views import ProjectMixin from ami.main.api.schemas import project_id_doc_param from ami.main.api.views import DefaultViewSet -from ami.main.models import Classification, Project, SourceImage +from ami.main.models import Project, SourceImage from ami.ml.schemas import PipelineRegistrationResponse from .models.algorithm import Algorithm, AlgorithmCategoryMap @@ -61,18 +61,29 @@ def get_queryset(self) -> QuerySet["Algorithm"]: if getattr(self, "action", None) == "list": project = self.get_active_project() if project: - # An algorithm belongs to the project if it actually produced classifications - # there. Pipeline configuration is deliberately not consulted: a configured but - # never-run algorithm has no results to filter or inspect, and post-processing - # algorithms (e.g. class masking) are created standalone with no pipeline at all, - # so a pipeline join would both list algorithms with nothing behind them and hide - # ones that own live determinations. - classified_in_project = ( - Classification.objects.filter(detection__source_image__project=project) - .values_list("algorithm", flat=True) - .distinct() - ) - qs = qs.filter(pk__in=classified_in_project) + # Algorithms reach the list two ways. An enabled pipeline configures most of + # them, and that join alone covers detectors, which never author a + # Classification and so cannot be found by their results at all. + # + # Post-processing algorithms (e.g. class masking) are created standalone with + # no pipeline, so they are found by their classifications instead. That lookup + # is restricted to unpipelined algorithms first, which is what keeps it cheap: + # Classification has no project column and reaches one only through + # detection -> source_image, so an unrestricted version scans the whole table. + # + # The two are collected separately rather than OR'd into one filter. An OR + # across the pipeline join forces a SELECT DISTINCT whose COUNT costs more + # than both queries together. + configured_for_project = Algorithm.objects.filter( + pipelines__project_pipeline_configs__project=project, + pipelines__project_pipeline_configs__enabled=True, + ).values_list("pk", flat=True) + post_processing_used_in_project = Algorithm.objects.filter( + pipelines__isnull=True, + classifications__detection__source_image__project=project, + ).values_list("pk", flat=True) + # Materialising is safe here: algorithms number in the dozens platform-wide. + qs = qs.filter(pk__in=set(configured_for_project) | set(post_processing_used_in_project)) return qs @extend_schema(parameters=[project_id_doc_param]) From 76755e4bd076ec323e2f77129aeb7bf266b66d78 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Tue, 21 Jul 2026 14:48:30 -0700 Subject: [PATCH 07/16] fix(algorithms): sort the algorithm id list so the query is cache-stable The ids came from a set, whose iteration order is not a documented guarantee. cachalot keys its cache on the generated query string, so a varying order of the same ids would produce cache misses for identical results. Also replaces the bound claimed for materialising the ids. "Dozens platform-wide" was true when written but class masking creates an algorithm per source algorithm, taxa list and reweight mode, so the count grows with use; the comment now says where the growth comes from and when to switch to a subquery. Co-Authored-By: Claude --- ami/ml/views.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ami/ml/views.py b/ami/ml/views.py index d41213ad0..b8df25088 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -82,8 +82,16 @@ def get_queryset(self) -> QuerySet["Algorithm"]: pipelines__isnull=True, classifications__detection__source_image__project=project, ).values_list("pk", flat=True) - # Materialising is safe here: algorithms number in the dozens platform-wide. - qs = qs.filter(pk__in=set(configured_for_project) | set(post_processing_used_in_project)) + # Sorted so the generated SQL is stable for a given result: cachalot keys its + # cache on the query string, and an unordered set would vary it. + # + # Materialising the ids is fine at this cardinality. Algorithms are created per + # model rather than per run, and the one path that adds them over time is class + # masking, which creates a single algorithm per source algorithm, taxa list and + # reweight mode. Should that ever reach the thousands, this wants to become a + # subquery rather than an IN list. + relevant_ids = set(configured_for_project) | set(post_processing_used_in_project) + qs = qs.filter(pk__in=sorted(relevant_ids)) return qs @extend_schema(parameters=[project_id_doc_param]) From ff0977b33ecfb7cb857ec3480681dc3e9ccd622d Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Tue, 21 Jul 2026 14:56:04 -0700 Subject: [PATCH 08/16] fix(algorithms): deduplicate the post-processing lookup in the database MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The join from an algorithm to a project through its classifications emits one row per matching classification, so the lookup shipped one id per masked classification in the project — hundreds of thousands after a real masking run — to identify a handful of algorithms. Adding DISTINCT collapses that to one id per algorithm in the database. Measured effect is small on current data (the projects checked hold hundreds to low thousands of masked classifications), but the row count was linear in classification volume, which is exactly what this feature produces at scale. Also document that an algorithm on a pipeline not enabled for the project stays absent even when it has determinations there. Widening the second query to cover that case measured roughly five times the runtime, so it is left as a known gap matching the pipeline-join behaviour rather than folded into this change. Co-Authored-By: Claude --- ami/ml/tests.py | 22 ++++++++++++++++++++++ ami/ml/views.py | 24 ++++++++++++++++++------ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/ami/ml/tests.py b/ami/ml/tests.py index d49a8d36a..08a7fe182 100644 --- a/ami/ml/tests.py +++ b/ami/ml/tests.py @@ -2188,6 +2188,28 @@ def test_lists_post_processing_algorithm_with_classifications_in_project(self): names = self._list_algorithm_names(project_id=self.project.pk) self.assertIn("Class Masked Classifier", names) + def test_post_processing_lookup_is_deduplicated_in_the_database(self): + """The post-processing join emits one row per matching classification, so it must + deduplicate in SQL rather than in Python. + + Without that, the rows fetched grow with a project's masked classification count — + hundreds of thousands on a real masking run — to identify a handful of algorithms. + The listed names are correct either way, so this asserts the row count of the + underlying lookup rather than the endpoint's output. + """ + masked_algo = Algorithm.objects.create(name="Chatty Masked Classifier", version=1) + for _ in range(5): + self._classify_in_project(masked_algo, self.project) + + lookup = Algorithm.objects.filter( + pipelines__isnull=True, + classifications__detection__source_image__project=self.project, + ).values_list("pk", flat=True) + + self.assertEqual(len(list(lookup)), 5, "Undeduplicated join emits one row per classification") + self.assertEqual(len(list(lookup.distinct())), 1, "Deduplicating collapses them to the one algorithm") + self.assertIn("Chatty Masked Classifier", self._list_algorithm_names(project_id=self.project.pk)) + def test_classifications_in_other_project_do_not_leak(self): """An algorithm whose classifications live in another project must not appear.""" other_masked_algo = Algorithm.objects.create(name="Other Project Masked", version=1) diff --git a/ami/ml/views.py b/ami/ml/views.py index b8df25088..44f7896b7 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -66,11 +66,16 @@ def get_queryset(self) -> QuerySet["Algorithm"]: # Classification and so cannot be found by their results at all. # # Post-processing algorithms (e.g. class masking) are created standalone with - # no pipeline, so they are found by their classifications instead. That lookup - # is restricted to unpipelined algorithms first, which is what keeps it cheap: + # no pipeline, so they are found by their classifications instead. Restricting + # that lookup to unpipelined algorithms bounds which rows are scanned: # Classification has no project column and reaches one only through # detection -> source_image, so an unrestricted version scans the whole table. # + # An algorithm attached to a pipeline that is not enabled for this project is + # therefore absent even when it owns determinations here. That matches the + # behaviour on the pipeline join alone, and widening the second query to cover + # it costs roughly five times the runtime. + # # The two are collected separately rather than OR'd into one filter. An OR # across the pipeline join forces a SELECT DISTINCT whose COUNT costs more # than both queries together. @@ -78,10 +83,17 @@ def get_queryset(self) -> QuerySet["Algorithm"]: pipelines__project_pipeline_configs__project=project, pipelines__project_pipeline_configs__enabled=True, ).values_list("pk", flat=True) - post_processing_used_in_project = Algorithm.objects.filter( - pipelines__isnull=True, - classifications__detection__source_image__project=project, - ).values_list("pk", flat=True) + # Deduplicated in the database: the join emits one row per matching + # classification, so without this the result grows with a project's masked + # classification count rather than with its handful of algorithms. + post_processing_used_in_project = ( + Algorithm.objects.filter( + pipelines__isnull=True, + classifications__detection__source_image__project=project, + ) + .values_list("pk", flat=True) + .distinct() + ) # Sorted so the generated SQL is stable for a given result: cachalot keys its # cache on the query string, and an unordered set would vary it. # From 3aade753be29a54c6dff8607dbf3c24f916048f5 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Tue, 21 Jul 2026 15:00:47 -0700 Subject: [PATCH 09/16] perf(algorithms): look up post-processing algorithms via an indexed IN list The single-join form (pipelines__isnull=True combined with the classification relation) hides the candidate algorithm ids behind an anti-join at plan time, so Postgres sequentially scans the whole classification table to answer it. Its cost therefore grows with the platform's total classification count, and it is scanned cold on every cachalot invalidation, which happens on every classification write. Split it into two steps. Collect the unpipelined algorithm ids first, as an explicit list, then filter classifications on algorithm_id IN (...). The planner answers that from the algorithm_id index and touches only the rows for those algorithms. EXPLAIN on a local copy of production data confirms the plan change: a parallel sequential scan of main_classification (about 48 ms) becomes an index scan (about 6 ms), and the whole endpoint drops from roughly 46 ms to 12 ms. The first query is also cheap to cache, changing only when algorithms or pipeline links change rather than on every classification. Clear Classification's default ordering with .order_by() before .distinct(), otherwise the ordering columns widen the DISTINCT back to one row per classification. The dedup test now covers this. Co-Authored-By: Claude --- ami/ml/tests.py | 20 ++++++++++------- ami/ml/views.py | 58 ++++++++++++++++++++++++------------------------- 2 files changed, 41 insertions(+), 37 deletions(-) diff --git a/ami/ml/tests.py b/ami/ml/tests.py index 08a7fe182..66cd7788e 100644 --- a/ami/ml/tests.py +++ b/ami/ml/tests.py @@ -2189,25 +2189,29 @@ def test_lists_post_processing_algorithm_with_classifications_in_project(self): self.assertIn("Class Masked Classifier", names) def test_post_processing_lookup_is_deduplicated_in_the_database(self): - """The post-processing join emits one row per matching classification, so it must + """The post-processing lookup matches one row per classification, so it must deduplicate in SQL rather than in Python. Without that, the rows fetched grow with a project's masked classification count — hundreds of thousands on a real masking run — to identify a handful of algorithms. The listed names are correct either way, so this asserts the row count of the - underlying lookup rather than the endpoint's output. + underlying lookup rather than the endpoint's output. The ``.order_by()`` matters: + Classification's default ordering would otherwise widen the DISTINCT back to one + row per classification. """ masked_algo = Algorithm.objects.create(name="Chatty Masked Classifier", version=1) for _ in range(5): self._classify_in_project(masked_algo, self.project) - lookup = Algorithm.objects.filter( - pipelines__isnull=True, - classifications__detection__source_image__project=self.project, - ).values_list("pk", flat=True) + lookup = Classification.objects.filter( + algorithm_id=masked_algo.pk, + detection__source_image__project=self.project, + ).values_list("algorithm_id", flat=True) - self.assertEqual(len(list(lookup)), 5, "Undeduplicated join emits one row per classification") - self.assertEqual(len(list(lookup.distinct())), 1, "Deduplicating collapses them to the one algorithm") + self.assertEqual(len(list(lookup)), 5, "The lookup matches one row per classification") + self.assertEqual( + len(list(lookup.order_by().distinct())), 1, "Deduplicating collapses them to the one algorithm" + ) self.assertIn("Chatty Masked Classifier", self._list_algorithm_names(project_id=self.project.pk)) def test_classifications_in_other_project_do_not_leak(self): diff --git a/ami/ml/views.py b/ami/ml/views.py index 44f7896b7..9236b53be 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -15,7 +15,7 @@ from ami.base.views import ProjectMixin from ami.main.api.schemas import project_id_doc_param from ami.main.api.views import DefaultViewSet -from ami.main.models import Project, SourceImage +from ami.main.models import Classification, Project, SourceImage from ami.ml.schemas import PipelineRegistrationResponse from .models.algorithm import Algorithm, AlgorithmCategoryMap @@ -64,44 +64,44 @@ def get_queryset(self) -> QuerySet["Algorithm"]: # Algorithms reach the list two ways. An enabled pipeline configures most of # them, and that join alone covers detectors, which never author a # Classification and so cannot be found by their results at all. - # - # Post-processing algorithms (e.g. class masking) are created standalone with - # no pipeline, so they are found by their classifications instead. Restricting - # that lookup to unpipelined algorithms bounds which rows are scanned: - # Classification has no project column and reaches one only through - # detection -> source_image, so an unrestricted version scans the whole table. - # - # An algorithm attached to a pipeline that is not enabled for this project is - # therefore absent even when it owns determinations here. That matches the - # behaviour on the pipeline join alone, and widening the second query to cover - # it costs roughly five times the runtime. - # - # The two are collected separately rather than OR'd into one filter. An OR - # across the pipeline join forces a SELECT DISTINCT whose COUNT costs more - # than both queries together. configured_for_project = Algorithm.objects.filter( pipelines__project_pipeline_configs__project=project, pipelines__project_pipeline_configs__enabled=True, ).values_list("pk", flat=True) - # Deduplicated in the database: the join emits one row per matching - # classification, so without this the result grows with a project's masked - # classification count rather than with its handful of algorithms. + + # Post-processing algorithms (e.g. class masking) are created standalone with + # no pipeline, so they are found by their classifications instead. This is done + # in two steps on purpose. Collecting the unpipelined algorithm ids first, as an + # explicit list, lets the classification lookup filter on `algorithm_id IN (...)`, + # which the planner answers from the algorithm_id index. Expressing the same thing + # as a single join (`pipelines__isnull=True, classifications__...`) hides those ids + # behind an anti-join at plan time, so the planner sequentially scans the whole + # classification table instead — a cost that grows with the platform's total + # classification count rather than with this project. The first query is also + # cheap to cache: it only changes when algorithms or pipeline links change, not on + # every classification write. + # + # An algorithm attached to a pipeline that is not enabled for this project is + # absent even when it owns determinations here. That matches the behaviour on the + # pipeline join alone; widening the second lookup to cover it costs several times + # the runtime and is left as a follow-up. + unpipelined_algorithm_ids = list( + Algorithm.objects.filter(pipelines__isnull=True).values_list("pk", flat=True) + ) + # `.order_by()` clears Classification's default ordering before `.distinct()`. + # Without it the ordering columns join the SELECT and widen the DISTINCT, so it + # would return one row per classification rather than one per algorithm. post_processing_used_in_project = ( - Algorithm.objects.filter( - pipelines__isnull=True, - classifications__detection__source_image__project=project, + Classification.objects.filter( + algorithm_id__in=unpipelined_algorithm_ids, + detection__source_image__project=project, ) - .values_list("pk", flat=True) + .order_by() + .values_list("algorithm_id", flat=True) .distinct() ) # Sorted so the generated SQL is stable for a given result: cachalot keys its # cache on the query string, and an unordered set would vary it. - # - # Materialising the ids is fine at this cardinality. Algorithms are created per - # model rather than per run, and the one path that adds them over time is class - # masking, which creates a single algorithm per source algorithm, taxa list and - # reweight mode. Should that ever reach the thousands, this wants to become a - # subquery rather than an IN list. relevant_ids = set(configured_for_project) | set(post_processing_used_in_project) qs = qs.filter(pk__in=sorted(relevant_ids)) return qs From b0263547a69afbf8e92a117e32b1e1a9f2d462c6 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Tue, 21 Jul 2026 15:06:32 -0700 Subject: [PATCH 10/16] fix(algorithms): sort the inner algorithm id list for a stable cache key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unpipelined algorithm ids are rendered verbatim into the classification lookup's IN clause. Algorithm orders by (name, version), so the ids came out in name order, and identical membership could produce a different id order — and so a different SQL string and cachalot cache key — across requests. Sorting makes the inner query cache-stable, matching the outer id list, which was already sorted for the same reason. Co-Authored-By: Claude --- ami/ml/views.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ami/ml/views.py b/ami/ml/views.py index 9236b53be..87ff46933 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -85,7 +85,11 @@ def get_queryset(self) -> QuerySet["Algorithm"]: # absent even when it owns determinations here. That matches the behaviour on the # pipeline join alone; widening the second lookup to cover it costs several times # the runtime and is left as a follow-up. - unpipelined_algorithm_ids = list( + # Sorted for the same reason as the outer id list below: this list is rendered + # verbatim into the IN clause, and Algorithm's (name, version) ordering would + # otherwise vary the id order — and so the SQL string and its cachalot key — + # for identical membership. + unpipelined_algorithm_ids = sorted( Algorithm.objects.filter(pipelines__isnull=True).values_list("pk", flat=True) ) # `.order_by()` clears Classification's default ordering before `.distinct()`. From a159b4f9b8ab79e6e12cb07972f7a7ab1d0f4760 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Tue, 21 Jul 2026 16:05:18 -0700 Subject: [PATCH 11/16] refactor(algorithms): scope project list to algorithms that actually ran MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the pipeline-config + unpipelined-classification hybrid in AlgorithmViewSet with a single reusable queryset method, Algorithm.objects.used_in_project(). The list now contains exactly the algorithms that produced results in the project — detectors via their detections, classifiers and post-processing algorithms via their classifications — regardless of whether the owning pipeline is still enabled. A superseded model version stays listed as long as its determinations survive; an algorithm configured on an enabled pipeline that never ran no longer appears. Add matching OccurrenceQuerySet methods, detected_or_classified_by() and not_detected_or_classified_by(), and route OccurrenceAlgorithmFilter through them. The filter previously joined detections__classifications, which returned one row per matching classification and inflated the paginator's COUNT, and it matched only classifiers — a detector produced no match at all. The EXISTS form counts each occurrence once and matches both roles, so every algorithm the list shows can now filter occurrences. Document in AGENTS.md the two query-work rules this change was built on: run EXPLAIN (ANALYZE) before claiming a query's plan, and measure on the largest project (surveying several sizes) rather than a single small one. Cost note: used_in_project scans classification and detection because neither table has a project column, so it runs ~0.1-0.6s cold and scales with total table size until a denormalized project column exists — the honest cost of showing exactly what ran. Co-Authored-By: Claude --- .agents/AGENTS.md | 2 + ami/main/api/views.py | 14 +++-- ami/main/models.py | 27 ++++++++++ ami/main/tests.py | 102 +++++++++++++++++++++++++++++++++++++ ami/ml/models/algorithm.py | 40 +++++++++++++++ ami/ml/tests.py | 77 +++++++++++++++++----------- ami/ml/views.py | 55 +++----------------- 7 files changed, 233 insertions(+), 84 deletions(-) diff --git a/.agents/AGENTS.md b/.agents/AGENTS.md index 1499fc174..fc240258b 100644 --- a/.agents/AGENTS.md +++ b/.agents/AGENTS.md @@ -19,6 +19,8 @@ Every call to the AI model API incurs a cost and requires electricity. Be smart - Focus on optimizing cold queries first before adding caching - When ordering by annotated fields, pagination COUNT queries include those annotations - use `.values('pk')` to strip them - For large tables (>10k rows), consider fuzzy counting using PostgreSQL's pg_class.reltuples +- **Run `EXPLAIN (ANALYZE)` before claiming anything about a query plan.** Do not describe a query as "uses the index", "stays off the table", or "only touches N rows" from reading the ORM expression — verify it. A filter that looks bounded can still trigger a `Seq Scan`: an anti-join such as `pipelines__isnull=True` hides the candidate ids at plan time, so Postgres scans the whole table despite an index being available. Materializing the ids first and filtering on `field__in=[literal ids]` is what lets the planner use the index. +- **Measure on the largest project in the local database, and survey several project sizes.** The local DB is a copy of production — use it. A small or empty project routinely hides the defect. If a query's cost does not move with project size, it is bound by total table size and will degrade as the platform grows regardless of tenant. Wrap timing loops in `cachalot_disabled()` and rebuild the queryset each iteration, or a reused queryset serves from `_result_cache` and reports a fake ~0 ms. **Git Commit Guidelines:** - Do NOT include "Generated with Claude Code" in commit messages diff --git a/ami/main/api/views.py b/ami/main/api/views.py index e245044a8..b499002b9 100644 --- a/ami/main/api/views.py +++ b/ami/main/api/views.py @@ -1222,11 +1222,15 @@ def filter_queryset(self, request, queryset, view): class OccurrenceAlgorithmFilter(filters.BaseFilterBackend): """ - Filter occurrences by the detection algorithm that detected them. + Filter occurrences by any algorithm that produced a result on them. - Accepts a list of algorithm ids to filter by or exclude by. + Matches an occurrence when one of its detections was made by the given + algorithm (detectors) or one of its classifications came from it + (classifiers and post-processing algorithms), so every algorithm listed + by ``Algorithm.objects.used_in_project()`` can be filtered here. - This filter can be both inclusive and exclusive. + Accepts a list of algorithm ids to filter by (``algorithm``) or exclude + by (``not_algorithm``). Both are supported and may be combined. """ query_param = "algorithm" @@ -1237,9 +1241,9 @@ def filter_queryset(self, request, queryset, view): algorithm_ids_exclusive = request.query_params.getlist(self.query_param_exclusive) if algorithm_ids: - queryset = queryset.filter(detections__classifications__algorithm__in=algorithm_ids) + queryset = queryset.detected_or_classified_by(algorithm_ids) if algorithm_ids_exclusive: - queryset = queryset.exclude(detections__classifications__algorithm__in=algorithm_ids_exclusive) + queryset = queryset.not_detected_or_classified_by(algorithm_ids_exclusive) return queryset diff --git a/ami/main/models.py b/ami/main/models.py index 3662b4107..017b51263 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -3353,6 +3353,33 @@ def valid(self): def with_detections_count(self): return self.annotate(detections_count=models.Count("detections", distinct=True)) + def _machine_results_by(self, algorithm_ids) -> Exists: + """Subquery matching occurrences with any result from the given algorithms — + a detection made by one (detectors) or a classification from one (classifiers + and post-processing algorithms).""" + return Exists( + Detection.objects.filter(occurrence_id=OuterRef("pk")).filter( + models.Q(detection_algorithm__in=algorithm_ids) + | models.Q(classifications__algorithm__in=algorithm_ids) + ) + ) + + def detected_or_classified_by(self, algorithm_ids) -> "OccurrenceQuerySet": + """Occurrences with at least one result from the given algorithms. + + Matches detectors through Detection.detection_algorithm and classifiers or + post-processing algorithms through their classifications, so every algorithm + listed by ``Algorithm.objects.used_in_project()`` can match here. The EXISTS + form returns each occurrence once; a join through + ``detections__classifications`` returns one row per matching result, which + inflates pagination counts and duplicates rows across pages. + """ + return self.filter(self._machine_results_by(algorithm_ids)) + + def not_detected_or_classified_by(self, algorithm_ids) -> "OccurrenceQuerySet": + """Occurrences with no result from any of the given algorithms.""" + return self.exclude(self._machine_results_by(algorithm_ids)) + def with_timestamps(self): """ These are timestamps used for filtering and ordering in the UI. diff --git a/ami/main/tests.py b/ami/main/tests.py index a0d0253a1..fb8bb771f 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -6510,6 +6510,108 @@ def test_valid_returns_only_real_with_determination(self): self.assertNotIn(no_determination.pk, valid_pks) +class TestOccurrenceAlgorithmFilterQuerySet(TestCase): + """ + Covers OccurrenceQuerySet.detected_or_classified_by / not_detected_or_classified_by, + which back the ?algorithm= and ?not_algorithm= occurrence filters (PR #1368). + + The filter must match an algorithm by either role it can play: the detector that + made a detection, or the classifier / post-processing algorithm that authored a + classification. It must also count each occurrence once — the previous join form + (``detections__classifications__algorithm__in``) returned one row per matching + classification, inflating the paginator's COUNT and duplicating occurrences across + pages. + """ + + def setUp(self): + from ami.main.models import Taxon + from ami.ml.models.algorithm import Algorithm + + self.project = Project.objects.create(name="Occurrence Algo Filter Project") + self.deployment = Deployment.objects.create(project=self.project, name="dep") + self.event = Event.objects.create( + project=self.project, + deployment=self.deployment, + group_by="2024-01-01", + start=datetime.datetime(2024, 1, 1, 0, 0), + ) + self.source_image = SourceImage.objects.create( + deployment=self.deployment, + project=self.project, + event=self.event, + path="occ-algo-filter.jpg", + ) + self.taxon = Taxon.objects.create(name="Occurrence Algo Filter Taxon") + + self.detector = Algorithm.objects.create(name="Filter Detector", version=1, task_type="localization") + self.classifier = Algorithm.objects.create(name="Filter Classifier", version=1, task_type="classification") + self.other = Algorithm.objects.create(name="Filter Other", version=1, task_type="classification") + + # Detected by `detector`, classified once by `classifier`. + self.occ_classified = self._make_occurrence(detector=self.detector, classifiers=[self.classifier]) + # Same pair, but classified three times — the double-count trap. + self.occ_multi = self._make_occurrence( + detector=self.detector, classifiers=[self.classifier, self.classifier, self.classifier] + ) + # A different algorithm entirely, to prove filters are exclusive. + self.occ_other = self._make_occurrence(detector=self.other, classifiers=[self.other]) + + def _make_occurrence(self, detector, classifiers) -> Occurrence: + occ = Occurrence.objects.create( + project=self.project, + event=self.event, + deployment=self.deployment, + determination=self.taxon, + ) + detection = Detection.objects.create( + source_image=self.source_image, + bbox=[0.0, 0.0, 1.0, 1.0], + detection_algorithm=detector, + occurrence=occ, + ) + for classifier in classifiers: + detection.classifications.create( + taxon=self.taxon, + algorithm=classifier, + score=0.9, + timestamp=datetime.datetime.now(), + ) + return occ + + def _pks(self, queryset): + return set(queryset.values_list("pk", flat=True)) + + def test_filter_by_classifier_matches_its_occurrences(self): + matched = Occurrence.objects.filter(project=self.project).detected_or_classified_by([self.classifier.pk]) + self.assertEqual(self._pks(matched), {self.occ_classified.pk, self.occ_multi.pk}) + + def test_filter_by_detector_matches_occurrences_it_detected(self): + """A detector authors no Classification, so the old classification-join form + returned nothing for it. Matching through Detection.detection_algorithm is what + lets the user filter by a localizer at all.""" + matched = Occurrence.objects.filter(project=self.project).detected_or_classified_by([self.detector.pk]) + self.assertEqual(self._pks(matched), {self.occ_classified.pk, self.occ_multi.pk}) + + def test_count_not_inflated_by_multiple_classifications(self): + """The occurrence with three classifications by the same algorithm must count + once. The paginator calls this same COUNT, so an inflated value would report + four occurrences where there are two and repeat rows across pages.""" + matched = Occurrence.objects.filter(project=self.project).detected_or_classified_by([self.classifier.pk]) + self.assertEqual(matched.count(), 2) + + def test_exclude_removes_matching_occurrences(self): + remaining = Occurrence.objects.filter(project=self.project).not_detected_or_classified_by([self.other.pk]) + self.assertEqual(self._pks(remaining), {self.occ_classified.pk, self.occ_multi.pk}) + + def test_exclude_is_the_complement_of_include(self): + base = Occurrence.objects.filter(project=self.project) + ids = [self.classifier.pk] + included = self._pks(base.detected_or_classified_by(ids)) + excluded = self._pks(base.not_detected_or_classified_by(ids)) + self.assertEqual(included | excluded, self._pks(base)) + self.assertEqual(included & excluded, set()) + + class TestCleanupNullOnlyOccurrencesCommand(TestCase): """ Covers ami/main/management/commands/cleanup_null_only_occurrences.py. diff --git a/ami/ml/models/algorithm.py b/ami/ml/models/algorithm.py index 0e9df4609..605f90861 100644 --- a/ami/ml/models/algorithm.py +++ b/ami/ml/models/algorithm.py @@ -157,6 +157,46 @@ def with_category_count(self): """ return self.annotate(category_count=ArrayLength("category_map__labels")) + def used_in_project(self, project) -> AlgorithmQuerySet: + """Algorithms that produced results in the project, whether or not their + pipeline is still enabled there. + + "Used" means owning actual output rows: classifiers and post-processing + algorithms are found through their classifications, detectors through their + detections (detectors never author a Classification). Superseded pipeline + versions and standalone post-processing algorithms therefore stay listed as + long as their results exist, while a configured-but-never-run algorithm does + not appear. + + Cost note (EXPLAIN-verified against a production copy): neither lookup can be + answered from an index, because neither table has a project column — project is + reachable only through source_image. Both sides scan, so the call costs roughly + 0.1-0.6 s cold regardless of project size, growing with total table size. + Executes the two lookups immediately rather than lazily; the id lists are tiny + (one row per algorithm) and sorted so the SQL string, and therefore cachalot's + cache key, is stable. Making this index-fast requires a denormalised project + column on Classification/Detection. + """ + from ami.main.models import Classification, Detection + + # ``order_by()`` clears each model's default ordering before ``distinct()``; + # otherwise the ordering columns widen the DISTINCT back to one row per result. + classifier_ids = ( + Classification.objects.filter(detection__source_image__project=project) + .order_by() + .values_list("algorithm_id", flat=True) + .distinct() + ) + detector_ids = ( + Detection.objects.filter(source_image__project=project) + .order_by() + .values_list("detection_algorithm", flat=True) + .distinct() + ) + ids = set(classifier_ids) | set(detector_ids) + ids.discard(None) # detections without a detection_algorithm + return self.filter(pk__in=sorted(ids)) + # Task types enum for better type checking class AlgorithmTaskType(str, enum.Enum): diff --git a/ami/ml/tests.py b/ami/ml/tests.py index 66cd7788e..206d42956 100644 --- a/ami/ml/tests.py +++ b/ami/ml/tests.py @@ -2070,13 +2070,15 @@ def test_deployment_counts_refresh_after_save_results(self): class TestAlgorithmViewSetProjectFilter(APITestCase): """ - The algorithm list endpoint is scoped to the algorithms relevant to the active - project, which they reach two ways. - - An enabled pipeline configures most of them. That covers detectors, which never - author a Classification and so could not be found by their results. Post-processing - algorithms such as class masking have no pipeline at all, so they are admitted by - having classified something in the project. + The algorithm list endpoint is scoped to the algorithms that actually produced + results in the active project, regardless of pipeline configuration. + + An algorithm qualifies by owning output rows: a detection made by it (detectors, + which never author a Classification) or a classification from it (classifiers and + standalone post-processing algorithms such as class masking). A superseded pipeline + version stays listed as long as its results survive, and an algorithm configured on + an enabled pipeline that has never run does not appear at all — the list reflects + what happened, not what is set up. """ def setUp(self): @@ -2086,14 +2088,14 @@ def setUp(self): self.project = Project.objects.create(name="Algo Project A", create_defaults=False) self.other_project = Project.objects.create(name="Algo Project B", create_defaults=False) - # Project A: an algorithm that has run, and one configured via a disabled pipeline + # Project A: an enabled-pipeline algorithm that has run, and one that never did. self.algo_used = Algorithm.objects.create(name="Algo Used", version=1) - self.algo_disabled = Algorithm.objects.create(name="Algo Disabled", version=1) - # Project A: enabled pipeline, but the algorithm never produced anything self.algo_configured_unused = Algorithm.objects.create(name="Algo Configured Unused", version=1) - # Project B: a different algorithm, also used + # Project A: an old version on a now-disabled pipeline, whose determinations survive. + self.algo_superseded = Algorithm.objects.create(name="Algo Superseded", version=1) + # Project B: a different algorithm, also used. self.algo_other_project = Algorithm.objects.create(name="Algo Other Project", version=1) - # Unrelated algorithm not attached to any pipeline and never run + # Unrelated algorithm attached to no pipeline and never run. self.algo_orphan = Algorithm.objects.create(name="Algo Orphan", version=1) enabled_pipeline = Pipeline.objects.create(name="Enabled Pipeline") @@ -2101,7 +2103,7 @@ def setUp(self): ProjectPipelineConfig.objects.create(project=self.project, pipeline=enabled_pipeline, enabled=True) disabled_pipeline = Pipeline.objects.create(name="Disabled Pipeline") - disabled_pipeline.algorithms.add(self.algo_disabled) + disabled_pipeline.algorithms.add(self.algo_superseded) ProjectPipelineConfig.objects.create(project=self.project, pipeline=disabled_pipeline, enabled=False) other_pipeline = Pipeline.objects.create(name="Other Project Pipeline") @@ -2109,6 +2111,7 @@ def setUp(self): ProjectPipelineConfig.objects.create(project=self.other_project, pipeline=other_pipeline, enabled=True) self._classify_in_project(self.algo_used, self.project) + self._classify_in_project(self.algo_superseded, self.project) self._classify_in_project(self.algo_other_project, self.other_project) self.client.force_authenticate(user=self.user) @@ -2130,23 +2133,33 @@ def _list_algorithm_names(self, project_id=None): self.assertEqual(response.status_code, 200) return {row["name"] for row in response.json()["results"]} - def test_lists_enabled_pipeline_algorithms_for_project(self): - """An enabled pipeline admits its algorithms whether or not they have run. + def test_lists_only_algorithms_that_produced_results(self): + """The project list is exactly the algorithms with output here: the classifier + that ran and the superseded version whose determinations survive. The enabled + pipeline's never-run algorithm is absent, proving configuration alone does not + admit an algorithm.""" + names = self._list_algorithm_names(project_id=self.project.pk) + self.assertEqual(names, {"Algo Used", "Algo Superseded"}) - Running is not required because detectors never author a Classification, so a - results-only rule would drop the detector behind every detection in the project. - """ + def test_configured_but_never_run_algorithm_is_hidden(self): + """An algorithm on an enabled pipeline that never produced a result does not + appear. This pins the semantic that the list follows results, not setup.""" + names = self._list_algorithm_names(project_id=self.project.pk) + self.assertNotIn("Algo Configured Unused", names) + + def test_superseded_pipeline_version_with_results_is_listed(self): + """An algorithm whose pipeline is disabled for the project — an older model + version, for instance — stays listed as long as its determinations exist, so the + user can still filter occurrences by what an earlier run produced.""" names = self._list_algorithm_names(project_id=self.project.pk) - self.assertEqual(names, {"Algo Used", "Algo Configured Unused"}) + self.assertIn("Algo Superseded", names) def test_detector_that_ran_is_listed_although_it_never_classified(self): """Detectors set ``Detection.detection_algorithm`` and never write a - Classification, so they are reachable only through their pipeline. This pins the - regression where scoping the list purely by classification authorship dropped + Classification, so they are reachable only through their detections. This pins + the regression where scoping the list purely by classification authorship dropped every localizer from the project's algorithm list.""" detector = Algorithm.objects.create(name="Algo Detector", version=1, task_type="localization") - pipeline = Pipeline.objects.get(name="Enabled Pipeline") - pipeline.algorithms.add(detector) source_image = SourceImage.objects.create(project=self.project) Detection.objects.create(source_image=source_image, detection_algorithm=detector) @@ -2163,20 +2176,22 @@ def test_unscoped_request_returns_all_algorithms(self): """Without project_id, current behavior lists all algorithms (unchanged).""" names = self._list_algorithm_names() self.assertIn("Algo Used", names) - self.assertIn("Algo Disabled", names) + self.assertIn("Algo Superseded", names) + self.assertIn("Algo Configured Unused", names) self.assertIn("Algo Other Project", names) self.assertIn("Algo Orphan", names) def test_detail_endpoint_unscoped_even_with_project_id(self): - """Detail stays unscoped so historical classification links still resolve.""" + """Detail stays unscoped so links to an algorithm outside the project's used + set — here an orphan that never ran — still resolve.""" url = reverse_with_params( "api:algorithm-detail", - kwargs={"pk": self.algo_disabled.pk}, + kwargs={"pk": self.algo_orphan.pk}, params={"project_id": self.project.pk}, ) response = self.client.get(url) self.assertEqual(response.status_code, 200) - self.assertEqual(response.json()["name"], "Algo Disabled") + self.assertEqual(response.json()["name"], "Algo Orphan") def test_lists_post_processing_algorithm_with_classifications_in_project(self): """A post-processing algorithm has no pipeline but produces determinations in @@ -2188,11 +2203,11 @@ def test_lists_post_processing_algorithm_with_classifications_in_project(self): names = self._list_algorithm_names(project_id=self.project.pk) self.assertIn("Class Masked Classifier", names) - def test_post_processing_lookup_is_deduplicated_in_the_database(self): - """The post-processing lookup matches one row per classification, so it must - deduplicate in SQL rather than in Python. + def test_used_lookup_is_deduplicated_in_the_database(self): + """``used_in_project`` matches one classification row per determination, so it + must deduplicate in SQL rather than in Python. - Without that, the rows fetched grow with a project's masked classification count — + Without that, the rows fetched grow with a project's classification count — hundreds of thousands on a real masking run — to identify a handful of algorithms. The listed names are correct either way, so this asserts the row count of the underlying lookup rather than the endpoint's output. The ``.order_by()`` matters: diff --git a/ami/ml/views.py b/ami/ml/views.py index 87ff46933..c0f626277 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -15,7 +15,7 @@ from ami.base.views import ProjectMixin from ami.main.api.schemas import project_id_doc_param from ami.main.api.views import DefaultViewSet -from ami.main.models import Classification, Project, SourceImage +from ami.main.models import Project, SourceImage from ami.ml.schemas import PipelineRegistrationResponse from .models.algorithm import Algorithm, AlgorithmCategoryMap @@ -61,53 +61,12 @@ def get_queryset(self) -> QuerySet["Algorithm"]: if getattr(self, "action", None) == "list": project = self.get_active_project() if project: - # Algorithms reach the list two ways. An enabled pipeline configures most of - # them, and that join alone covers detectors, which never author a - # Classification and so cannot be found by their results at all. - configured_for_project = Algorithm.objects.filter( - pipelines__project_pipeline_configs__project=project, - pipelines__project_pipeline_configs__enabled=True, - ).values_list("pk", flat=True) - - # Post-processing algorithms (e.g. class masking) are created standalone with - # no pipeline, so they are found by their classifications instead. This is done - # in two steps on purpose. Collecting the unpipelined algorithm ids first, as an - # explicit list, lets the classification lookup filter on `algorithm_id IN (...)`, - # which the planner answers from the algorithm_id index. Expressing the same thing - # as a single join (`pipelines__isnull=True, classifications__...`) hides those ids - # behind an anti-join at plan time, so the planner sequentially scans the whole - # classification table instead — a cost that grows with the platform's total - # classification count rather than with this project. The first query is also - # cheap to cache: it only changes when algorithms or pipeline links change, not on - # every classification write. - # - # An algorithm attached to a pipeline that is not enabled for this project is - # absent even when it owns determinations here. That matches the behaviour on the - # pipeline join alone; widening the second lookup to cover it costs several times - # the runtime and is left as a follow-up. - # Sorted for the same reason as the outer id list below: this list is rendered - # verbatim into the IN clause, and Algorithm's (name, version) ordering would - # otherwise vary the id order — and so the SQL string and its cachalot key — - # for identical membership. - unpipelined_algorithm_ids = sorted( - Algorithm.objects.filter(pipelines__isnull=True).values_list("pk", flat=True) - ) - # `.order_by()` clears Classification's default ordering before `.distinct()`. - # Without it the ordering columns join the SELECT and widen the DISTINCT, so it - # would return one row per classification rather than one per algorithm. - post_processing_used_in_project = ( - Classification.objects.filter( - algorithm_id__in=unpipelined_algorithm_ids, - detection__source_image__project=project, - ) - .order_by() - .values_list("algorithm_id", flat=True) - .distinct() - ) - # Sorted so the generated SQL is stable for a given result: cachalot keys its - # cache on the query string, and an unordered set would vary it. - relevant_ids = set(configured_for_project) | set(post_processing_used_in_project) - qs = qs.filter(pk__in=sorted(relevant_ids)) + # Scope the list to algorithms that actually produced results in the project — + # any superseded pipeline version or standalone post-processing algorithm whose + # output still exists, so the user can filter occurrences by anything that ran. + # Pipeline configuration is not consulted; configured-but-never-run algorithms + # do not appear. See the method for cost characteristics. + qs = qs.used_in_project(project) # type: ignore[union-attr] # Custom queryset method return qs @extend_schema(parameters=[project_id_doc_param]) From f0acd83f8efb534252839fb8ec27e4d80c7223fa Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Tue, 21 Jul 2026 16:55:06 -0700 Subject: [PATCH 12/16] feat(algorithms): flag which used algorithms are still enabled, and gray the rest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The project algorithm list returns every algorithm that produced results in the project, including superseded versions and standalone post-processing algorithms. Serve both consumers from that one list rather than splitting semantics: the occurrence filter needs the full historical set, and the algorithms page now shows the same set but grays out the algorithms that are no longer enabled for the project. Each algorithm in the project-scoped list carries `enabled_in_project` — true when it is on a pipeline the project has enabled, false when it only ran historically, and null on the unscoped list where there is no project for it to be relative to. The frontend Algorithms table dims the false rows through a new optional `rowClassName` on the shared Table component. Co-Authored-By: Claude --- ami/ml/serializers.py | 13 +++++++++++ ami/ml/tests.py | 23 +++++++++++++++++-- ami/ml/views.py | 18 ++++++++++++--- ui/src/data-services/models/algorithm.ts | 8 +++++++ .../components/table/table/table.tsx | 4 +++- .../pages/project/algorithms/algorithms.tsx | 5 ++++ 6 files changed, 65 insertions(+), 6 deletions(-) diff --git a/ami/ml/serializers.py b/ami/ml/serializers.py index e7e9e6aaf..4162b76db 100644 --- a/ami/ml/serializers.py +++ b/ami/ml/serializers.py @@ -30,6 +30,7 @@ class Meta: class AlgorithmSerializer(DefaultSerializer): category_map = MinimalCategoryMapNestedSerializer(read_only=True, source="category_map_id") + enabled_in_project = serializers.SerializerMethodField() class Meta: model = Algorithm @@ -45,10 +46,22 @@ class Meta: "task_type", "category_map", "category_count", + "enabled_in_project", "created_at", "updated_at", ] + def get_enabled_in_project(self, obj) -> bool | None: + """Whether the algorithm is on a pipeline the active project has enabled. + + The project algorithm list includes algorithms that produced results but are no + longer enabled — superseded versions, standalone post-processing algorithms — so + the UI can gray those out. Only the project-scoped list annotates this; it is + ``None`` on the unscoped list and on detail responses, where the flag has no + project to be relative to. + """ + return getattr(obj, "enabled_in_project", None) + class AlgorithmNestedSerializer(DefaultSerializer): class Meta: diff --git a/ami/ml/tests.py b/ami/ml/tests.py index 206d42956..4faf5e88e 100644 --- a/ami/ml/tests.py +++ b/ami/ml/tests.py @@ -2126,12 +2126,15 @@ def _classify_in_project(self, algorithm, project): timestamp=datetime.datetime.now(datetime.timezone.utc), ) - def _list_algorithm_names(self, project_id=None): + def _list_rows(self, project_id=None): params = {"project_id": project_id} if project_id is not None else {} url = reverse_with_params("api:algorithm-list", params=params) response = self.client.get(url) self.assertEqual(response.status_code, 200) - return {row["name"] for row in response.json()["results"]} + return {row["name"]: row for row in response.json()["results"]} + + def _list_algorithm_names(self, project_id=None): + return set(self._list_rows(project_id).keys()) def test_lists_only_algorithms_that_produced_results(self): """The project list is exactly the algorithms with output here: the classifier @@ -2154,6 +2157,22 @@ def test_superseded_pipeline_version_with_results_is_listed(self): names = self._list_algorithm_names(project_id=self.project.pk) self.assertIn("Algo Superseded", names) + def test_enabled_in_project_flag_distinguishes_current_from_historical(self): + """Every listed algorithm carries `enabled_in_project`: True when it is on a + pipeline the project has enabled, False when it only ran historically (a + superseded version on a disabled pipeline). The UI grays out the False ones, + which is what lets the same list serve both the algorithms page and the + occurrence filter.""" + rows = self._list_rows(project_id=self.project.pk) + self.assertTrue(rows["Algo Used"]["enabled_in_project"]) + self.assertFalse(rows["Algo Superseded"]["enabled_in_project"]) + + def test_enabled_in_project_flag_is_null_when_unscoped(self): + """The flag is relative to a project, so the unscoped list reports it as null + rather than guessing a project to be enabled in.""" + rows = self._list_rows() + self.assertIsNone(rows["Algo Used"]["enabled_in_project"]) + def test_detector_that_ran_is_listed_although_it_never_classified(self): """Detectors set ``Detection.detection_algorithm`` and never write a Classification, so they are reachable only through their detections. This pins diff --git a/ami/ml/views.py b/ami/ml/views.py index c0f626277..4a175a88a 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -1,7 +1,7 @@ import logging from django.db import transaction -from django.db.models import Prefetch +from django.db.models import Exists, OuterRef, Prefetch from django.db.models.query import QuerySet from django.utils.text import slugify from drf_spectacular.utils import extend_schema @@ -64,9 +64,21 @@ def get_queryset(self) -> QuerySet["Algorithm"]: # Scope the list to algorithms that actually produced results in the project — # any superseded pipeline version or standalone post-processing algorithm whose # output still exists, so the user can filter occurrences by anything that ran. - # Pipeline configuration is not consulted; configured-but-never-run algorithms - # do not appear. See the method for cost characteristics. + # Pipeline configuration is not consulted for membership; configured-but-never-run + # algorithms do not appear. See the method for cost characteristics. qs = qs.used_in_project(project) # type: ignore[union-attr] # Custom queryset method + # Flag each one with whether it is still enabled for the project (on a pipeline + # the project has enabled). The list intentionally includes algorithms that ran + # but are no longer enabled, e.g. superseded versions; the UI grays those out. + qs = qs.annotate( + enabled_in_project=Exists( + ProjectPipelineConfig.objects.filter( + project=project, + enabled=True, + pipeline__algorithms=OuterRef("pk"), + ) + ) + ) return qs @extend_schema(parameters=[project_id_doc_param]) diff --git a/ui/src/data-services/models/algorithm.ts b/ui/src/data-services/models/algorithm.ts index c36d507c4..b4dc1871e 100644 --- a/ui/src/data-services/models/algorithm.ts +++ b/ui/src/data-services/models/algorithm.ts @@ -43,4 +43,12 @@ export class Algorithm extends Entity { ? this._algorithm.category_count : undefined } + + // Whether the algorithm is on a pipeline the active project has enabled. The + // project list also includes algorithms that only ran historically (superseded + // versions, post-processing algorithms), which come back false so the UI can gray + // them out. Undefined on the unscoped list, where there is no project to be enabled in. + get enabledInProject(): boolean | undefined { + return this._algorithm.enabled_in_project ?? undefined + } } diff --git a/ui/src/nova-ui-kit/components/table/table/table.tsx b/ui/src/nova-ui-kit/components/table/table/table.tsx index 987a21b4f..f61f86545 100644 --- a/ui/src/nova-ui-kit/components/table/table/table.tsx +++ b/ui/src/nova-ui-kit/components/table/table/table.tsx @@ -29,6 +29,7 @@ interface TableProps { items?: T[] onSelectedItemsChange?: (selectedItems: string[]) => void onSortSettingsChange?: (sortSettings?: TableSortSettings) => void + rowClassName?: (item: T) => string | undefined selectable?: boolean selectedItems?: string[] sortable?: boolean @@ -43,6 +44,7 @@ export const Table = ({ items = [], onSelectedItemsChange, onSortSettingsChange, + rowClassName, selectable, selectedItems = [], sortable, @@ -125,7 +127,7 @@ export const Table = ({ {items.map((item, rowIndex) => ( - + {selectable && ( diff --git a/ui/src/pages/project/algorithms/algorithms.tsx b/ui/src/pages/project/algorithms/algorithms.tsx index dc66fd17e..d4c24a141 100644 --- a/ui/src/pages/project/algorithms/algorithms.tsx +++ b/ui/src/pages/project/algorithms/algorithms.tsx @@ -61,6 +61,11 @@ export const Algorithms = () => { isLoading={isLoading} items={algorithms} onSortSettingsChange={setSort} + // Algorithms that ran in the project but are no longer enabled here (superseded + // versions, post-processing algorithms) are shown grayed rather than hidden. + rowClassName={(algorithm) => + algorithm.enabledInProject === false ? 'opacity-50' : undefined + } sortable sortSettings={sort} /> From 5a2ae45def950a02817fe6fadaf7fe043439186b Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Wed, 22 Jul 2026 14:45:54 -0700 Subject: [PATCH 13/16] refactor(algorithms): split the project algorithm page from occurrence filter choices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The project-scoped algorithm list reverts to main's behavior — the algorithms on the project's enabled pipelines — so a freshly configured project sees what it can run before any job has completed. The used-only list (every algorithm that produced results here, including superseded versions, detectors, and standalone post-processing algorithms) moves to a new OccurrenceViewSet sub-action at /occurrences/algorithms/, where it serves the occurrence filter's choices so the filter never offers a zero-result value. The enabled_in_project annotation and serializer field are dropped — the page no longer mixes historical algorithms in, so there is nothing to gray out. Also renames the occurrence queryset methods: detected_or_classified_by → processed_by_algorithm (and the not_/_q variants), since the EXISTS matches post-processing algorithms such as a size filter through the classification leg — they neither detect nor classify. Future siblings can follow the same family: processed_by_pipeline, processed_by_job. Co-Authored-By: Claude --- ami/main/api/views.py | 33 ++- ami/main/models.py | 12 +- ami/main/tests.py | 14 +- ami/ml/serializers.py | 13 -- ami/ml/tests.py | 197 ++++++++++-------- ami/ml/views.py | 28 +-- ui/src/data-services/models/algorithm.ts | 8 - .../components/table/table/table.tsx | 4 +- .../pages/project/algorithms/algorithms.tsx | 5 - 9 files changed, 169 insertions(+), 145 deletions(-) diff --git a/ami/main/api/views.py b/ami/main/api/views.py index b499002b9..18670fdcf 100644 --- a/ami/main/api/views.py +++ b/ami/main/api/views.py @@ -35,6 +35,8 @@ from ami.main.api.schemas import limit_doc_param, project_id_doc_param from ami.main.api.serializers import TagSerializer from ami.main.models_future.occurrence import model_agreement_for_project, top_identifiers_for_project +from ami.ml.models.algorithm import Algorithm +from ami.ml.serializers import AlgorithmSerializer from ami.utils.requests import get_default_classification_threshold from ami.utils.storages import ConnectionTestResult @@ -1241,9 +1243,9 @@ def filter_queryset(self, request, queryset, view): algorithm_ids_exclusive = request.query_params.getlist(self.query_param_exclusive) if algorithm_ids: - queryset = queryset.detected_or_classified_by(algorithm_ids) + queryset = queryset.processed_by_algorithm(algorithm_ids) if algorithm_ids_exclusive: - queryset = queryset.not_detected_or_classified_by(algorithm_ids_exclusive) + queryset = queryset.not_processed_by_algorithm(algorithm_ids_exclusive) return queryset @@ -1471,6 +1473,33 @@ def get_queryset(self) -> QuerySet["Occurrence"]: def list(self, request, *args, **kwargs): return super().list(request, *args, **kwargs) + @extend_schema(parameters=[project_id_doc_param], responses=AlgorithmSerializer(many=True)) + @action(detail=False, methods=["get"], name="algorithms") + def algorithms(self, request: Request) -> Response: + """Choices for the occurrence algorithm filter: every algorithm that produced a result in the project. + + Includes superseded pipeline versions and standalone post-processing algorithms + (their output is still filterable), so it differs from the project's algorithm + list at ``/ml/algorithms/``, which shows what the project can run — its + enabled-pipeline algorithms. Serving choices from what actually ran means the + filter never offers a value with zero results. + """ + project = self.get_active_project() + if project is None: + raise api_exceptions.ValidationError({"project_id": "This parameter is required."}) + if not Project.objects.visible_for_user(request.user).filter(pk=project.pk).exists(): + raise NotFound("Project not found.") + + qs = ( + Algorithm.objects.all() + .with_category_count() # type: ignore[union-attr] # Custom queryset method + .used_in_project(project) + .order_by("name") + ) + page = self.paginate_queryset(qs) + serializer = AlgorithmSerializer(page, many=True, context=self.get_serializer_context()) + return self.get_paginated_response(serializer.data) + class OccurrenceStatsViewSet(viewsets.GenericViewSet, ProjectMixin): """Aggregate stats over Occurrences. Each @action == one stats kind. diff --git a/ami/main/models.py b/ami/main/models.py index 017b51263..1b4ac3d4e 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -3353,10 +3353,10 @@ def valid(self): def with_detections_count(self): return self.annotate(detections_count=models.Count("detections", distinct=True)) - def _machine_results_by(self, algorithm_ids) -> Exists: + def _processed_by_algorithm_q(self, algorithm_ids) -> Exists: """Subquery matching occurrences with any result from the given algorithms — a detection made by one (detectors) or a classification from one (classifiers - and post-processing algorithms).""" + and post-processing algorithms such as class masking or a size filter).""" return Exists( Detection.objects.filter(occurrence_id=OuterRef("pk")).filter( models.Q(detection_algorithm__in=algorithm_ids) @@ -3364,7 +3364,7 @@ def _machine_results_by(self, algorithm_ids) -> Exists: ) ) - def detected_or_classified_by(self, algorithm_ids) -> "OccurrenceQuerySet": + def processed_by_algorithm(self, algorithm_ids) -> "OccurrenceQuerySet": """Occurrences with at least one result from the given algorithms. Matches detectors through Detection.detection_algorithm and classifiers or @@ -3374,11 +3374,11 @@ def detected_or_classified_by(self, algorithm_ids) -> "OccurrenceQuerySet": ``detections__classifications`` returns one row per matching result, which inflates pagination counts and duplicates rows across pages. """ - return self.filter(self._machine_results_by(algorithm_ids)) + return self.filter(self._processed_by_algorithm_q(algorithm_ids)) - def not_detected_or_classified_by(self, algorithm_ids) -> "OccurrenceQuerySet": + def not_processed_by_algorithm(self, algorithm_ids) -> "OccurrenceQuerySet": """Occurrences with no result from any of the given algorithms.""" - return self.exclude(self._machine_results_by(algorithm_ids)) + return self.exclude(self._processed_by_algorithm_q(algorithm_ids)) def with_timestamps(self): """ diff --git a/ami/main/tests.py b/ami/main/tests.py index fb8bb771f..1bc8568b8 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -6512,7 +6512,7 @@ def test_valid_returns_only_real_with_determination(self): class TestOccurrenceAlgorithmFilterQuerySet(TestCase): """ - Covers OccurrenceQuerySet.detected_or_classified_by / not_detected_or_classified_by, + Covers OccurrenceQuerySet.processed_by_algorithm / not_processed_by_algorithm, which back the ?algorithm= and ?not_algorithm= occurrence filters (PR #1368). The filter must match an algorithm by either role it can play: the detector that @@ -6582,32 +6582,32 @@ def _pks(self, queryset): return set(queryset.values_list("pk", flat=True)) def test_filter_by_classifier_matches_its_occurrences(self): - matched = Occurrence.objects.filter(project=self.project).detected_or_classified_by([self.classifier.pk]) + matched = Occurrence.objects.filter(project=self.project).processed_by_algorithm([self.classifier.pk]) self.assertEqual(self._pks(matched), {self.occ_classified.pk, self.occ_multi.pk}) def test_filter_by_detector_matches_occurrences_it_detected(self): """A detector authors no Classification, so the old classification-join form returned nothing for it. Matching through Detection.detection_algorithm is what lets the user filter by a localizer at all.""" - matched = Occurrence.objects.filter(project=self.project).detected_or_classified_by([self.detector.pk]) + matched = Occurrence.objects.filter(project=self.project).processed_by_algorithm([self.detector.pk]) self.assertEqual(self._pks(matched), {self.occ_classified.pk, self.occ_multi.pk}) def test_count_not_inflated_by_multiple_classifications(self): """The occurrence with three classifications by the same algorithm must count once. The paginator calls this same COUNT, so an inflated value would report four occurrences where there are two and repeat rows across pages.""" - matched = Occurrence.objects.filter(project=self.project).detected_or_classified_by([self.classifier.pk]) + matched = Occurrence.objects.filter(project=self.project).processed_by_algorithm([self.classifier.pk]) self.assertEqual(matched.count(), 2) def test_exclude_removes_matching_occurrences(self): - remaining = Occurrence.objects.filter(project=self.project).not_detected_or_classified_by([self.other.pk]) + remaining = Occurrence.objects.filter(project=self.project).not_processed_by_algorithm([self.other.pk]) self.assertEqual(self._pks(remaining), {self.occ_classified.pk, self.occ_multi.pk}) def test_exclude_is_the_complement_of_include(self): base = Occurrence.objects.filter(project=self.project) ids = [self.classifier.pk] - included = self._pks(base.detected_or_classified_by(ids)) - excluded = self._pks(base.not_detected_or_classified_by(ids)) + included = self._pks(base.processed_by_algorithm(ids)) + excluded = self._pks(base.not_processed_by_algorithm(ids)) self.assertEqual(included | excluded, self._pks(base)) self.assertEqual(included & excluded, set()) diff --git a/ami/ml/serializers.py b/ami/ml/serializers.py index 4162b76db..e7e9e6aaf 100644 --- a/ami/ml/serializers.py +++ b/ami/ml/serializers.py @@ -30,7 +30,6 @@ class Meta: class AlgorithmSerializer(DefaultSerializer): category_map = MinimalCategoryMapNestedSerializer(read_only=True, source="category_map_id") - enabled_in_project = serializers.SerializerMethodField() class Meta: model = Algorithm @@ -46,22 +45,10 @@ class Meta: "task_type", "category_map", "category_count", - "enabled_in_project", "created_at", "updated_at", ] - def get_enabled_in_project(self, obj) -> bool | None: - """Whether the algorithm is on a pipeline the active project has enabled. - - The project algorithm list includes algorithms that produced results but are no - longer enabled — superseded versions, standalone post-processing algorithms — so - the UI can gray those out. Only the project-scoped list annotates this; it is - ``None`` on the unscoped list and on detail responses, where the flag has no - project to be relative to. - """ - return getattr(obj, "enabled_in_project", None) - class AlgorithmNestedSerializer(DefaultSerializer): class Meta: diff --git a/ami/ml/tests.py b/ami/ml/tests.py index 4faf5e88e..bd92bb02f 100644 --- a/ami/ml/tests.py +++ b/ami/ml/tests.py @@ -2068,17 +2068,13 @@ def test_deployment_counts_refresh_after_save_results(self): ) -class TestAlgorithmViewSetProjectFilter(APITestCase): - """ - The algorithm list endpoint is scoped to the algorithms that actually produced - results in the active project, regardless of pipeline configuration. +class AlgorithmProjectTestBase(APITestCase): + """Shared fixture for the two project-scoped algorithm listings. - An algorithm qualifies by owning output rows: a detection made by it (detectors, - which never author a Classification) or a classification from it (classifiers and - standalone post-processing algorithms such as class masking). A superseded pipeline - version stays listed as long as its results survive, and an algorithm configured on - an enabled pipeline that has never run does not appear at all — the list reflects - what happened, not what is set up. + Project A has an enabled pipeline carrying one algorithm that ran ("Algo Used") + and one that never did ("Algo Configured Unused"), plus a disabled pipeline whose + algorithm's determinations survive ("Algo Superseded"). Project B has its own + used algorithm. "Algo Orphan" belongs to no pipeline and never ran anywhere. """ def setUp(self): @@ -2126,66 +2122,40 @@ def _classify_in_project(self, algorithm, project): timestamp=datetime.datetime.now(datetime.timezone.utc), ) - def _list_rows(self, project_id=None): + +class TestAlgorithmViewSetProjectFilter(AlgorithmProjectTestBase): + """ + The algorithm list endpoint scoped to a project shows what the project can run: + the algorithms on its enabled pipelines. + + It reflects configuration, not history — a freshly configured project sees its + algorithms before anything has run, and an algorithm only on a disabled pipeline + is not offered even if it ran in the past. The algorithms that actually produced + results are served separately as occurrence filter choices (see + TestOccurrenceAlgorithmChoices), and detail pages stay reachable for any + algorithm through the unscoped detail endpoint. + """ + + def _list_algorithm_names(self, project_id=None): params = {"project_id": project_id} if project_id is not None else {} url = reverse_with_params("api:algorithm-list", params=params) response = self.client.get(url) self.assertEqual(response.status_code, 200) - return {row["name"]: row for row in response.json()["results"]} - - def _list_algorithm_names(self, project_id=None): - return set(self._list_rows(project_id).keys()) - - def test_lists_only_algorithms_that_produced_results(self): - """The project list is exactly the algorithms with output here: the classifier - that ran and the superseded version whose determinations survive. The enabled - pipeline's never-run algorithm is absent, proving configuration alone does not - admit an algorithm.""" - names = self._list_algorithm_names(project_id=self.project.pk) - self.assertEqual(names, {"Algo Used", "Algo Superseded"}) - - def test_configured_but_never_run_algorithm_is_hidden(self): - """An algorithm on an enabled pipeline that never produced a result does not - appear. This pins the semantic that the list follows results, not setup.""" - names = self._list_algorithm_names(project_id=self.project.pk) - self.assertNotIn("Algo Configured Unused", names) + return {row["name"] for row in response.json()["results"]} - def test_superseded_pipeline_version_with_results_is_listed(self): - """An algorithm whose pipeline is disabled for the project — an older model - version, for instance — stays listed as long as its determinations exist, so the - user can still filter occurrences by what an earlier run produced.""" + def test_project_list_shows_enabled_pipeline_algorithms(self): + """The scoped list is exactly the enabled pipelines' algorithms — including + one that has never run, so a new project can see what it is set up to use + before any job has completed.""" names = self._list_algorithm_names(project_id=self.project.pk) - self.assertIn("Algo Superseded", names) - - def test_enabled_in_project_flag_distinguishes_current_from_historical(self): - """Every listed algorithm carries `enabled_in_project`: True when it is on a - pipeline the project has enabled, False when it only ran historically (a - superseded version on a disabled pipeline). The UI grays out the False ones, - which is what lets the same list serve both the algorithms page and the - occurrence filter.""" - rows = self._list_rows(project_id=self.project.pk) - self.assertTrue(rows["Algo Used"]["enabled_in_project"]) - self.assertFalse(rows["Algo Superseded"]["enabled_in_project"]) - - def test_enabled_in_project_flag_is_null_when_unscoped(self): - """The flag is relative to a project, so the unscoped list reports it as null - rather than guessing a project to be enabled in.""" - rows = self._list_rows() - self.assertIsNone(rows["Algo Used"]["enabled_in_project"]) - - def test_detector_that_ran_is_listed_although_it_never_classified(self): - """Detectors set ``Detection.detection_algorithm`` and never write a - Classification, so they are reachable only through their detections. This pins - the regression where scoping the list purely by classification authorship dropped - every localizer from the project's algorithm list.""" - detector = Algorithm.objects.create(name="Algo Detector", version=1, task_type="localization") - - source_image = SourceImage.objects.create(project=self.project) - Detection.objects.create(source_image=source_image, detection_algorithm=detector) - self.assertFalse(Classification.objects.filter(algorithm=detector).exists()) + self.assertEqual(names, {"Algo Used", "Algo Configured Unused"}) + def test_disabled_pipeline_algorithm_is_not_listed(self): + """An algorithm only on a pipeline the project has disabled is not part of + what the project can run, even though its past determinations survive. Those + stay reachable through the occurrence filter choices and the detail endpoint.""" names = self._list_algorithm_names(project_id=self.project.pk) - self.assertIn("Algo Detector", names) + self.assertNotIn("Algo Superseded", names) def test_other_project_only_sees_its_own_algorithms(self): names = self._list_algorithm_names(project_id=self.other_project.pk) @@ -2194,15 +2164,19 @@ def test_other_project_only_sees_its_own_algorithms(self): def test_unscoped_request_returns_all_algorithms(self): """Without project_id, current behavior lists all algorithms (unchanged).""" names = self._list_algorithm_names() - self.assertIn("Algo Used", names) - self.assertIn("Algo Superseded", names) - self.assertIn("Algo Configured Unused", names) - self.assertIn("Algo Other Project", names) - self.assertIn("Algo Orphan", names) + for name in ( + "Algo Used", + "Algo Superseded", + "Algo Configured Unused", + "Algo Other Project", + "Algo Orphan", + ): + self.assertIn(name, names) def test_detail_endpoint_unscoped_even_with_project_id(self): - """Detail stays unscoped so links to an algorithm outside the project's used - set — here an orphan that never ran — still resolve.""" + """Detail stays unscoped so a link from a historical classification — here an + algorithm outside the project's enabled set — still resolves to its details + and category map.""" url = reverse_with_params( "api:algorithm-detail", kwargs={"pk": self.algo_orphan.pk}, @@ -2212,15 +2186,82 @@ def test_detail_endpoint_unscoped_even_with_project_id(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["name"], "Algo Orphan") - def test_lists_post_processing_algorithm_with_classifications_in_project(self): + +class TestOccurrenceAlgorithmChoices(AlgorithmProjectTestBase): + """ + The occurrence filter's algorithm choices, served at /occurrences/algorithms/, + are exactly the algorithms that produced results in the project. + + An algorithm qualifies by owning output rows: a detection made by it (detectors, + which never author a Classification) or a classification from it (classifiers and + standalone post-processing algorithms such as class masking). A superseded + pipeline version stays a choice as long as its results survive, and a configured + algorithm that never ran is not offered — the filter never lists a value with + zero matching occurrences. + """ + + def _choice_names(self, project_id): + url = reverse_with_params("api:occurrence-algorithms", params={"project_id": project_id}) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + return {row["name"] for row in response.json()["results"]} + + def test_choices_are_exactly_the_algorithms_that_produced_results(self): + """The classifier that ran and the superseded version whose determinations + survive are choices; the enabled pipeline's never-run algorithm is not. + This pins the semantic that choices follow results, not setup.""" + names = self._choice_names(self.project.pk) + self.assertEqual(names, {"Algo Used", "Algo Superseded"}) + + def test_configured_but_never_run_algorithm_is_not_a_choice(self): + """Filtering by an algorithm that never produced a result would always return + zero occurrences, so configuration alone does not admit one.""" + names = self._choice_names(self.project.pk) + self.assertNotIn("Algo Configured Unused", names) + + def test_detector_that_ran_is_a_choice_although_it_never_classified(self): + """Detectors set ``Detection.detection_algorithm`` and never write a + Classification, so they are reachable only through their detections. This pins + the regression where scoping choices purely by classification authorship + dropped every localizer.""" + detector = Algorithm.objects.create(name="Algo Detector", version=1, task_type="localization") + + source_image = SourceImage.objects.create(project=self.project) + Detection.objects.create(source_image=source_image, detection_algorithm=detector) + self.assertFalse(Classification.objects.filter(algorithm=detector).exists()) + + self.assertIn("Algo Detector", self._choice_names(self.project.pk)) + + def test_post_processing_algorithm_with_classifications_is_a_choice(self): """A post-processing algorithm has no pipeline but produces determinations in - the project, so the list must include it — otherwise the user cannot filter + the project, so it must be offered — otherwise the user cannot filter occurrences by the masked result.""" masked_algo = Algorithm.objects.create(name="Class Masked Classifier", version=1) self._classify_in_project(masked_algo, self.project) - names = self._list_algorithm_names(project_id=self.project.pk) - self.assertIn("Class Masked Classifier", names) + self.assertIn("Class Masked Classifier", self._choice_names(self.project.pk)) + + def test_classifications_in_other_project_do_not_leak(self): + """An algorithm whose classifications live in another project must not appear.""" + other_masked_algo = Algorithm.objects.create(name="Other Project Masked", version=1) + self._classify_in_project(other_masked_algo, self.other_project) + + self.assertNotIn("Other Project Masked", self._choice_names(self.project.pk)) + + def test_project_id_is_required(self): + """Choices are relative to a project; without one the request is rejected + rather than listing every algorithm on the platform.""" + url = reverse_with_params("api:occurrence-algorithms") + response = self.client.get(url) + self.assertEqual(response.status_code, 400) + + def test_response_is_paginated_like_a_list_endpoint(self): + """The endpoint returns the standard ``{count, results}`` shape the + frontend's entity picker consumes.""" + url = reverse_with_params("api:occurrence-algorithms", params={"project_id": self.project.pk}) + data = self.client.get(url).json() + self.assertEqual(data["count"], 2) + self.assertEqual(len(data["results"]), 2) def test_used_lookup_is_deduplicated_in_the_database(self): """``used_in_project`` matches one classification row per determination, so it @@ -2246,12 +2287,4 @@ def test_used_lookup_is_deduplicated_in_the_database(self): self.assertEqual( len(list(lookup.order_by().distinct())), 1, "Deduplicating collapses them to the one algorithm" ) - self.assertIn("Chatty Masked Classifier", self._list_algorithm_names(project_id=self.project.pk)) - - def test_classifications_in_other_project_do_not_leak(self): - """An algorithm whose classifications live in another project must not appear.""" - other_masked_algo = Algorithm.objects.create(name="Other Project Masked", version=1) - self._classify_in_project(other_masked_algo, self.other_project) - - names = self._list_algorithm_names(project_id=self.project.pk) - self.assertNotIn("Other Project Masked", names) + self.assertIn("Chatty Masked Classifier", self._choice_names(self.project.pk)) diff --git a/ami/ml/views.py b/ami/ml/views.py index 4a175a88a..63e460af6 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -1,7 +1,7 @@ import logging from django.db import transaction -from django.db.models import Exists, OuterRef, Prefetch +from django.db.models import Prefetch from django.db.models.query import QuerySet from django.utils.text import slugify from drf_spectacular.utils import extend_schema @@ -61,24 +61,14 @@ def get_queryset(self) -> QuerySet["Algorithm"]: if getattr(self, "action", None) == "list": project = self.get_active_project() if project: - # Scope the list to algorithms that actually produced results in the project — - # any superseded pipeline version or standalone post-processing algorithm whose - # output still exists, so the user can filter occurrences by anything that ran. - # Pipeline configuration is not consulted for membership; configured-but-never-run - # algorithms do not appear. See the method for cost characteristics. - qs = qs.used_in_project(project) # type: ignore[union-attr] # Custom queryset method - # Flag each one with whether it is still enabled for the project (on a pipeline - # the project has enabled). The list intentionally includes algorithms that ran - # but are no longer enabled, e.g. superseded versions; the UI grays those out. - qs = qs.annotate( - enabled_in_project=Exists( - ProjectPipelineConfig.objects.filter( - project=project, - enabled=True, - pipeline__algorithms=OuterRef("pk"), - ) - ) - ) + # The project-scoped list shows the algorithms available to the project — those + # on its enabled pipelines — so a freshly configured project sees what it can + # run before anything has run. The algorithms that actually produced results + # (including superseded versions) are served by /occurrences/algorithms/. + qs = qs.filter( + pipelines__project_pipeline_configs__project=project, + pipelines__project_pipeline_configs__enabled=True, + ).distinct() return qs @extend_schema(parameters=[project_id_doc_param]) diff --git a/ui/src/data-services/models/algorithm.ts b/ui/src/data-services/models/algorithm.ts index b4dc1871e..c36d507c4 100644 --- a/ui/src/data-services/models/algorithm.ts +++ b/ui/src/data-services/models/algorithm.ts @@ -43,12 +43,4 @@ export class Algorithm extends Entity { ? this._algorithm.category_count : undefined } - - // Whether the algorithm is on a pipeline the active project has enabled. The - // project list also includes algorithms that only ran historically (superseded - // versions, post-processing algorithms), which come back false so the UI can gray - // them out. Undefined on the unscoped list, where there is no project to be enabled in. - get enabledInProject(): boolean | undefined { - return this._algorithm.enabled_in_project ?? undefined - } } diff --git a/ui/src/nova-ui-kit/components/table/table/table.tsx b/ui/src/nova-ui-kit/components/table/table/table.tsx index f61f86545..987a21b4f 100644 --- a/ui/src/nova-ui-kit/components/table/table/table.tsx +++ b/ui/src/nova-ui-kit/components/table/table/table.tsx @@ -29,7 +29,6 @@ interface TableProps { items?: T[] onSelectedItemsChange?: (selectedItems: string[]) => void onSortSettingsChange?: (sortSettings?: TableSortSettings) => void - rowClassName?: (item: T) => string | undefined selectable?: boolean selectedItems?: string[] sortable?: boolean @@ -44,7 +43,6 @@ export const Table = ({ items = [], onSelectedItemsChange, onSortSettingsChange, - rowClassName, selectable, selectedItems = [], sortable, @@ -127,7 +125,7 @@ export const Table = ({ {items.map((item, rowIndex) => ( - + {selectable && ( diff --git a/ui/src/pages/project/algorithms/algorithms.tsx b/ui/src/pages/project/algorithms/algorithms.tsx index d4c24a141..dc66fd17e 100644 --- a/ui/src/pages/project/algorithms/algorithms.tsx +++ b/ui/src/pages/project/algorithms/algorithms.tsx @@ -61,11 +61,6 @@ export const Algorithms = () => { isLoading={isLoading} items={algorithms} onSortSettingsChange={setSort} - // Algorithms that ran in the project but are no longer enabled here (superseded - // versions, post-processing algorithms) are shown grayed rather than hidden. - rowClassName={(algorithm) => - algorithm.enabledInProject === false ? 'opacity-50' : undefined - } sortable sortSettings={sort} /> From f9ea55f16c2fefdf3a22e0aa2bda93459158f4be Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Wed, 22 Jul 2026 14:46:13 -0700 Subject: [PATCH 14/16] refactor(ui): point the occurrence algorithm filter at /occurrences/algorithms/ The filter's choices now come from the used-only sub-action, so the picker offers exactly the algorithms with results in the project. The algorithms page keeps reading /ml/algorithms/, which lists the enabled-pipeline set. Co-Authored-By: Claude --- ui/src/components/filtering/filters/algorithm-filter.tsx | 2 +- ui/src/data-services/constants.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ui/src/components/filtering/filters/algorithm-filter.tsx b/ui/src/components/filtering/filters/algorithm-filter.tsx index b4cbe932e..0e487880c 100644 --- a/ui/src/components/filtering/filters/algorithm-filter.tsx +++ b/ui/src/components/filtering/filters/algorithm-filter.tsx @@ -7,7 +7,7 @@ export const AlgorithmFilter = ({ onAdd, }: FilterProps & { placeholder?: string }) => ( { if (value) { onAdd(value) diff --git a/ui/src/data-services/constants.ts b/ui/src/data-services/constants.ts index 135896386..7cfd6b33c 100644 --- a/ui/src/data-services/constants.ts +++ b/ui/src/data-services/constants.ts @@ -14,6 +14,7 @@ export const API_ROUTES = { LOGOUT: 'auth/token/logout', ME: 'users/me', MEMBERS: (projectId: string) => `projects/${projectId}/members`, + OCCURRENCE_ALGORITHMS: 'occurrences/algorithms', OCCURRENCES: 'occurrences', PAGES: 'pages', PIPELINES: 'ml/pipelines', From 6332ab571929db69081bf21a93ea4f0a4d4e9db2 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Wed, 22 Jul 2026 16:51:15 -0700 Subject: [PATCH 15/16] test(post-processing): pass the masking algorithm to the scope-shape tests The scope-shape tests merged from main call _scoped_classifications with its pre-idempotency two-argument signature; this branch added a third argument, the masking algorithm whose lineage the scope excludes. A textual merge could not catch the mismatch. Co-Authored-By: Claude --- ami/ml/post_processing/tests/test_class_masking.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ami/ml/post_processing/tests/test_class_masking.py b/ami/ml/post_processing/tests/test_class_masking.py index 2143971ab..bf38decc9 100644 --- a/ami/ml/post_processing/tests/test_class_masking.py +++ b/ami/ml/post_processing/tests/test_class_masking.py @@ -563,7 +563,10 @@ def test_collection_scope_returns_each_classification_once(self): taxa_list_id=taxa_list.pk, algorithm_id=self.algorithm.pk, ) - scoped, _ = task._scoped_classifications(task.config, self.algorithm) + masking_algorithm = task._get_or_create_masking_algorithm( + self.algorithm, taxa_list, reweight=task.config.reweight + ) + scoped, _ = task._scoped_classifications(task.config, self.algorithm, masking_algorithm) self.assertEqual(list(scoped.filter(pk=classification.pk)), [classification]) self.assertEqual( @@ -589,7 +592,10 @@ def test_scope_query_does_not_deduplicate_rows(self): ): with self.subTest(**kwargs): task = ClassMaskingTask(taxa_list_id=taxa_list.pk, algorithm_id=self.algorithm.pk, **kwargs) - scoped, _ = task._scoped_classifications(task.config, self.algorithm) + masking_algorithm = task._get_or_create_masking_algorithm( + self.algorithm, taxa_list, reweight=task.config.reweight + ) + scoped, _ = task._scoped_classifications(task.config, self.algorithm, masking_algorithm) self.assertFalse(scoped.query.distinct) def test_scope_size_is_reported_before_the_first_batch(self): From 0306324ccd8771c823b6245883557d70394c5138 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Wed, 22 Jul 2026 20:34:12 -0700 Subject: [PATCH 16/16] docs(post-processing): point the replay-guard exclude at its generalization path [skip ci] Comment-only change. If a second post-processing task records applied_to lineage and wants the same idempotency, the exclude should become a ClassificationQuerySet method instead of being copied. Co-Authored-By: Claude --- ami/ml/post_processing/class_masking.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ami/ml/post_processing/class_masking.py b/ami/ml/post_processing/class_masking.py index 282ed8583..2da2001b7 100644 --- a/ami/ml/post_processing/class_masking.py +++ b/ami/ml/post_processing/class_masking.py @@ -298,8 +298,12 @@ def _scoped_classifications( scores__isnull=False, logits__isnull=False, ) - .exclude(derived_classifications__algorithm=masking_algorithm) - .select_related("detection", "detection__occurrence") + # If another task needs this replay guard, hoist it to a + # ClassificationQuerySet method (e.g. not_derived_by(algorithm)) + # rather than copying the exclude. + .exclude(derived_classifications__algorithm=masking_algorithm).select_related( + "detection", "detection__occurrence" + ) ) if config.occurrence_id is not None: