feat: change human scores to None, add prediction and identification fields to CSV export#1214
Open
feat: change human scores to None, add prediction and identification fields to CSV export#1214
Conversation
…elds Separate machine predictions from human identifications in exports and API, so researchers see both side-by-side. Previously the determination was overwritten when a human verified, losing the original ML prediction. Model layer: - Extract find_best_prediction() and find_best_identification() from update_occurrence_determination() for reuse by exports and API - Set determination_score to None for human-determined occurrences (ML confidence preserved in best_machine_prediction_score) New CSV export fields: - best_machine_prediction_name, _algorithm, _score - verified_by, verified_by_count, agreed_with_algorithm - determination_matches_machine_prediction - best_detection_bbox, best_detection_source_image_url, best_detection_occurrence_url API changes: - Add best_machine_prediction nested object to OccurrenceListSerializer (always populated regardless of verification status) Closes #1213 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for antenna-ssec canceled.
|
✅ Deploy Preview for antenna-preview canceled.
|
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 59 minutes and 20 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds machine-prediction, verification, and best-detection annotations and fields to Occurrence exports and API; refactors determination logic to preserve ML scores, adds queryset annotation helpers, a backfill command, and tests exercising new export/management behaviors. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant ExportAPI as Export Request
participant Serializer as OccurrenceTabularSerializer
participant QuerySet as OccurrenceQuerySet
participant DB as Database
participant Occurrence as Occurrence instance
Client->>ExportAPI: Request CSV export
ExportAPI->>QuerySet: build queryset (.with_best_detection(), .with_best_machine_prediction(), .with_verification_info())
QuerySet->>DB: execute annotated query
DB-->>QuerySet: annotated rows
QuerySet-->>Serializer: rows
Serializer->>Occurrence: get_best_detection_source_image_url(obj), get_verified_by(obj), get_determination_matches_machine_prediction(obj)
Occurrence-->>Serializer: computed field values (from annotations / methods)
Serializer-->>Client: CSV with ML prediction, verification, detection fields
sequenceDiagram
actor Client
participant API as OccurrenceListSerializer
participant Occ as Occurrence.find_best_prediction()
participant Classification as Classification (ML)
participant Taxon
participant Algorithm
Client->>API: GET /occurrences/
API->>Occ: call find_best_prediction()
Occ->>Classification: select best (terminal first, highest score)
Classification-->>Occ: best classification (or null)
alt prediction exists
API->>Taxon: serialize prediction.taxon
API->>Algorithm: serialize prediction.algorithm
API-->>Client: {taxon, algorithm, score, determination_matches_machine_prediction}
else no prediction
API-->>Client: null for best_machine_prediction
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates occurrence exports and the API to preserve and expose both machine predictions and human verifications side-by-side, preventing ML predictions from being lost when a human identification is added.
Changes:
- Refactors occurrence logic to expose reusable “best prediction” and “best identification” selection methods and adjusts determination score semantics for human IDs.
- Extends CSV exports with new machine prediction, verification, and detection-related fields backed by queryset annotations.
- Adds a
best_machine_predictionnested object to the occurrences list API serializer.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
ami/main/models.py |
Adds queryset annotations for best detection/prediction/verification and refactors best prediction/identification selection + determination score update logic. |
ami/main/api/serializers.py |
Adds best_machine_prediction field to occurrence list API responses. |
ami/exports/format_types.py |
Adds new CSV export fields and wires exporter queryset to include new annotations. |
ami/exports/tests.py |
Adds tests covering the new CSV export fields across ML-only and human verification scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ami/exports/format_types.py`:
- Around line 192-204: Use the annotated winning detection already attached to
the exported object instead of recomputing it: in
get_best_detection_source_image_url and get_best_detection_occurrence_url, read
the annotated detection from the obj (e.g. getattr(obj, "best_detection", None)
or the specific annotated attributes like
best_detection_source_image_path/public_base_url on that detection) and build
both the source image URL and the occurrence/context URL from that detection's
stored fields (path, public_base_url or signed_url, and occurrence/context link)
rather than calling obj.context_url() which re-queries and may use a different
ordering; this ensures URLs correspond to the exported bbox/path and avoids N+1
queries.
In `@ami/main/api/serializers.py`:
- Around line 1396-1421: get_best_machine_prediction currently calls
obj.find_best_prediction() and directly accesses prediction.taxon and
prediction.algorithm, causing an N+1; change it to reuse obj.best_prediction
(use that if present, fall back to obj.find_best_prediction() only if needed)
and avoid dereferencing relations that aren't prefetched—either use the
already-attached taxon/algorithm on the prediction object or ensure these are
provided via queryset annotations/prefetch_related and passed through context to
TaxonNestedSerializer/AlgorithmNestedSerializer to prevent per-row queries;
update get_best_machine_prediction to check obj.best_prediction first and only
call find_best_prediction as a fallback, and document that views/queries should
prefetch prediction__taxon and prediction__algorithm.
In `@ami/main/models.py`:
- Around line 3210-3218: The code only sets new_score when new_determination
changes, so when authority flips but the taxon stays the same we never recompute
determination_score; update the logic around
occurrence.find_best_identification() and occurrence.find_best_prediction()
(references: top_identification, top_prediction, new_determination, new_score,
determination_score) so that whenever the authority changes you still set
new_score appropriately (None for a human top_identification,
top_prediction.score for an ML top_prediction) and assign determination_score on
the occurrence even if the taxon equals current_determination; apply the same
fix in the analogous block handling lines ~3225-3228.
- Around line 2864-2867: The aggregate verified_by_count is over-counting
because Count("identifications") is performed across detection rows; change the
Count to be distinct so each identification is only counted once (e.g. use
Count("identifications", distinct=True,
filter=Q(identifications__withdrawn=False))). Update the verified_by_count
definition (the Count call) to include distinct=True while keeping the existing
filter on identifications.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca26e0c4-7c2a-409c-8ae0-af43172af29c
📒 Files selected for processing (4)
ami/exports/format_types.pyami/exports/tests.pyami/main/api/serializers.pyami/main/models.py
- Add distinct=True to verified_by_count to prevent join multiplication - Fix update_occurrence_determination to recompute score even when taxon stays the same (handles authority flip without taxon change) - Remove email fallback in verified_by (PII concern, name-only) - Filter withdrawn identifications in verification_status fallback - Use obj.best_prediction cached property instead of find_best_prediction() in API serializer to avoid N+1 queries - Build occurrence URL from annotated fields instead of context_url() to avoid N+1 queries in export - Use source_image.timestamp instead of datetime.now() in tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mihow
commented
Apr 8, 2026
Member
|
From reading the PR description, this looks great Michael! Some comments and questions:
|
Collaborator
Author
|
Very astute observations @annavik !
That makes sense! Although I am leaning away from including this count at all, since we haven't talked about verification at different ranks or disagreements. I realized what I want here is just "participant_count" to highlight if more than one user son is in the identification history. In general, I want to stick with the assumption that the platform is for experts to ID and the last expert gets the word -- like a museum collection, rather than a community ID platform.
Great question! Probably yes! This would require additional changes (I am just exposing this value in this PR, not calculating it). Maybe an easy change though.
I agree it's too soon! Will remove.
I like this one since it's based on the taxon ID rather than string matching, and I think it will be a common use case. |
…urrence URL Per @annavik and @mihow's review: - Rename `verified_by_count` to `participant_count` (counts distinct users, not total IDs) - Remove `best_detection_occurrence_url` (URLs not stable enough to distribute in exports yet) - Clean up unused annotations (event_id, source_image_id) from with_best_detection() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ami/exports/format_types.py (1)
112-145:⚠️ Potential issue | 🟠 MajorDon’t bake the preview frontend domain into exported CSVs.
best_detection_occurrence_urlis hardcoded to the preview app, so exported files become wrong as soon as the frontend base URL differs by environment or changes later. That makes the new column non-portable for exactly the reason raised in the PR discussion.Either drop this column or build it from a configured frontend base URL instead of a literal hostname.
Also applies to: 199-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ami/exports/format_types.py` around lines 112 - 145, The serializer currently hardcodes the preview frontend domain for the best_detection_occurrence_url column (the SerializerMethodField best_detection_occurrence_url and its getter), making exported CSVs non-portable; either remove best_detection_occurrence_url from the serializer fields, or change the getter (e.g., get_best_detection_occurrence_url) to construct the link from a configurable frontend base URL (read from settings or an env var like FRONTEND_BASE_URL) and fall back to None if not set; update the Meta.fields list and the corresponding method(s) (also referenced in the same file around the other similar fields at the later occurrence) so no literal hostname is embedded.ami/main/models.py (1)
3215-3242:⚠️ Potential issue | 🟠 MajorCompare determination IDs and allow clearing stale determinations.
The fallback query at Lines 3215-3220 returns a raw
determinationid, whilenew_determinationis aTaxoninstance. Combined with theif new_determination ...guard, unchanged rows look changed, and rows with no remaining identification/prediction never clear an old saved determination.Suggested fix
- current_determination = ( - current_determination - or Occurrence.objects.select_related("determination") - .values("determination") - .get(pk=occurrence.pk)["determination"] - ) + current_determination_id = getattr(current_determination, "pk", current_determination) + if current_determination_id is None: + current_determination_id = ( + Occurrence.objects.values_list("determination_id", flat=True).get(pk=occurrence.pk) + ) new_determination = None new_score = None @@ - if new_determination and new_determination != current_determination: + new_determination_id = new_determination.pk if new_determination else None + if new_determination_id != current_determination_id: logger.debug(f"Changing det. of {occurrence} from {current_determination} to {new_determination}") occurrence.determination = new_determination needs_update = True🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ami/main/models.py` around lines 3215 - 3242, The bug is that current_determination is being read as a raw determination id while new_determination is a Taxon instance, causing false positives and preventing clearing of stale determinations; fix by normalizing types before comparison: either load the actual Taxon object for current_determination (e.g. use Occurrence.objects.select_related("determination").get(pk=occurrence.pk).determination) or compare IDs consistently (e.g. get current_determination_id via .values_list("determination", flat=True) and compare to new_determination.pk), and ensure when no identification/prediction remains you set occurrence.determination = None (and update determination_score) so stale determinations are cleared; update the comparisons around current_determination, new_determination, and occurrence.determination_score accordingly (references: current_determination, new_determination, occurrence.find_best_identification, occurrence.find_best_prediction, determination_score).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ami/exports/tests.py`:
- Line 376: The unpacking at the call to _create_occurrence_with_prediction
currently assigns unused variables occurrence and classification (e.g.,
"occurrence, classification = self._create_occurrence_with_prediction()") which
triggers RUF059; change the bindings to use the underscore discard pattern
(e.g., "_" or "_classification") for both returned values at each call site
(including the other similar call around line 417) or assign only the needed
value via a single-variable assignment from the function result, updating
references accordingly so unused names are removed.
In `@ami/main/api/serializers.py`:
- Around line 1396-1421: The function get_best_machine_prediction currently
returns None when obj.best_prediction is missing but the API contract requires
the nested object to always be present; change the early-return to return a dict
with the expected keys populated with nullable defaults instead. Specifically,
inside get_best_machine_prediction (and using obj.best_prediction,
TaxonNestedSerializer, AlgorithmNestedSerializer as anchors), always return a
dict with keys taxon, algorithm, score, determination_matches_machine_prediction
where taxon and algorithm are None if no prediction, score is None, and
determination_matches_machine_prediction is None (or False if you prefer
boolean) when no prediction exists; keep the existing logic that fills these
fields when prediction is present.
In `@ami/main/models.py`:
- Around line 3078-3105: find_best_prediction currently calls self.predictions()
which applies per-algorithm max-score filtering and causes a different result
than OccurrenceQuerySet.with_best_machine_prediction(); change
find_best_prediction to mirror with_best_machine_prediction by querying
Classification objects for this occurrence directly (not via predictions()),
ordering by "-terminal", "-score" across all classifications, and returning the
first result so the API/CSV and this method pick the same best machine
prediction; update any references to find_best_prediction/best_prediction if
needed to rely on the new query behavior.
---
Outside diff comments:
In `@ami/exports/format_types.py`:
- Around line 112-145: The serializer currently hardcodes the preview frontend
domain for the best_detection_occurrence_url column (the SerializerMethodField
best_detection_occurrence_url and its getter), making exported CSVs
non-portable; either remove best_detection_occurrence_url from the serializer
fields, or change the getter (e.g., get_best_detection_occurrence_url) to
construct the link from a configurable frontend base URL (read from settings or
an env var like FRONTEND_BASE_URL) and fall back to None if not set; update the
Meta.fields list and the corresponding method(s) (also referenced in the same
file around the other similar fields at the later occurrence) so no literal
hostname is embedded.
In `@ami/main/models.py`:
- Around line 3215-3242: The bug is that current_determination is being read as
a raw determination id while new_determination is a Taxon instance, causing
false positives and preventing clearing of stale determinations; fix by
normalizing types before comparison: either load the actual Taxon object for
current_determination (e.g. use
Occurrence.objects.select_related("determination").get(pk=occurrence.pk).determination)
or compare IDs consistently (e.g. get current_determination_id via
.values_list("determination", flat=True) and compare to new_determination.pk),
and ensure when no identification/prediction remains you set
occurrence.determination = None (and update determination_score) so stale
determinations are cleared; update the comparisons around current_determination,
new_determination, and occurrence.determination_score accordingly (references:
current_determination, new_determination, occurrence.find_best_identification,
occurrence.find_best_prediction, determination_score).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b1730fd-0066-43a0-8e14-4c23b916b341
📒 Files selected for processing (4)
ami/exports/format_types.pyami/exports/tests.pyami/main/api/serializers.pyami/main/models.py
Collaborator
Author
|
Claude says: Thanks for the great feedback @annavik! Addressed in 66d9390:
|
Exposes the email of the prior human identifier when the best identification was explicitly an "Agree" with another user. Lets consumers trace agreement chains without the backend having to resolve transitivity. Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
Author
|
Claude says: Added While implementing this I noticed a pre-existing UI bug where agreeing with a human identification from the occurrence details card sends the wrong FK type. Filed separately as #1226 — it means |
mihow
changed the title
- Add `backfill_determination_score` management command to null-out determination_score on legacy human-determined occurrences. Supports --dry-run and --project filtering. - Tighten the legacy score-backfill block in Occurrence.save() to skip human-determined occurrences (avoids a redundant query and suppresses the misleading "Could not determine score" warning). - Fix type mismatch in update_occurrence_determination(): when the caller didn't pass current_determination, the fallback fetched the FK id (int) and compared it against a Taxon instance, which was always unequal and caused unnecessary save() calls and misleading debug logs. Normalize current_determination to an id for the comparison and accept int|Taxon|None. Tests: BackfillDeterminationScoreTest covers the ML-only, human-determined, and withdrawn-identification cases. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
ami/exports/tests.py (1)
376-376:⚠️ Potential issue | 🟡 MinorFix unused unpacked variable to avoid RUF059.
At Line [376] and Line [417],
classificationis assigned but never used.Suggested fix
- occurrence, classification = self._create_occurrence_with_prediction() + occurrence, _classification = self._create_occurrence_with_prediction() ... - occurrence, classification = self._create_occurrence_with_prediction(taxon=self.taxon_a) + occurrence, _classification = self._create_occurrence_with_prediction(taxon=self.taxon_a)Also applies to: 417-417
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ami/exports/tests.py` at line 376, The test assigns an unused second value from self._create_occurrence_with_prediction() to classification causing RUF059; update both call sites where you do "occurrence, classification = self._create_occurrence_with_prediction()" (and the duplicate) to ignore the unused value by renaming it to "_" or "_classification" (e.g., "occurrence, _ = self._create_occurrence_with_prediction()") so the returned but unused classification is not flagged.ami/main/models.py (1)
3067-3075:⚠️ Potential issue | 🟠 MajorAlign
find_best_prediction()with the export/API query.At Line 3075 this still starts from
self.predictions(), which pre-filters to per-algorithm max scores.OccurrenceQuerySet.with_best_machine_prediction()in the same file orders across all classifications, so the model method can disagree with the exportedbest_machine_prediction_*fields for the same occurrence.Suggested fix
def find_best_prediction(self) -> "Classification | None": """ Find the best machine prediction for this occurrence. @@ Uses the highest scoring classification (from any algorithm) as the best prediction. Considers terminal classifications first, then non-terminal ones. (Terminal classifications are the final classifications of a pipeline, non-terminal are intermediate models.) """ - return self.predictions().order_by("-terminal", "-score").first() + return Classification.objects.filter(detection__occurrence=self).order_by("-terminal", "-score").first()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ami/main/models.py` around lines 3067 - 3075, The method find_best_prediction() currently calls self.predictions(), which applies the per-algorithm max-score prefilter and can disagree with OccurrenceQuerySet.with_best_machine_prediction() that considers all classifications; update find_best_prediction() to query all Classification rows for this occurrence (e.g., via the Classification model filter by occurrence=self or the raw reverse relation like self.classifications.all()) and then order by "-terminal", "-score" to pick the single best prediction so it matches the logic used in with_best_machine_prediction().
🧹 Nitpick comments (1)
ami/exports/tests.py (1)
465-465: Update docstring to match renamed field semantics.The text still mentions
verified_by_count; tests now validateparticipant_count(distinct participants), so this wording is stale.Suggested fix
- """Multiple identifications: verified_by_count reflects all non-withdrawn IDs.""" + """Multiple identifications: participant_count reflects distinct non-withdrawn users."""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ami/exports/tests.py` at line 465, Update the test docstring to reflect the renamed field: replace references to "verified_by_count" with "participant_count" and adjust the description to state that participant_count reflects the number of distinct non-withdrawn participants (or similar wording used elsewhere in the codebase). Locate the docstring in ami/exports/tests.py (the triple-quoted string currently reading "Multiple identifications: verified_by_count reflects all non-withdrawn IDs.") and change it to mention participant_count and distinct participants so the docstring matches the assertions in the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ami/exports/format_types.py`:
- Around line 163-165: The method get_agreed_with_user currently returns the
collaborator's email (agreed_with_user_email) which leaks PII; change it to
return a non-PII identifier or display name instead (e.g., agreed_with_user_id
or agreed_with_user_display_name) by reading the corresponding attribute from
obj and falling back to None if absent—update get_agreed_with_user to use that
non-PII field rather than agreed_with_user_email and ensure callers/CSV exporter
expect the new identifier.
In `@ami/main/models.py`:
- Around line 3224-3229: The current guard only sets occurrence.determination
when new_determination is truthy, leaving a stale determination when everything
authoritative is removed; update the logic around
new_determination/current_determination_id so that if new_determination is None
but current_determination_id is set you explicitly clear
occurrence.determination (set to None), mark needs_update = True, and log the
change (similar to the existing logger.debug), otherwise keep the existing
assignment when new_determination differs; touch the block that references
new_determination, occurrence.determination, current_determination_id, and
needs_update to implement this behavior.
---
Duplicate comments:
In `@ami/exports/tests.py`:
- Line 376: The test assigns an unused second value from
self._create_occurrence_with_prediction() to classification causing RUF059;
update both call sites where you do "occurrence, classification =
self._create_occurrence_with_prediction()" (and the duplicate) to ignore the
unused value by renaming it to "_" or "_classification" (e.g., "occurrence, _ =
self._create_occurrence_with_prediction()") so the returned but unused
classification is not flagged.
In `@ami/main/models.py`:
- Around line 3067-3075: The method find_best_prediction() currently calls
self.predictions(), which applies the per-algorithm max-score prefilter and can
disagree with OccurrenceQuerySet.with_best_machine_prediction() that considers
all classifications; update find_best_prediction() to query all Classification
rows for this occurrence (e.g., via the Classification model filter by
occurrence=self or the raw reverse relation like self.classifications.all()) and
then order by "-terminal", "-score" to pick the single best prediction so it
matches the logic used in with_best_machine_prediction().
---
Nitpick comments:
In `@ami/exports/tests.py`:
- Line 465: Update the test docstring to reflect the renamed field: replace
references to "verified_by_count" with "participant_count" and adjust the
description to state that participant_count reflects the number of distinct
non-withdrawn participants (or similar wording used elsewhere in the codebase).
Locate the docstring in ami/exports/tests.py (the triple-quoted string currently
reading "Multiple identifications: verified_by_count reflects all non-withdrawn
IDs.") and change it to mention participant_count and distinct participants so
the docstring matches the assertions in the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6709b2e-0997-453a-bfba-a309722205ef
📒 Files selected for processing (5)
ami/exports/format_types.pyami/exports/tests.pyami/main/management/commands/backfill_determination_score.pyami/main/models.pyami/main/tests.py
…otation
The API's cached `best_prediction` and the CSV export's annotated
`best_machine_prediction_*` fields could pick different rows for the same
occurrence:
- `find_best_prediction()` went through `self.predictions()`, which filters to
the max-score row per algorithm **before** applying the `-terminal, -score`
ordering. A high-score non-terminal could survive dedup and then lose to a
lower-score terminal on ordering.
- `with_best_machine_prediction()` orders directly over all classifications, so
it picks the terminal winner regardless of intra-algorithm dedup.
Align `find_best_prediction()` to query classifications directly with the same
`-terminal, -score, -pk` ordering. Add `-pk` to both so ties break
deterministically.
Also:
- Serializer `get_best_machine_prediction()` now returns a stable shape
({taxon: null, algorithm: null, score: null, ...}) when no prediction exists,
instead of null — clients read nullable fields instead of branching on type.
- `update_occurrence_determination()` now clears `occurrence.determination` when
the last non-withdrawn identification is deleted and no prediction remains,
instead of leaving a stale taxon.
Tests: new `test_api_and_csv_pick_same_best_prediction_with_mixed_terminal`
covers the divergence case.
Co-Authored-By: Claude <noreply@anthropic.com>
- Extract BEST_MACHINE_PREDICTION_ORDER as a shared constant used by both Occurrence.find_best_prediction() (row-at-a-time) and OccurrenceQuerySet.with_best_machine_prediction() (bulk annotation). Both methods now reference the constant and each other's docstrings, so future edits to the ordering touch one place and can't silently diverge. - Revert get_best_machine_prediction() to return None when no prediction exists (it was briefly returning a dict of nulls). Annotate the method's return type as `dict | None` so drf-spectacular emits nullability. - Add UpdateOccurrenceDeterminationTest covering the stale-determination clearing fix: withdrawing the only identification on an occurrence with no predictions now clears occurrence.determination (previously left a stale taxon in place). Co-Authored-By: Claude <noreply@anthropic.com>
mihow
changed the title
mihow
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Separates machine predictions from human identifications in exports and API, so researchers see both side-by-side. Previously the
determinationwas overwritten when a human verified, losing the original ML prediction.find_best_prediction()andfind_best_identification()fromupdate_occurrence_determination()for reuse by exports and APIdetermination_scoretoNonefor human-determined occurrences (ML confidence preserved inbest_machine_prediction_score)best_machine_predictionnested object to the APIbackfill_determination_scoremanagement command to null-out scores on legacy human-determined occurrences (supports--dry-runand--project)update_occurrence_determination()— the fallback path was comparing a Taxon instance to an FK id (int), causing needless save() churnOccurrence.save()to skip human-determined occurrences (suppresses a misleading "Could not determine score" warning)distinct=Trueon counts, PII removal, withdrawn ID filtering, timezone-safe testsverified_by_count→participant_count(distinct users), removedbest_detection_occurrence_url(URLs not stable enough yet)Closes #1213
New CSV export columns
best_machine_prediction_nameIdia aemulabest_machine_prediction_algorithmmoth-classifier-v2best_machine_prediction_score0.881verified_byJane Smithparticipant_count2agreed_with_algorithmmoth-classifier-v2agreed_with_userjane@example.orgdetermination_matches_machine_predictionTruebest_detection_bbox[0.1, 0.1, 0.5, 0.5]best_detection_source_image_urlhttps://s3.../image.jpgConsumers that want to follow transitive agreement chains (B agreed with A, A agreed with ML) can do so by chaining
agreed_with_useracross rows themselves — the backend exposes the direct link only.Example CSV rows
ML prediction only (no human verification):
Human agrees with ML:
Human disagrees with ML:
Human agrees with another human:
New API field:
best_machine_predictionAdded to
OccurrenceListSerializer— always populated regardless of verification status:{ "id": 993, "determination": {"id": 4205, "name": "Catocala relicta"}, "determination_score": null, "best_machine_prediction": { "taxon": {"id": 4205, "name": "Idia aemula"}, "algorithm": {"id": 3, "name": "moth-classifier-v2"}, "score": 0.881, "determination_matches_machine_prediction": false }, "identifications": [...] }Backfill for existing data
Run after deploy to clear
determination_scoreon human-determined occurrences that predate this change:Test plan
BackfillDeterminationScoreTestcovers dry-run, ML-only preserved, human-determined cleared, withdrawn-id ignoredFollow-up
ami/main/checks.py(see feat: add check_occurrences for occurrence data integrity #1188)Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores