Default taxa filter follow-up: handle cached counts and add default taxa to projects #1045
Conversation
✅ Deploy Preview for antenna-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThis PR introduces configurable default filter application throughout the codebase. It adds environment-backed taxa inclusion/exclusion settings, makes default filters conditionally applicable in viewsets and query builders, implements per-model detection count methods that respect project filters, and establishes signal-driven cache invalidation when filters change. Changes
Sequence DiagramsequenceDiagram
actor Client
participant ViewSet as TaxonViewSet
participant Filters as build_occurrence_<br/>default_filters_q()
participant Models as Deployment/<br/>Event Model
participant Signals as Signal Handlers
Client->>ViewSet: retrieve(project_id)
ViewSet->>ViewSet: get_queryset()
ViewSet->>ViewSet: get_taxa_observed(apply_default_score_filter=True,<br/>apply_default_taxa_filter=False)
ViewSet->>Filters: build_occurrence_default_filters_q(<br/>apply_default_score_filter=True,<br/>apply_default_taxa_filter=False)
Note over Filters: Apply score filter<br/>Skip taxa filter
Filters-->>ViewSet: filtered Q object
ViewSet-->>Client: taxa queryset
Note over Signals: Separate Flow: When Filters Change
Client->>Models: update default_filters_score_threshold
Models->>Signals: pre_save signal (cache_old_threshold)
Signals->>Signals: store old value
Models->>Signals: post_save signal (threshold_updated)
Signals->>Signals: compare old vs new
alt threshold changed
Signals->>Models: refresh_cached_counts_for_project()
Models->>Models: update_related_calculated_fields()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
… deployment, and source image
mohamedelabbas1996
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
ami/main/signals.py (1)
8-11: Default-filter signals correctly trigger cache refresh; consider linter and perf tweaksThe new
refresh_cached_counts_for_project, threshold-change, and include/exclude-taxa m2m signals are wired correctly and line up withProject.update_related_calculated_fields(); behavior looks sound.Two small follow-ups:
- To satisfy Ruff’s ARG001 without changing behavior, you can rename unused args to underscore variants, e.g.:
def cache_old_threshold(_sender, instance, **_kwargs): ... def threshold_updated(_sender, instance, **_kwargs): ... def include_taxa_updated(_sender, instance: Project, action, **_kwargs): ... def exclude_taxa_updated(_sender, instance: Project, action, **_kwargs): ...project.update_related_calculated_fields()can be expensive for large projects (iterates all events/deployments and saves). If this becomes a bottleneck, consider deferring to a background job or at least wrapping with some timing/metrics to monitor impact.Also applies to: 133-191
ami/main/models_future/filters.py (1)
128-135: New per-component default-filter flags are well structured; docstring could mention themThe addition of
apply_default_score_filterandapply_default_taxa_filterpreserves existing behavior when left at their defaults and cleanly allows callers to toggle score vs taxa components independently. Early returns forproject is Noneandapply_defaults=falsekeep existing call sites safe.Consider updating the function docstring/Examples section to briefly describe these two flags and show a usage where only score or only taxa defaults are applied, so future readers don’t have to infer the new behavior from the implementation.
Also applies to: 191-213
ami/main/models.py (2)
153-162: Default taxa application to new “Scratch Project” is straightforward; consider logging unknown taxa
get_project_default_filters()and the updatedget_or_create_default_project()cleanly wire the env-configured taxa into a new default project via the include/exclude M2M fields; behavior on existing projects is unchanged.If you want to make ops misconfigurations easier to detect, a small enhancement would be to log any names in
settings.DEFAULT_INCLUDE_TAXA/DEFAULT_EXCLUDE_TAXAthat don’t resolve to aTaxon, e.g. by comparing the configured name lists to theTaxon.objects.filter(...).values_list("name", flat=True)results before returning.Also applies to: 164-186
704-715: Detection-count helpers correctly respect project defaults; align bulk updater semantics and return typesThe new
get_detections_countmethods onDeployment,Event, andSourceImagecorrectly centralize counting throughbuild_occurrence_default_filters_q, so cacheddetections_countnow reflects project default filters when a project is present, and falls back to unfiltered counts when not.Two follow-ups to consider:
- The type hints for
Deployment.get_detections_countandEvent.get_detections_countare-> int | None, but the implementations always return anint(count()), even when there is no project. Tightening these to-> intwould better match actual behavior.SourceImage.update_calculated_fields()now setsdetections_countusing the default-filtered helper, whileupdate_detection_counts()still bulk-updatesdetections_countas the raw number of detections (no default filters). BecauseEvent.timeline()usesupdate_detection_counts(qs, null_only=True)before aggregatingdetections_count, older rows (with null counts) will be populated with unfiltered counts, while newer rows updated viaget_detections_count()will be filtered. If the intent is thatdetections_counteverywhere represents the default-filtered value, it would be worth updatingupdate_detection_counts()to apply the same filtering (or documenting that it intentionally uses raw counts and only for legacy backfill).Also applies to: 919-922, 1085-1095, 1799-1813, 1970-1993
ami/main/api/views.py (1)
1478-1483: Taxon detail/list default-filter handling is consistent; consider documenting new flagsThe revised
get_taxa_observedflow looks good:
- List view: still filters taxa by
filter_by_project_default_taxa(...)and callsget_taxa_observedwith default flags, so both score and taxa defaults apply (unlessapply_defaults=falseis passed, in which casebuild_occurrence_default_filters_qreturnsQ()andbase_filteris unchanged).- Detail view: calls
get_taxa_observed(..., apply_default_score_filter=True, apply_default_taxa_filter=False), so the project’s score threshold now applies while taxa include/exclude are not re-applied inside the occurrence subqueries. That matches the intent of aligning detail counts with list behavior for included taxa while still allowing detail on excluded taxa.The use of
build_occurrence_default_filters_qwith the new flags keeps the semantics consistent with other parts of the codebase. As a small improvement, you might extend theget_taxa_observeddocstring to mention the two flags and how they’re used for list vs detail views, since the signature is now doing a bit more work than the current docstring reflects.Also applies to: 1499-1505, 1523-1529, 1537-1538
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ami/main/api/views.py(3 hunks)ami/main/models.py(6 hunks)ami/main/models_future/filters.py(2 hunks)ami/main/signals.py(2 hunks)config/settings/base.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
ami/main/models_future/filters.py (1)
ami/utils/requests.py (1)
get_default_classification_threshold(124-145)
ami/main/api/views.py (1)
ami/main/models_future/filters.py (1)
build_occurrence_default_filters_q(128-214)
ami/main/models.py (1)
ami/main/models_future/filters.py (1)
build_occurrence_default_filters_q(128-214)
ami/main/signals.py (1)
ami/main/models.py (2)
Project(249-489)update_related_calculated_fields(331-341)
🪛 Ruff (0.14.5)
ami/main/signals.py
142-142: Unused function argument: sender
(ARG001)
142-142: Unused function argument: kwargs
(ARG001)
163-163: Unused function argument: sender
(ARG001)
163-163: Unused function argument: kwargs
(ARG001)
179-179: Unused function argument: sender
(ARG001)
179-179: Unused function argument: kwargs
(ARG001)
187-187: Unused function argument: sender
(ARG001)
187-187: Unused function argument: kwargs
(ARG001)
🔇 Additional comments (1)
config/settings/base.py (1)
451-453: Env-configured default taxa lists look correctReading
DEFAULT_INCLUDE_TAXA/DEFAULT_EXCLUDE_TAXAviaenv.listwith list defaults is consistent with the existing settings usage; no issues from a settings perspective.
Summary
This PR continues the default filters follow-up work from #1029 by adding environment-variable–based default include and exclude taxa to projects.
It also ensures that all cached count fields (detections, occurrences, and taxa counts) consistently respect the project’s default filters across the
Deployment,Event, andSourceImagemodels.Finally, this PR fixes an inconsistency in the Taxon Detail view, where the project’s default score threshold was not being applied causing the direct occurrence count on the detail page to diverge from the count shown in the Taxon List view.
List of Changes
Added
DEFAULT_INCLUDE_TAXAandDEFAULT_EXCLUDE_TAXAenvironment variables and updatedget_or_create_default_project()to attach these default taxa to newly created default projects.Added signals to refresh cached counts when:
default_filters_score_thresholdchangesApplied project default filters to detection counting in:
Deployment.get_detections_count()
Event.get_detections_count()
SourceImage.get_detections_count()
Fixed get_taxa_observed() to allow separate application of score vs taxa filters
Related Issues
#994
Detailed Description
This PR completes the default taxa filter follow-up tasks by addressing three issues:
1. Environment-based default include/exclude taxa
Default include/exclude taxa can now be provided via environment variables (DEFAULT_INCLUDE_TAXA, DEFAULT_EXCLUDE_TAXA).
These are automatically assigned to default projects during creation.
2. Cached count updates more efficiently
Cached fields for occurrences, detections, and taxa counts in
Deployment,Event, andSourceImageare now updated only when the project’s default filter values change.Signals detect changes to: default score threshold, default include taxa and default exclude taxa and only then trigger recalculation of cached counts.
3. Taxon Detail view filter consistency fix
The Taxon Detail view previously skipped the default score threshold, causing mismatched occurrence counts compared to the Taxon List view.
This has now been corrected, ensuring consistent filtering across both views.
How to Test the Changes
call
get_or_create_default_project(user)Confirm include/exclude taxa are assigned
2.
a. Update the project’s default score threshold (default_filters_score_threshold) from the Project admin page antenna ui.
Verify that detections, occurrences, and taxa counts update consistently in:
Session (Event) list view
Session detail view
Stations (Deployment) list view
b. Modify the project’s default include or exclude taxa (add/remove taxa).
Again, verify that all cached counts update correctly across:
Event list & detail views
Deployment (stations) list view
Screenshots
N/A
Deployment Notes
Make sure to set the environment variables for default project taxa:
DEFAULT_INCLUDE_TAXADEFAULT_EXCLUDE_TAXAChecklist
Summary by CodeRabbit
New Features
Improvements