Skip to content

Commit ea1e498

Browse files
mihowclaude
andcommitted
refactor(occurrence-stats): rename to model-agreement + push aggregation to SQL
Addresses review feedback on PR #1307: Rename (drop "human"): - URL: /occurrences/stats/human-model-agreement/ -> /model-agreement/ - Function: human_model_agreement_for_project -> model_agreement_for_project - Serializer: HumanModelAgreementSerializer -> ModelAgreementSerializer - Viewset action + url_path: human_model_agreement -> model_agreement - FE hook: useHumanModelAgreement -> useModelAgreement (file + symbol) - FE type: Response -> ModelAgreementResponse (fixes DOM Response shadow) - Test class: TestHumanModelAgreementForProject -> TestModelAgreementForProject SQL push-down (Copilot+CodeRabbit perf flag): - Replace list(qs) full-row materialization with annotated aggregate(). - Annotate best_user_taxon_id via Subquery over Identification (BEST_IDENTIFICATION_ORDER). Drop the prefetch + select_related("taxon") on identifications since only taxon_id is read. - aggregate() Count(filter=Q(...)) for total/verified/exact/no-prediction. - For under-order disagreement: group disagreement set by distinct (user_taxon, machine_taxon) pair before LCA. Each pair's LCA runs once. - Bench against project 18 (43,149 occurrences): pre-rework apply_defaults=false curl timed out at 159s; post-rework 1.96s unfiltered / 3.4s with bypass (93,019 occurrences post-filter). Denominator fix (Copilot): - agreed_*_pct now divides by verified_with_prediction_count instead of verified_count. A verified occurrence with no machine prediction can't agree or disagree; including it in the denominator drags the rate down without representing actual model disagreement. - Surface no_prediction_count + verified_with_prediction_count as sibling fields so consumers can see how many such occurrences exist. UNKNOWN rank bug (Copilot): - TaxonRank.UNKNOWN sorts after SPECIES in OrderedEnum definition order, so without explicit exclusion UNKNOWN >= ORDER is True and a shared UNKNOWN ancestor would wrongly count as under-order agreement. Filter UNKNOWN out of lca_rank_between's candidate ranks. Add regression test. Tests: - New: test_unknown_rank_excluded_from_lca (LCA regression) - New: test_agreement_under_order_bucket (HTTP coverage for sister-species case, previously only exact-match shortcut was exercised) - Updated: happy-path asserts verified_with_prediction_count and no_prediction_count. 22/22 backend tests green: docker compose exec django python manage.py test ami.main.tests.TestLcaRankBetween ami.main.tests.TestModelAgreementForProject ami.main.tests.TestOccurrenceStatsViewSet Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 9b70aeb commit ea1e498

6 files changed

Lines changed: 182 additions & 86 deletions

File tree

ami/main/api/serializers.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,19 +1751,34 @@ class TopIdentifiersResponseSerializer(serializers.Serializer):
17511751
top_identifiers = UserIdentificationCountSerializer(many=True)
17521752

17531753

1754-
class HumanModelAgreementSerializer(serializers.Serializer):
1754+
class ModelAgreementSerializer(serializers.Serializer):
17551755
"""Verified / agreement rates over the filtered Occurrence set.
17561756
17571757
`agreed_exact_count` is a subset of `agreed_under_order_count` by
17581758
construction — an exact match implies an LCA at SPECIES, which is
17591759
deeper than ORDER. `*_pct` percentages are 0.0..1.0 (not 0..100).
1760+
1761+
Denominator note: `agreed_*_pct` divide by `verified_with_prediction_count`
1762+
(verified occurrences that *also* have a machine prediction), NOT by
1763+
`verified_count`. A verified occurrence with no machine prediction can't
1764+
agree or disagree — including it in the denominator would drag the rate
1765+
down without representing actual model disagreement. `no_prediction_count`
1766+
is surfaced so the consumer can see how many such occurrences exist.
17601767
"""
17611768

17621769
project_id = serializers.IntegerField()
17631770
total_occurrences = serializers.IntegerField()
1764-
verified_count = serializers.IntegerField()
1771+
verified_count = serializers.IntegerField(help_text="Occurrences with at least one non-withdrawn identification.")
17651772
verified_pct = serializers.FloatField(help_text="verified_count / total_occurrences")
1773+
verified_with_prediction_count = serializers.IntegerField(
1774+
help_text="Verified occurrences that also have a machine prediction (denominator for agreed_*_pct)."
1775+
)
1776+
no_prediction_count = serializers.IntegerField(
1777+
help_text="Verified occurrences with no machine prediction (excluded from agreement denominator)."
1778+
)
17661779
agreed_exact_count = serializers.IntegerField()
1767-
agreed_exact_pct = serializers.FloatField(help_text="agreed_exact_count / verified_count")
1780+
agreed_exact_pct = serializers.FloatField(help_text="agreed_exact_count / verified_with_prediction_count")
17681781
agreed_under_order_count = serializers.IntegerField()
1769-
agreed_under_order_pct = serializers.FloatField(help_text="agreed_under_order_count / verified_count")
1782+
agreed_under_order_pct = serializers.FloatField(
1783+
help_text="agreed_under_order_count / verified_with_prediction_count"
1784+
)

ami/main/api/views.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
from ami.base.views import ProjectMixin
3333
from ami.main.api.schemas import limit_doc_param, project_id_doc_param
3434
from ami.main.api.serializers import TagSerializer
35-
from ami.main.models_future.occurrence import human_model_agreement_for_project, top_identifiers_for_project
35+
from ami.main.models_future.occurrence import model_agreement_for_project, top_identifiers_for_project
3636
from ami.utils.requests import get_default_classification_threshold
3737
from ami.utils.storages import ConnectionTestResult
3838

@@ -71,8 +71,8 @@
7171
EventListSerializer,
7272
EventSerializer,
7373
EventTimelineSerializer,
74-
HumanModelAgreementSerializer,
7574
IdentificationSerializer,
75+
ModelAgreementSerializer,
7676
OccurrenceListSerializer,
7777
OccurrenceSerializer,
7878
PageListSerializer,
@@ -1332,10 +1332,10 @@ def top_identifiers(self, request):
13321332

13331333
@extend_schema(
13341334
parameters=[project_id_doc_param],
1335-
responses=HumanModelAgreementSerializer,
1335+
responses=ModelAgreementSerializer,
13361336
)
1337-
@action(detail=False, methods=["get"], url_path="human-model-agreement")
1338-
def human_model_agreement(self, request):
1337+
@action(detail=False, methods=["get"], url_path="model-agreement")
1338+
def model_agreement(self, request):
13391339
"""Verified / human↔model agreement rates over the filtered occurrence set.
13401340
13411341
Accepts every query param the `/occurrences/` list endpoint accepts.
@@ -1349,9 +1349,9 @@ def human_model_agreement(self, request):
13491349

13501350
base_qs = Occurrence.objects.filter(project=project).valid().apply_default_filters(project, request)
13511351
filtered_qs = self.filter_queryset(base_qs)
1352-
payload = human_model_agreement_for_project(filtered_qs)
1352+
payload = model_agreement_for_project(filtered_qs)
13531353
payload["project_id"] = project.pk
1354-
return Response(HumanModelAgreementSerializer(payload, context={"request": request}).data)
1354+
return Response(ModelAgreementSerializer(payload, context={"request": request}).data)
13551355

13561356

13571357
class TaxonTaxaListFilter(filters.BaseFilterBackend):

ami/main/models_future/occurrence.py

Lines changed: 71 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
from typing import TYPE_CHECKING
1414

15-
from django.db.models import Count, Prefetch, Q, QuerySet
15+
from django.db.models import Count, F, OuterRef, Prefetch, Q, QuerySet, Subquery
1616

1717
from ami.main.models import Project, TaxonRank, User
1818

@@ -30,14 +30,20 @@ def lca_rank_between(a: TaxonTuple, b: TaxonTuple) -> TaxonRank | None:
3030
3131
The taxon itself counts as part of its own ancestor chain — passing the
3232
same taxon twice returns that taxon's rank. Returns ``None`` when the two
33-
chains share no ancestor (e.g. one has an empty parents_json and the other
34-
doesn't include it).
33+
chains share no ancestor at a real taxonomic rank.
34+
35+
``TaxonRank.UNKNOWN`` is excluded from the candidate set even though it
36+
sorts after SPECIES in OrderedEnum definition order — it isn't a real
37+
taxonomic rank and treating it as deeper-than-ORDER produces false
38+
under-order agreements when an UNKNOWN ancestor happens to be shared.
3539
"""
3640
chain_a = [(p["id"], TaxonRank(p["rank"])) for p in a[2]] + [(a[0], TaxonRank(a[1]))]
3741
chain_b_ids = {p["id"] for p in b[2]} | {b[0]}
3842

3943
deepest: TaxonRank | None = None
4044
for tid, rank in chain_a:
45+
if rank == TaxonRank.UNKNOWN:
46+
continue
4147
if tid in chain_b_ids:
4248
if deepest is None or rank > deepest:
4349
deepest = rank
@@ -157,74 +163,83 @@ def detection_image_urls_from_prefetch(occurrence: Occurrence, limit: int | None
157163
return [get_media_url(det.path) for det in detections]
158164

159165

160-
def human_model_agreement_for_project(queryset: QuerySet[Occurrence]) -> dict:
166+
def model_agreement_for_project(queryset: QuerySet[Occurrence]) -> dict:
161167
"""Verified / agreement stats over a pre-filtered Occurrence queryset.
162168
163169
The queryset MUST already be filtered to the project + user-supplied
164170
filters (caller wires apply_default_filters + OccurrenceFilter). This
165-
function adds the prefetches/annotations it needs and returns a dict
166-
matching HumanModelAgreementSerializer's field set (without project_id —
167-
the view layer adds that).
171+
function adds the annotations it needs and returns a dict matching
172+
ModelAgreementSerializer's field set (without project_id — the view
173+
layer adds that).
168174
169175
"Verified" means the occurrence has at least one non-withdrawn
170176
Identification. "Model prediction" means the Classification chosen by
171177
BEST_MACHINE_PREDICTION_ORDER. "Under-order" agreement means the user's
172178
taxon and the model's prediction share an ancestor at rank >= ORDER
173179
(inclusive of ORDER itself).
180+
181+
Aggregation is SQL-side. Only the disagreement set (occurrences where
182+
user and machine disagree at SPECIES) is materialized in Python, and
183+
even then it's deduplicated to distinct (user_taxon, machine_taxon)
184+
pairs so LCA runs once per pair, not once per occurrence.
174185
"""
175-
from ami.main.models import Identification, Taxon
186+
from ami.main.models import BEST_IDENTIFICATION_ORDER, Identification, Taxon
176187

177-
qs = queryset.with_best_machine_prediction().prefetch_related( # type: ignore[attr-defined]
178-
Prefetch(
179-
"identifications",
180-
queryset=Identification.objects.filter(withdrawn=False)
181-
.select_related("taxon")
182-
.order_by("-created_at", "-pk"),
183-
to_attr="_non_withdrawn_idents",
184-
)
188+
best_user_ident = Identification.objects.filter(occurrence=OuterRef("pk"), withdrawn=False).order_by(
189+
*BEST_IDENTIFICATION_ORDER
190+
)
191+
192+
qs = queryset.with_best_machine_prediction().annotate( # type: ignore[attr-defined]
193+
best_user_taxon_id=Subquery(best_user_ident.values("taxon_id")[:1]),
185194
)
186195

187-
occurrences = list(qs)
196+
verified_q = Q(best_user_taxon_id__isnull=False)
197+
has_pred_q = Q(best_machine_prediction_taxon_id__isnull=False)
198+
exact_q = verified_q & has_pred_q & Q(best_user_taxon_id=F("best_machine_prediction_taxon_id"))
188199

189-
needed_taxa_ids: set[int] = set()
190-
for occ in occurrences:
191-
machine_id = getattr(occ, "best_machine_prediction_taxon_id", None)
192-
if machine_id:
193-
needed_taxa_ids.add(machine_id)
194-
idents = getattr(occ, "_non_withdrawn_idents", [])
195-
if idents:
196-
needed_taxa_ids.add(idents[0].taxon_id)
200+
aggregates = qs.aggregate(
201+
total_occurrences=Count("pk"),
202+
verified_count=Count("pk", filter=verified_q),
203+
verified_with_prediction_count=Count("pk", filter=verified_q & has_pred_q),
204+
no_prediction_count=Count("pk", filter=verified_q & ~has_pred_q),
205+
agreed_exact_count=Count("pk", filter=exact_q),
206+
)
207+
208+
# Under-order: only the disagreement set hits Python, grouped by distinct
209+
# (user_taxon, machine_taxon) pair so each pair's LCA is computed once.
210+
disagreement_pairs = (
211+
qs.filter(verified_q & has_pred_q)
212+
.exclude(best_user_taxon_id=F("best_machine_prediction_taxon_id"))
213+
.values("best_user_taxon_id", "best_machine_prediction_taxon_id")
214+
.annotate(occurrence_count=Count("pk"))
215+
)
216+
217+
pairs = list(disagreement_pairs)
218+
needed_taxa_ids = {p["best_user_taxon_id"] for p in pairs} | {p["best_machine_prediction_taxon_id"] for p in pairs}
197219

198220
taxa_by_id: dict[int, TaxonTuple] = {}
199-
for t in Taxon.objects.filter(pk__in=needed_taxa_ids):
200-
parents = [{"id": p.id, "rank": p.rank.name if hasattr(p.rank, "name") else p.rank} for p in t.parents_json]
201-
taxa_by_id[t.pk] = (t.pk, t.rank, parents)
202-
203-
total = len(occurrences)
204-
verified = 0
205-
agreed_exact = 0
206-
agreed_under_order = 0
207-
208-
for occ in occurrences:
209-
idents = getattr(occ, "_non_withdrawn_idents", [])
210-
if not idents:
211-
continue
212-
verified += 1
213-
user_taxon_id = idents[0].taxon_id
214-
machine_taxon_id = getattr(occ, "best_machine_prediction_taxon_id", None)
215-
if not machine_taxon_id or not user_taxon_id:
221+
if needed_taxa_ids:
222+
for t in Taxon.objects.filter(pk__in=needed_taxa_ids):
223+
parents = [
224+
{"id": p.id, "rank": p.rank.name if hasattr(p.rank, "name") else p.rank} for p in t.parents_json
225+
]
226+
taxa_by_id[t.pk] = (t.pk, t.rank, parents)
227+
228+
under_order_disagreement_count = 0
229+
for pair in pairs:
230+
u = taxa_by_id.get(pair["best_user_taxon_id"])
231+
m = taxa_by_id.get(pair["best_machine_prediction_taxon_id"])
232+
if not u or not m:
216233
continue
217-
if user_taxon_id == machine_taxon_id:
218-
agreed_exact += 1
219-
agreed_under_order += 1
220-
continue
221-
user_tuple = taxa_by_id.get(user_taxon_id)
222-
machine_tuple = taxa_by_id.get(machine_taxon_id)
223-
if not user_tuple or not machine_tuple:
224-
continue
225-
lca = lca_rank_between(user_tuple, machine_tuple)
234+
lca = lca_rank_between(u, m)
226235
if lca is not None and lca >= TaxonRank.ORDER:
227-
agreed_under_order += 1
236+
under_order_disagreement_count += pair["occurrence_count"]
237+
238+
agreed_exact = aggregates["agreed_exact_count"]
239+
agreed_under_order = agreed_exact + under_order_disagreement_count
240+
total = aggregates["total_occurrences"]
241+
verified = aggregates["verified_count"]
242+
verified_with_pred = aggregates["verified_with_prediction_count"]
228243

229244
def _pct(num: int, denom: int) -> float:
230245
return round(num / denom, 4) if denom else 0.0
@@ -233,10 +248,12 @@ def _pct(num: int, denom: int) -> float:
233248
"total_occurrences": total,
234249
"verified_count": verified,
235250
"verified_pct": _pct(verified, total),
251+
"verified_with_prediction_count": verified_with_pred,
252+
"no_prediction_count": aggregates["no_prediction_count"],
236253
"agreed_exact_count": agreed_exact,
237-
"agreed_exact_pct": _pct(agreed_exact, verified),
254+
"agreed_exact_pct": _pct(agreed_exact, verified_with_pred),
238255
"agreed_under_order_count": agreed_under_order,
239-
"agreed_under_order_pct": _pct(agreed_under_order, verified),
256+
"agreed_under_order_pct": _pct(agreed_under_order, verified_with_pred),
240257
}
241258

242259

ami/main/tests.py

Lines changed: 68 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4771,8 +4771,36 @@ def test_no_shared_ancestor_returns_none(self):
47714771
rank = lca_rank_between(rootless, self.SPECIES_NOCTUA_PRONUBA)
47724772
self.assertIsNone(rank)
47734773

4774+
def test_unknown_rank_excluded_from_lca(self):
4775+
"""TaxonRank.UNKNOWN sorts after SPECIES in OrderedEnum definition order,
4776+
so without explicit exclusion `UNKNOWN >= ORDER` would be True and a
4777+
shared UNKNOWN ancestor would wrongly count as under-order agreement.
4778+
"""
4779+
from ami.main.models_future.occurrence import lca_rank_between
4780+
4781+
# Both chains share a KINGDOM ancestor and an UNKNOWN ancestor; the LCA
4782+
# at a real taxonomic rank is KINGDOM, not UNKNOWN.
4783+
unknown_a = (
4784+
701,
4785+
"SPECIES",
4786+
[
4787+
{"id": 1, "rank": "KINGDOM"},
4788+
{"id": 999, "rank": "UNKNOWN"},
4789+
],
4790+
)
4791+
unknown_b = (
4792+
702,
4793+
"SPECIES",
4794+
[
4795+
{"id": 1, "rank": "KINGDOM"},
4796+
{"id": 999, "rank": "UNKNOWN"},
4797+
],
4798+
)
4799+
rank = lca_rank_between(unknown_a, unknown_b)
4800+
self.assertEqual(rank, TaxonRank.KINGDOM)
47744801

4775-
class TestHumanModelAgreementForProject(APITestCase):
4802+
4803+
class TestModelAgreementForProject(APITestCase):
47764804
"""Aggregation function over a filtered Occurrence queryset.
47774805
47784806
Covers the four bucket transitions: unverified, verified+exact-agreed,
@@ -4806,18 +4834,18 @@ def _identify(self, occurrence: Occurrence, taxon: Taxon) -> Identification:
48064834
return Identification.objects.create(user=self.user, occurrence=occurrence, taxon=taxon)
48074835

48084836
def test_empty_project_returns_zeros_not_nans(self):
4809-
from ami.main.models_future.occurrence import human_model_agreement_for_project
4837+
from ami.main.models_future.occurrence import model_agreement_for_project
48104838

48114839
empty_project = Project.objects.create(name="empty")
4812-
result = human_model_agreement_for_project(Occurrence.objects.filter(project=empty_project))
4840+
result = model_agreement_for_project(Occurrence.objects.filter(project=empty_project))
48134841
self.assertEqual(result["total_occurrences"], 0)
48144842
self.assertEqual(result["verified_count"], 0)
48154843
self.assertEqual(result["verified_pct"], 0.0)
48164844
self.assertEqual(result["agreed_exact_pct"], 0.0)
48174845
self.assertEqual(result["agreed_under_order_pct"], 0.0)
48184846

48194847
def test_buckets_canonical_cases(self):
4820-
from ami.main.models_future.occurrence import human_model_agreement_for_project
4848+
from ami.main.models_future.occurrence import model_agreement_for_project
48214849

48224850
occurrences = list(Occurrence.objects.filter(project=self.project).order_by("pk"))
48234851
self.assertEqual(len(occurrences), 4)
@@ -4829,7 +4857,7 @@ def test_buckets_canonical_cases(self):
48294857
self._identify(occurrences[2], self.pieris_brassicae)
48304858
# 3: unverified
48314859

4832-
result = human_model_agreement_for_project(Occurrence.objects.filter(project=self.project))
4860+
result = model_agreement_for_project(Occurrence.objects.filter(project=self.project))
48334861
self.assertEqual(result["total_occurrences"], 4)
48344862
self.assertEqual(result["verified_count"], 3)
48354863
self.assertEqual(result["agreed_exact_count"], 1)
@@ -4922,9 +4950,9 @@ def test_registration_order_preserves_occurrence_retrieve(self):
49224950
self.assertEqual(stats_response.status_code, 200, "stats URL must resolve")
49234951
self.assertEqual(retrieve_response.status_code, 200, "occurrence retrieve must still work")
49244952

4925-
# ----- /occurrences/stats/human-model-agreement/ -----
4953+
# ----- /occurrences/stats/model-agreement/ -----
49264954

4927-
agreement_url = "/api/v2/occurrences/stats/human-model-agreement/"
4955+
agreement_url = "/api/v2/occurrences/stats/model-agreement/"
49284956

49294957
def test_agreement_no_project_id_returns_400(self):
49304958
response = self.client.get(self.agreement_url)
@@ -4965,9 +4993,42 @@ def test_agreement_happy_path(self):
49654993
body = response.json()
49664994
self.assertEqual(body["total_occurrences"], 4)
49674995
self.assertEqual(body["verified_count"], 1)
4996+
self.assertEqual(body["verified_with_prediction_count"], 1)
4997+
self.assertEqual(body["no_prediction_count"], 0)
49684998
self.assertEqual(body["agreed_exact_count"], 1)
49694999
self.assertEqual(body["agreed_under_order_count"], 1)
49705000

5001+
def test_agreement_under_order_bucket(self):
5002+
"""Disagreement at species but same genus → counted under-order, not exact.
5003+
5004+
Pick the machine prediction's sister species (same parent genus) for the
5005+
identification. LCA between the two species is GENUS, which is >= ORDER,
5006+
so the occurrence falls into the under-order bucket without contributing
5007+
to agreed_exact_count.
5008+
"""
5009+
occurrence = Occurrence.objects.filter(project=self.project).order_by("pk").first()
5010+
machine_taxon = occurrence.detections.first().classifications.first().taxon
5011+
# Sister species: same parent (genus Vanessa), different SPECIES.
5012+
sister = (
5013+
Taxon.objects.filter(parent=machine_taxon.parent, rank=TaxonRank.SPECIES.name)
5014+
.exclude(pk=machine_taxon.pk)
5015+
.first()
5016+
)
5017+
self.assertIsNotNone(sister, "Test fixture must have a sister species under the same genus")
5018+
Taxon.objects.update_all_parents()
5019+
Identification.objects.create(user=self.alice, occurrence=occurrence, taxon=sister)
5020+
5021+
response = self.client.get(f"{self.agreement_url}?project_id={self.project.pk}")
5022+
self.assertEqual(response.status_code, 200)
5023+
body = response.json()
5024+
self.assertEqual(body["verified_count"], 1)
5025+
self.assertEqual(body["verified_with_prediction_count"], 1)
5026+
self.assertEqual(body["agreed_exact_count"], 0)
5027+
self.assertEqual(body["agreed_under_order_count"], 1)
5028+
# 0/1 exact, 1/1 under-order
5029+
self.assertEqual(body["agreed_exact_pct"], 0.0)
5030+
self.assertEqual(body["agreed_under_order_pct"], 1.0)
5031+
49715032
def test_agreement_filter_passthrough(self):
49725033
"""`?deployment=` should narrow the set."""
49735034
other_deployment = Deployment.objects.create(name="other", project=self.project)

docs/claude/reference/api-stats-pattern.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ into pagination only if the kind genuinely needs it):
232232

233233
- `GET /occurrences/stats/top-identifiers/` — done (this PR)
234234
- `GET /occurrences/stats/identifications-summary/` — total / distinct / verified counts
235-
- `GET /occurrences/stats/human-model-agreement/` — model agreement rate
235+
- `GET /occurrences/stats/model-agreement/` — model agreement rate
236236
- `GET /occurrences/stats/identifications-by-species/` — per-taxon ID counts
237237
- `GET /occurrences/stats/timeline/` — Plotly-shaped time series
238238
- `GET /deployments/stats/processed-images/` — processed images per station

0 commit comments

Comments
 (0)