Speed up the Taxa Lists page & add a reusable way to look up permissions once per page

[mihow]

Summary

The Taxa Lists page gets slower with every list a project has, because the API that feeds it asks the database several extra questions for each row: which projects the list belongs to, how many taxa it holds, which project is active, and what the current user may do. This PR makes that page cost the same number of database queries however many lists are shown.

It also puts a reusable way of doing that in the shared code, because the cause is not specific to taxa lists. Every list response in Antenna resolves the caller's permissions once per row: DefaultSerializer.get_permissions calls BaseModel._get_object_perms, which looks up the row's project and then asks django-guardian what the user may do with it — even though the answer is the same for every row in one response. Six serializers override that method; the rest inherit it. The helper here now accepts an already-resolved permission set, so a serializer can compute it once per request and hand it in, and the same three techniques apply to any list endpoint that needs them. Where to use them next is listed below.

List of Changes

#	What changes	How
1	Listing taxa lists takes a fixed number of queries instead of growing with each row	See 2 to 5
2	The projects of every list on the page are loaded together	TaxaListViewSet prefetches projects; get_projects() reads the prefetched rows
3	The taxa count no longer runs a count query per row when the viewset has already counted	getattr(obj, "annotated_taxa_count", obj.taxa.count()) evaluated its default on every call; the fallback now only runs when the annotation is missing
4	The active project and the user's permissions on it are looked up once per request	Cached on the serializer instance, which DRF builds per request and reuses for every row of a list
5	The check that a list belongs to the active project uses the prefetched projects	add_m2m_object_permissions reads the prefetch cache when present and falls back to a query otherwise; it accepts the already-resolved permissions through a new optional argument
6	Project ids on a taxa list come back sorted	They had no guaranteed order before; no consumer relying on an order was found

Where this should be used next

Tracked as #1430, which also weighs whether the remaining endpoints should thread the resolved permissions through each serializer as this PR does, or cache them inside the shared helper so no call site has to change.

The same shape appears elsewhere. These are counted from reading the code, not measured, and none of them is fixed here:

Endpoint	What runs per row	Size
Occurrence list (OccurrenceListSerializer.get_permissions)	The row's project is not select_related, so reading it is a query, and guardian is then asked the same question again for every row. An earlier investigation put this endpoint at roughly 5 + 12 queries per row.	Small; the same diff shape as this PR
Occurrence identifications (OccurrenceIdentificationSerializer.get_permissions), nested inside the above	Each identification walks back to its occurrence's project and then runs a role check that is itself a query, so it is a repeat per row inside a repeat per row	Small to medium; the project is already known from the parent and can come through the serializer context
Capture set list (SourceImageCollectionSerializer.get_permissions)	The direct-foreign-key version of what this PR fixes: resolve the project, ask guardian, once per row	Small
The nested "minimal" serializer helper (MinimalNestedModelSerializer)	It routes through DefaultSerializer.to_representation(), so each nested object appends user_permissions and pays the same lookup. Which parents embed it in a list response was not traced	Unranked

None of those endpoints has a query-count test, so a regression in them would not be noticed today. The lasting fix is to let the object-level helper take a resolved permission set the way the many-to-many one now does, and to give each list endpoint a query-count test with a multi-row fixture; that is follow-up work rather than something to add here.

Two serializers were checked and are not affected: the nested capture-taxon and classification serializers return an empty permission list without touching the database.

Related Issues

Relates to #1424. Independent of #1425, which touches the same permission helper; whichever merges second needs a small rebase.

Detailed Description

Measured with a test fixture (a project, N taxa lists with one taxon each, requested by a project member who is not a superuser, with query caching out of the way):

Rows on the page	Queries before	Queries after
3	16	8
10	37	8

A superuser skips the per-row permission lookup, so the tests use an ordinary member; measuring as a superuser hides most of the cost.

Also measured end to end against a copy of the production database, running this branch and a clean main checkout in the same container image against the same data: the project with the most occurrences (about 180,000), requested by an ordinary member, with 40 temporary taxa lists created inside a transaction and rolled back afterwards, after a warm-up request and with the query cache invalidated before each request.

Rows on the page	Response time on main	Response time here
5	209 ms	84 ms
20	549 ms	134 ms
40	1143 ms and 1035 ms	107 ms and 116 ms

On main the time grows with the number of rows; here it stays flat.

The permission cache lives on the serializer instance, not on the module, the user model or the shared get_active_project() helper. DRF creates a serializer per request and reuses one child instance for every row of a list response, so the cached values cannot outlive a request or reach another user. Keeping the change inside TaxaListSerializer also keeps this PR from changing behaviour for other endpoints.

Two tests pin that the speed-up does not loosen permissions: a list that is not in the active project still reports no update or delete when its projects are prefetched, and a member of project A gets no write permissions on a shared list while requesting with project B, where they are not a member.

