Show recently updated capture sets first in pickers, and stop them waiting on counts they never show - #1381
Merged
Merged
Conversation
The capture set list endpoint reports the number of captures, occurrences and taxa in each set. Filters and pickers only need to name a set, so serving their options from that endpoint made every dropdown wait on counts nobody reads. /api/v2/captures/collections/choices/ returns the id, name and sampling method of each capture set in a project, following the same pattern as the occurrence algorithm filter's choices at /occurrences/algorithms/. The list endpoint keeps its counts. The viewset also gains a default ordering of most recently updated first. It had none, so row order came back in whatever order Postgres produced, which left pagination unstable and could put a capture set a user had just created outside the first page of a picker. Co-Authored-By: Claude <noreply@anthropic.com>
The job form, the export form and the capture set filter now read their options from /captures/collections/choices/, so they no longer wait on counts they do not display, and the most recently updated capture sets come first. The job and export picker still shows how many captures the selected set holds. That count now comes from a single-record fetch of the selected set, the same shape the taxon filter uses for its selected taxon. A set that was picked before it fell off the end of the list is added back to the options, so an existing selection never disappears from the form. Co-Authored-By: Claude <noreply@anthropic.com>
✅ Deploy Preview for antenna-preview canceled.
|
✅ Deploy Preview for antenna-ssec canceled.
|
Contributor
📝 WalkthroughWalkthroughThe backend adds a project-scoped, capped choices endpoint for capture sets. The frontend uses this endpoint for filtering and loads selected-set details separately. Existing list counts and ordering remain supported. ChangesCapture set choices
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CaptureSetPicker
participant useEntities
participant ChoicesAPI
participant useCaptureSetDetails
CaptureSetPicker->>useEntities: load up to 200 capture-set choices
useEntities->>ChoicesAPI: request project-scoped choices
ChoicesAPI-->>useEntities: return lightweight choices
CaptureSetPicker->>useCaptureSetDetails: load selected capture set by IDs
useCaptureSetDetails-->>CaptureSetPicker: return selected capture-set details
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
🟡 Not ready to approve
The new choices action assumes pagination is always enabled and the capture set filter UI can lose visibility of an active selection when it falls outside the loaded page.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR improves capture set selection UX/performance by making capture set lists default to “most recently updated first” and adding a lightweight “choices” endpoint for dropdowns/filters so they don’t block on expensive count annotations.
Changes:
- Add default
-updated_atordering for capture sets and introduceGET /api/v2/captures/collections/choices/that skips count annotations. - Update UI pickers/filters to consume the new choices endpoint, and add a dedicated hook to fetch a single selected capture set when counts are needed.
- Add API tests that pin the choices endpoint contract and ensure the list endpoint still supports counts and count-based ordering.
File summaries
| File | Description |
|---|---|
| ui/src/nova-ui-kit/components/select/capture-set-picker.tsx | Switch picker to /choices/, add selected-item fallback, and fetch selected-set details separately for the capture count message. |
| ui/src/data-services/hooks/capture-sets/useCaptureSetDetails.ts | New hook to fetch a single capture set (without occurrence/taxa counts) for selected-set display. |
| ui/src/data-services/constants.ts | Add CAPTURE_SET_CHOICES route constant. |
| ui/src/components/filtering/filters/capture-set-filter.tsx | Point capture set filter to /choices/ endpoint. |
| ami/main/tests.py | Add TestCaptureSetChoices to pin ordering/scoping/shape and guard list endpoint behavior. |
| ami/main/api/views.py | Move expensive annotations into get_queryset(), add default ordering, and implement the new choices action. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Both sides appended a test class to the end of ami/main/tests.py, so the resolution keeps both: the capture set choices tests from this branch and the bulk identification tests from #1371. Co-Authored-By: Claude <noreply@anthropic.com>
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/components/filtering/filters/capture-set-filter.tsx (1)
5-17: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve a previously selected capture set in
CaptureSetFilter.
EntityPickeronly uses the first choices page, then clears_valueunlessentities.some(...)matches, so a previously selected set outside that page makes the filter read-only empty and removes the selection from view. Apply the same behavior asCaptureSetPicker: load the selected capture set separately when it is not in the loaded choices and include it in the displayed options.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/src/components/filtering/filters/capture-set-filter.tsx` around lines 5 - 17, Update CaptureSetFilter to preserve a selected value that is absent from the initial CAPTURE_SET_CHOICES page, matching CaptureSetPicker’s behavior. Load the selected capture set separately when needed and include it in the options passed to EntityPicker so the existing selection remains visible and editable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/src/nova-ui-kit/components/select/capture-set-picker.tsx`:
- Around line 38-43: Add an effect in the capture-set picker that, after both
data fetches settle, calls onValueChange(undefined) when _value is non-empty but
absent from choices. Keep the existing fallback that preserves captureSet during
loading, and avoid clearing until loading has completed so transient fetch
states do not erase a valid selection.
---
Outside diff comments:
In `@ui/src/components/filtering/filters/capture-set-filter.tsx`:
- Around line 5-17: Update CaptureSetFilter to preserve a selected value that is
absent from the initial CAPTURE_SET_CHOICES page, matching CaptureSetPicker’s
behavior. Load the selected capture set separately when needed and include it in
the options passed to EntityPicker so the existing selection remains visible and
editable.
🪄 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 Plus
Run ID: cc4b2fa7-361c-4ab6-9a97-7b7ac043bd36
📒 Files selected for processing (6)
ami/main/api/views.pyami/main/tests.pyui/src/components/filtering/filters/capture-set-filter.tsxui/src/data-services/constants.tsui/src/data-services/hooks/capture-sets/useCaptureSetDetails.tsui/src/nova-ui-kit/components/select/capture-set-picker.tsx
…t when it is gone The capture set filter loaded only the first page of options and showed a blank field whenever the selected set was not on it. It now loads the same number of options as the job form, and both share one constant for that limit. The job form picker clears its value once both of its fetches have settled and the selected set is still missing from the choices, so a set that has been deleted cannot be submitted while the field looks empty. Co-Authored-By: Claude <noreply@anthropic.com>
…this repo The comments explaining the choices limit repeated each other across three files and ran longer than the surrounding code. Each reason is now stated once, in the place a future editor will be looking, with the ticket carrying the rest. Co-Authored-By: Claude <noreply@anthropic.com>
…kip ci] The rule in .agents/AGENTS.md is written for the Python side, where two or three lines of rationale sit comfortably. Most components here carry none, so the same block stands out far more, and reviewers have had to ask for the same trim more than once. Records the leaner frontend norm, where a shared constant belongs versus a module-local one, and a way to compare a file's comment density with its neighbours before opening a PR. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/src/nova-ui-kit/components/select/entity-picker.tsx`:
- Around line 20-22: Update the entity loading and value derivation in the
entity picker around useEntities and the current value calculation so selected
entities not included in the paginated first page are fetched or merged into the
available entities before deriving value. Preserve valid selections, and only
clear a selection after the API confirms the corresponding entity was deleted.
🪄 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 Plus
Run ID: aba0e699-0a1f-425c-9ee0-b49b145fd4e6
📒 Files selected for processing (7)
ami/main/api/views.pyami/main/tests.pyui/AGENTS.mdui/src/components/filtering/filters/capture-set-filter.tsxui/src/data-services/constants.tsui/src/nova-ui-kit/components/select/capture-set-picker.tsxui/src/nova-ui-kit/components/select/entity-picker.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- ui/src/components/filtering/filters/capture-set-filter.tsx
- ui/src/data-services/constants.ts
- ami/main/tests.py
- ui/src/nova-ui-kit/components/select/capture-set-picker.tsx
- ami/main/api/views.py
…wn gets Callers were each asking for a page size, which is a decision none of them are in a position to make: a dropdown cannot page, so any number they pick is either too small to hold a project's capture sets or an arbitrary guess. The choices endpoint now sends up to a hundred in one response, ordered most recently updated first, and refuses to send more however large a limit is requested. The frontend no longer passes pagination anywhere. Reaching capture sets beyond the cap is what the search field in #1380 is for. Co-Authored-By: Claude <noreply@anthropic.com>
Two tests covered the response size, one below the cap and one above it, and the below-cap case is already covered by the ordering test. They are now a single test asserting that the size does not change whether or not a caller sends a limit, which is the property the endpoint actually promises. Also drops an assertion that the other project's capture set is absent, since the test above it already compares the returned ids to an exact set. Co-Authored-By: Claude <noreply@anthropic.com>
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
Picking a capture set when starting a job is meant to be the easy part of the form, and
in a project that has accumulated a lot of capture sets it currently is not. The dropdown
loads one page of results in whatever order the database happens to return them, so a set
a user created minutes ago may not be in the list at all, and there is no way to reach it
from the form. The same dropdown also waits on the number of captures, occurrences and
taxa in every capture set in the project — figures it never displays — which is why it
sits disabled for a long time before it becomes usable.
This changes both. Capture sets now come back most recently updated first, so the sets
someone is actively working with are at the top of every picker and filter. And the
pickers read from a new endpoint that returns just enough to name a capture set, so they
no longer pay for counts they do not show. That endpoint sends a project's capture sets
in a single response rather than in pages, because a dropdown has no way to ask for a
second one. It follows the pattern introduced for the occurrence algorithm filter in
#1368: a small, purpose-built sub-action that serves a filter's choices, separate from
the full list endpoint the table page uses.
Reported against a project whose capture set list had grown past one page.
List of Changes
ordering = ["-updated_at"]onSourceImageCollectionViewSet. The capture sets table page asks for its own sort order and is unaffectedGET /api/v2/captures/collections/choices/returning id, name and sampling method per set, serialized with the existingSourceImageCollectionNestedSerializer.CaptureSetPickerandCaptureSetFilterread from itCaptureSetChoicesPaginationsets both the default and the maximum size to 100, so the response size is the same whether or not a caller sends a limit. No pagination arguments are passed from the frontend at alluseCaptureSetDetailsfetches the selected set on its own, the same shape the taxon filter uses for its selected taxonui/AGENTS.md. Docs only, no behaviour change — happy to split it out if you would rather review it separatelyCapture choices load almost instantaneously now:
Detailed Description
The capture set list endpoint annotates three counts on every row —
source_images_count,source_images_with_detections_countandsource_images_processed_count— each of which joins through captures to detections andde-duplicates. Grouping happens before
LIMIT, so asking for 20 rows still costs thewhole project. Two measurements from a local copy of production data, on a project with
46 capture sets:
list(qs[:20])with no count annotationslist(qs[:20])with the three count annotationsOver HTTP on the same machine and project, the choices endpoint answered in 0.9s against
2.7s for the list endpoint once both were warm, and 6.1s for the list endpoint on the
first request after a restart. Those figures include local development middleware and a
warm query cache, so treat them as indicative of the gap; the ORM timings above are the
direct measurement.
The counts themselves are untouched — the capture sets table still reports all of them,
and
?with_counts=truestill adds occurrence and taxa counts. The annotations simplymoved from the viewset's class-level queryset into
get_queryset()so the choices actioncan opt out of them. Sorting by a count is still available on the list endpoint and is
excluded from the choices action, where the annotations do not exist.
The page size lives on the server rather than with each caller. A dropdown cannot page,
so any size a caller picks is either too small to hold a project's capture sets or an
arbitrary guess, and different callers had been picking differently.
max_limitis setalongside
default_limitso the size is a contract rather than a default: no caller canraise it, and the response stays bounded for a project of any size. The total count in
the response still reports every capture set, so a caller can tell when it has received
a subset.
Testing
ami.main.testsandami.exportspass locally, alongside CI.TestCaptureSetChoices(9 tests) pins the contract the pickers depend on: mostrecently updated first, names without counts, scoped to the requested project, a
project is required, capture sets in a draft project stay hidden from non-members, one
capped response whether or not a limit is sent, and the list endpoint keeping both its
counts and its ability to sort by them.
tsc --noEmit,eslintandprettier --checkall clean.makemigrations --checkpasses.Verified in a browser
Clicked through against a local copy of production data, on a project with 46 capture
sets:
/captures/collections/choices/?project_id=…withno pagination arguments and lists all 46 sets, most recently updated first. Selecting
one fetches
/captures/collections/{id}/?project_id=…&with_counts=falseand shows"This will select 1,000 captures."
order.
every count column.
Not exercised in a browser: the 100 capture set cap, since no project on hand has that
many. It is covered by a test that creates 120 and asserts the response size with and
without a
limitargument.Follow-up
#1380 proposes replacing the dropdown with a searchable field like the taxon filter, which
is the real fix for a project with more capture sets than one response can hold. This
change makes the common case work; it does not make an arbitrarily long list navigable.