The three techniques, for reuse. Resolve the caller's permissions once per request rather than once per row, and pass them in. Resolve the active project once per request the same way. Read a prefetched relation instead of querying it, and keep the fallback for callers that did not prefetch. A fourth thing worth knowing is the getattr(obj, "annotation", obj.thing.count()) trap fixed here: the default argument is evaluated on every call, so the fallback query runs even when the annotation is present. That was the only instance of it in the serializers.

Not fixed here, from reading the code and not measured: the taxa endpoint (/taxa/) resolves the active project per row through TaxonSerializer.get_summary_data(). Its existing query-count test compares two page sizes against a fixture that only ever holds six taxa, so both requests return the same rows and the test cannot see growth per row.

How to Test the Changes

• TestTaxaListListQueryCount asserts the query count is equal for 3 and 10 rows and pins the absolute number. It fails on main (16 and 37) and passes here (8 and 8).
• TaxaListPermissionScopingTestCase covers the two permission guards above.
• The existing taxa list test classes pass with these (29 tests across the six classes, run locally on the final commit). The full ami.main module (386 tests), makemigrations --check --dry-run, black, isort and flake8 passed during development. Backend CI is currently failing on every branch before tests start because an image pull is refused (fix in Update docker image source for minio and minio-init from docker hub to quay.io #1419), so CI here will not be informative until that merges.

Deployment Notes

None. No migration, no settings, no change to response shape apart from project ids being sorted.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TngW6AshkUEhNp9D6z3U9Z

Summary by CodeRabbit

• Performance

  • Improved taxa list loading performance by reducing repeated database queries.
  • Project memberships and permissions are reused during list responses.

• Bug Fixes

  • Corrected permission handling for prefetched project memberships.
  • Ensured write permissions are shown only for projects the user belongs to.
  • Prevented permission changes when the referenced project is missing.

• Tests

  • Added coverage for stable query counts and project-scoped permission behavior.

[netlify]

✅ Deploy Preview for antenna-ssec canceled.

Name	Link
🔨 Latest commit	96e2671
🔍 Latest deploy log	https://app.netlify.com/projects/antenna-ssec/deploys/6aad934e374c13000885fcbe

[netlify]

✅ Deploy Preview for antenna-preview canceled.

Name	Link
🔨 Latest commit	96e2671
🔍 Latest deploy log	https://app.netlify.com/projects/antenna-preview/deploys/6aad934e40208d0008ac79b4

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 0cef1f3c-7458-4f5c-8e51-ac9eb8ad4965

📥 Commits

Reviewing files that changed from the base of the PR and between 903b16f and 637d619.

📒 Files selected for processing (2)

• ami/main/api/serializers.py
• ami/main/tests.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The change reduces repeated project and permission queries during taxa list serialization. It adds prefetched project membership checks, cached project permissions, sorted prefetched project IDs, and regression tests for query counts and permission scoping.

Changes

Taxa list permissions and query behavior

Layer / File(s)	Summary
Permission guards and cached project permissions ami/base/permissions.py	add_m2m_object_permissions now accepts cached project permissions, separates project existence and membership checks, and uses prefetched projects when available.
Serializer and queryset data reuse ami/main/api/serializers.py, ami/main/api/views.py	The serializer reuses project data and permissions, avoids unnecessary count queries, and returns sorted prefetched project IDs. The viewset prefetches projects.
Query and permission regression coverage ami/main/tests.py	Tests cover stable list query counts and project-scoped write permissions with prefetched project relations.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Suggested reviewers: mohamedelabbas1996

Merge Risk: ⚪ Minimal · up to 637d6

The optimization preserves the described permission and response contracts, with regression coverage for query behavior and project scoping.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the main change: improving Taxa Lists performance and reusing permission lookups.
Description check	✅ Passed	The description is complete and relevant. It covers the summary, changes, related issues, technical details, testing, performance measurements, risks, and deployment notes. The checklist and screensho…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

🟡 Changes recommended

The new permission scoping tests don’t currently grant taxa-list write perms, so they can pass without exercising the intended cross-project permission-leak scenarios.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR reduces N+1 query behavior on the Taxa Lists list endpoint by batching related lookups (projects, taxa counts, and permissions) so list performance stays flat as row count grows.

Changes:

• Prefetch TaxaList.projects and use that prefetch in serializer/project-membership checks to avoid per-row queries.
• Fix taxa_count to only execute obj.taxa.count() when the annotation is actually missing.
• Cache active project and project permissions per request (serializer instance) and add targeted query-count/permission-scoping tests.

File summaries

File	Description
ami/main/api/views.py	Prefetches projects in TaxaListViewSet.get_queryset() to prevent per-row DB hits.
ami/main/api/serializers.py	Avoids eager default evaluation for taxa counts; caches active project/perms per request; reads prefetched projects and returns sorted IDs.
ami/base/permissions.py	Extends add_m2m_object_permissions() to reuse prefetched projects and accept optional precomputed project perms.
ami/main/tests.py	Adds query-count regression test and permission scoping tests for the new prefetch/perms fast paths.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.