Let a pipeline pair any detector with any classifier, and add a general insect-detection pipeline#160
Let a pipeline pair any detector with any classifier, and add a general insect-detection pipeline#160mihow wants to merge 19 commits into
Conversation
Stand up a reversible, dev-branch scaffold for a new "any-bug" pipeline
(slug: anybug_global_moths_2024): a YOLO26 object detector, an insect-order
gate that forwards only Lepidoptera to the terminal classifier, and the
existing global species classifier as the terminal stage. No live inference
is wired up yet -- the model weight is not in the object store, so the
detector and pipeline are dormant behind clearly-marked TODOs.
Turn the pipeline registry into an explicit, ordered stage definition so the
detector, intermediate gates, and terminal classifier are configured in one
place instead of being hardcoded in process().
Changes:
- Add AnyBugObjectDetector_YOLO26 (ml) + APIAnyBugDetector (api). Outputs
absolute raw-pixel xyxy in the original image space: Ultralytics de-letterboxes
to original pixels, and feeding raw NumPy arrays neutralizes its default EXIF
auto-transpose so boxes are not rotated on portrait captures. Distinct
algorithm key ("anybug-yolo26-detector-2024") so reprocessing treats it as new
work. weights_path is a marked placeholder; ultralytics import is lazy.
- Replace CLASSIFIER_CHOICES with PIPELINE_CHOICES, a dict of PipelineDefinition
dataclasses (detector, intermediates, terminal). IntermediateStage gates on
"top label in pass_labels". Existing slugs keep their exact behavior
(APIMothDetector + binary moth filter + terminal). A backward-compat
CLASSIFIER_CHOICES/should_filter_detections shim (excluding anybug) keeps the
antenna GPU worker, CLI, registration, and tests working unchanged.
- Generalize process() to run the configured detector and iterate intermediate
gates; make_pipeline_config_response iterates stages for /info;
initialize_service_info tolerates a pipeline that cannot build yet (the
weightless anybug detector) so the rest stay online.
- Add score_scale (default 1.0) to APIMothClassifier, applied to softmax scores;
set 0.5 on InsectOrderClassifier. Argmax-invariant, so the Lepidoptera gate is
unaffected.
- Add ultralytics to dependencies (version/YOLO26-support TODO).
Confirmed the Lepidoptera order label is exactly "Lepidoptera" (index 0) in the
insect_orders_2025 category map.
Co-Authored-By: Claude <noreply@anthropic.com>
… license The any-bug detector weight is now in the object store, so replace the placeholder URL with the real one and remove the "pending upload" TODOs. - Point AnyBugObjectDetector_YOLO26.weights_path at the uploaded weight (ami-models/moths/localization/yolo26n-anybug-v1.pt). The "moths" prefix is a slight misnomer for an any-taxa detector but is kept to match where the other localization weights live. - Pin ultralytics>=8.4.0: v8.4.0 is the first stable release that ships YOLO26 (v8.3.204 only carried a docs preview). See the Ultralytics v8.4.0 release notes. - Set the pyproject license to AGPL-3.0 to match the repo LICENSE file (the MIT metadata was wrong). Ultralytics is AGPL-3.0, which is fine under the repo's AGPL license. Co-Authored-By: Claude <noreply@anthropic.com>
… gates The anybug_global_moths_2024 pipeline registered via /info but could never be polled or processed: the worker's subscription, --pipeline validation, and job dispatch all read the legacy CLASSIFIER_CHOICES shim, which excludes it. Point those three gates at the full PIPELINE_CHOICES registry so every advertised pipeline is subscribable, requestable, and dispatchable. Legacy moth pipelines dispatch through the new lookup unchanged. The in-process worker still only executes the FasterRCNN moth detector with the optional binary filter, so it now fails loudly (NotImplementedError) on a pipeline whose stages it cannot run yet, instead of silently detecting with the wrong model. Making _process_batch stage-aware is a follow-up. Also harden the shared staged-execution path: - Derive the CLASSIFIER_CHOICES / should_filter_detections shims from PIPELINE_CHOICES so the two views cannot drift; mark them deprecated. - Extract the gate/terminal assembly from /process into run_classification_stages, documenting that it merges detections by identity (not list position) so a gate-dropped or un-croppable detection keeps its own label, and a passing detection carries a non-terminal gate label plus a terminal label. - Validate each pipeline stage's role on construction. New weight-free unit tests cover the advertise==subscribe==dispatch invariant, the mixed-batch identity merge with drops in the middle, gate-vs-terminal labelling, score_scale tempering scores but not logits, stage shape validation, and worker dispatch accepting legacy pipelines while rejecting anybug. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make the async worker's _process_batch stage-driven instead of hardcoding the FasterRCNN detector and the binary moth/non-moth filter, so `ami worker` can run the anybug pipeline (YOLO26 + Lepidoptera order gate + global species) and any other PIPELINE_CHOICES pipeline end-to-end. The detector now comes from the pipeline's PipelineDefinition, and the intermediate gate(s) and terminal classifier run through the SAME run_classification_stages() engine as the FastAPI /process path, so the two code paths can no longer drift. run_classification_stages() gained a run_stage strategy: /process constructs a fresh model and reads crops from URLs; the worker reuses a preloaded model and slices crops from image tensors it already has in memory. Stage models are still built once per job and reused across batches. Legacy detector+binary-gate+terminal pipelines stay behavior-preserving: the moth/non-moth split is unchanged and the posted payloads match. The NotImplementedError capability guard is removed (every advertised pipeline is now composed of stages this path can run), and the now-unused should_filter_detections() shim and the binary-only crop loop are deleted. Added weight-free unit tests that stub the detector and stage models to assert _process_batch runs the anybug stage list and posts correctly-labelled results without NotImplementedError, and that a legacy binary pipeline still returns non-moth detections tagged with the non-terminal binary label and no species. Co-Authored-By: Claude <noreply@anthropic.com>
The async `ami worker` path builds every detector's input batch from a shared dataloader (`RESTDataset`) that hardcodes torchvision `ToTensor`, producing CHW float32 RGB tensors in [0, 1]. That is what the FasterRCNN detector expects, and the classifier stages rely on it too because they slice CHW crops (`image_tensor[:, y1:y2, x1:x2]`) from the same tensors. `AnyBugObjectDetector_YOLO26.predict_batch`, however, expected the HWC uint8 arrays produced by the FastAPI `/process` path's own transform. On the async path it received CHW float tensors instead, and its RGB->BGR `img[..., ::-1]` flipped the width axis rather than the channel axis, handing Ultralytics channels-first float garbage. FasterRCNN pipelines were unaffected. Because the shared dataloader must keep emitting CHW tensors for the classifier crops, the fix is a defensive conversion in the detector, not a per-detector dataloader transform. `predict_batch` now normalizes every image to HWC uint8 RGB (transposing and rescaling floating-point ToTensor input, passing integer HWC input through unchanged) before the flip to BGR, so the `/process` path stays byte-for-byte identical. Adds weight-free unit tests that stub the YOLO model and assert the array handed to it is HWC, uint8, BGR, and round-trips a known pixel pattern for both dataloader formats, plus guards pinning the FasterRCNN and YOLO `/process` transform contracts. Co-Authored-By: Claude <noreply@anthropic.com>
The async worker and the FastAPI /process path feed the YOLO26 detector two different single-image formats, and _as_hwc_uint8_rgb previously guessed which one it had from the dtype alone (float => channels-first ToTensor, int => HWC passthrough). That guess silently corrupts the pixels handed to the model on any input the guess misreads: a normalized-but-not-[0,1] float (e.g. an ImageNet mean/std standardized tensor), an already channels-last float, an integer channels-first array, or an unexpected rank. Replace the guess with an explicit, asserted contract so a future dataloader-transform change fails loudly instead of feeding garbage to the model. Floating point must be channels-first and within [0, 1]; it is transposed to channels-last and rescaled to 0-255 with rounding. Integer must be channels-last and is returned as uint8 unchanged, keeping the /process path byte-for-byte identical. Both a 3D single image and a 4D NCHW/NHWC batch are accepted. Anything else raises a ValueError naming the offending shape, dtype, and range. Also notes the per-call full-image copy cost so nobody fans many large images through the helper blind. Adds weight-free tests: out-of-[0,1] float, already-HWC float, integer-CHW, and unexpected rank all raise; the 4D batch is converted per-image; and a round-trip whose values are not multiples of 1/255 pins rounding over truncation. Co-Authored-By: Claude <noreply@anthropic.com>
…runners Three correctness fixes for the staged pipeline's classification crops, plus two guards. Tag uncroppable detections instead of returning them naked. A detection whose bounding box is degenerate was skipped in the worker's crop loop and returned with no classification, breaking the pipeline invariant that every returned detection carries a classification (non-passing gate items are already returned tagged). It is now tagged with a non-terminal "uncroppable" sentinel carrying its own algorithm reference, mirroring how a gate tags the detections it drops, so the platform never shows a naked box and never mistakes the sentinel for a real result. Align crop behavior at image boundaries. The /process runner (PIL Image.crop) zero-pads out-of-bounds coordinates while the worker runner (tensor slicing) wraps negative indices, so the same detection could yield different crops and different labels on the two paths. Both now clamp through a shared BoundingBox.clamp_to_bounds before cropping, so they see the same in-bounds region. For the in-bounds integer boxes detectors actually emit this is a no-op; only edge and past-edge boxes change, and a box entirely off-image collapses to a degenerate region that is then tagged uncroppable. Guards: a test pins that the derived CLASSIFIER_CHOICES projection stays a 1:1 view of the default-detector pipelines (no slug can collide as pipelines are added), and a runner-equivalence test asserts the PIL-crop and tensor-slice regions are pixel-identical for in-bounds, edge, past-edge, and degenerate boxes. Co-Authored-By: Claude <noreply@anthropic.com>
Regenerate uv.lock so the deploy box runs a known dependency set instead of re-resolving at sync time. The change is purely additive: ultralytics 8.4.104 and its transitive deps (opencv-python, matplotlib, polars, psutil, ultralytics-thop, nvidia-ml-py, and their supports) are added. The pre-existing core pins are unchanged — torch 2.10.0, torchvision 0.25.0, numpy 2.2.6/2.4.2, pillow 10.4.0, timm 1.0.24, pandas — so the existing pipelines resolve to the same versions they do today. Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds YOLO26 any-bug detection, introduces registered multi-stage pipelines with intermediate gates, updates API and worker orchestration, standardizes crop bounds, adds score scaling, and expands regression coverage. ChangesPipeline and detector integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant APIAnyBugDetector
participant run_classification_stages
participant APIMothClassifier
Client->>API: submit images and pipeline
API->>APIAnyBugDetector: run detector
APIAnyBugDetector-->>API: return detections
API->>run_classification_stages: run registered stages
run_classification_stages->>APIMothClassifier: run gates and terminal classifier
APIMothClassifier-->>run_classification_stages: return classifications
run_classification_stages-->>API: assemble detections
API-->>Client: return results
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
…ing it The insect order classifier has no "not an insect" class, so it is as confident on non-animal crops (smudges, plant matter, debris) as on real ones. The previous 0.5 factor halved every order score to signal that uncertainty, which pushed genuine predictions down to around 0.4-0.5 and made confident results look unreliable in the UI. Scaling by 0.9 keeps the same guard - no order prediction is ever presented as certain - without flattening the whole range. The factor is a positive constant, so it remains argmax-invariant: the stored logits and the Lepidoptera gate are unaffected. Co-Authored-By: Claude <noreply@anthropic.com>
Antenna refuses to save detections from a pipeline whose /info response advertises more than one localization algorithm. A pre-refactor config builder injected the FasterRCNN moth detector into every pipeline, so once the anybug pipeline arrived with its own YOLO26 detector it advertised two and its detections were silently dropped on the Antenna side. The typed PipelineDefinition already sources the detector from a single validated field and the config builder emits one algorithm per stage. These tests pin both halves so the regression cannot come back: every registered pipeline is checked for exactly one localization algorithm, the anybug pipeline is checked for its own YOLO26 detector rather than the moth detector, and the builder is checked against stub stages so it is covered independently of the registry contents. All three read class attributes or stubs, so no model weights are downloaded. Co-Authored-By: Claude <noreply@anthropic.com>
make_pipeline_config_response now takes the PipelineDefinition for a slug, not a bare terminal classifier class: it walks the definition's detector, intermediate gates, and terminal stage to build the advertised algorithm list. This test still handed it a classifier class, so it failed with "type object 'MothClassifierQuebecVermont' has no attribute 'detector'". Read both pipelines from PIPELINE_CHOICES, which is the single source of truth for how a slug is staged, rather than from the deprecated CLASSIFIER_CHOICES shim that maps a slug to its terminal classifier only. The assertions are unchanged in substance - a species pipeline advertises three algorithms and the binary-only pipeline two - and a docstring now records why those counts are what they are. Co-Authored-By: Claude <noreply@anthropic.com>
Wikimedia now serves thumbnails only at the widths listed on https://w.wiki/GHai and returns HTTP 400 for anything else, so the 103px URL this test has always used started failing everywhere. Switch to 120px, which is on that list, and record the constraint in a comment so the width is not casually changed again. Unrelated to the rest of this branch; the test fails the same way on main. Co-Authored-By: Claude <noreply@anthropic.com>
…nistic get_test_images truncated an unsorted glob, so which images a test received depended on directory iteration order and differed between checkouts. The vermont fixture set spans one trapping night: the 00:04 frame yields 18 detections and the 04:30 frame 8, but the 20:04 dusk frame is empty and the detector correctly finds nothing in it. Two tests request a single image and then assert that detections came back, so on any checkout that happened to enumerate the dusk frame first they failed with "No detections found in response" - most recently on the Python 3.10 CI job while 3.12 passed at the same commit. Sorting first makes the selection stable and puts the 00:04 frame at the front, so asking for one image now reliably yields one with insects in it. Pre-existing and unrelated to the pipeline registry work; the helper is unchanged from main. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
trapdata/api/models/localization.py (1)
64-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
APIAnyBugDetectoris a near-verbatim copy ofAPIMothDetector. Every method here (__init__,reset,get_dataset,get_source_image,save_results,run) is byte-identical toAPIMothDetector(Lines 14-61); only the localization mixin in the class bases differs. Consider extracting a shared_APILocalizationBase(APIInferenceBaseClass)that both detectors inherit, so the request/response plumbing is defined once and cannot drift between the two paths.🤖 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 `@trapdata/api/models/localization.py` around lines 64 - 122, Extract the duplicated localization request/response plumbing from APIAnyBugDetector and APIMothDetector into a shared _APILocalizationBase extending APIInferenceBaseClass. Move the common methods (__init__, reset, get_dataset, get_source_image, save_results, and run) into that base, then have both detectors inherit it alongside their detector-specific mixin while preserving their existing behavior.
🤖 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 `@trapdata/api/api.py`:
- Around line 455-459: Update PipelineResponse.pipeline to use the
required-field pattern already used by PipelineRequest.pipeline, replacing the
PipelineChoice default with an explicit required Field(...) while preserving its
description and examples.
In `@trapdata/api/models/localization.py`:
- Line 113: Update both `APIMothDetector.save_results` loops to call `zip` with
`strict=True`, including the loop over `item_ids` and `batch_output`, so
mismatched lengths raise an error instead of being silently truncated.
---
Nitpick comments:
In `@trapdata/api/models/localization.py`:
- Around line 64-122: Extract the duplicated localization request/response
plumbing from APIAnyBugDetector and APIMothDetector into a shared
_APILocalizationBase extending APIInferenceBaseClass. Move the common methods
(__init__, reset, get_dataset, get_source_image, save_results, and run) into
that base, then have both detectors inherit it alongside their detector-specific
mixin while preserving their existing behavior.
🪄 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: 3520b9dd-359b-43b8-ae63-7d3d752c18e5
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
pyproject.tomltrapdata/antenna/worker.pytrapdata/api/api.pytrapdata/api/datasets.pytrapdata/api/models/classification.pytrapdata/api/models/localization.pytrapdata/api/schemas.pytrapdata/api/tests/test_api.pytrapdata/api/tests/test_models.pytrapdata/api/tests/test_pipeline_registry.pytrapdata/api/tests/utils.pytrapdata/cli/worker.pytrapdata/ml/models/localization.py
| class PipelineResponse(PipelineResponse_): | ||
| pipeline: PipelineChoice = pydantic.Field( | ||
| PipelineChoice, | ||
| description=PipelineResponse_.model_fields["pipeline"].description, | ||
| examples=list(_classifier_choices.keys()), | ||
| examples=list(_pipeline_choices.keys()), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
pydantic v2 Field first positional argument default value semantics
💡 Result:
In Pydantic V2, the first positional argument of the Field() function is explicitly mapped to the default parameter [1][2][3]. This parameter defines the default value for the field if no value is provided during model instantiation [4][5][6]. If you do not provide a value to Field() and the field is not otherwise assigned a default (e.g., via field: type = value), Pydantic treats the field as required [5][7]. Key points regarding this semantic: - Ellipsis (...): You can pass ... as the first positional argument (or to the default parameter) to explicitly indicate that a field is required [2]. - Mutual Exclusivity: You cannot provide both a default value (positional or keyword) and a default_factory simultaneously; doing so will raise an error [2][6]. - Mutation Safety: If you provide a mutable object as a default value, Pydantic will, in the event that the value is not hashable, create a deep copy of the default for each instance of the model to prevent shared-state bugs [4][5][6]. - Static Type Checking: For optimal compatibility with static type checkers, it is generally recommended to use the explicit default=... keyword argument rather than relying on the positional argument, as this helps tools correctly synthesize the model's init method [4][8][6].
Citations:
- 1: https://pydantic.dev/docs/validation/latest/api/pydantic/fields/
- 2: https://github.com/pydantic/pydantic/blob/5ccbdcb/pydantic/fields.py
- 3: https://pydantic.dev/docs/validation/2.9/api/pydantic/fields/
- 4: https://pydantic.dev/docs/validation/2.11/concepts/fields/
- 5: https://pydantic.dev/docs/validation/latest/concepts/fields/
- 6: https://pydantic.dev/docs/validation/2.10/concepts/fields/
- 7: https://pydantic.dev/docs/validation/latest/get-started/migration/
- 8: https://pydantic.dev/docs/validation/2.12/concepts/fields/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the declared field, imports, enum definition, model usages, and test fixtures if present.
printf 'Files matching api.py:\n'
fd -a 'api\.py$' . | sed 's#^\./##'
printf '\nRelevant trapdata/api/api.py section:\n'
sed -n '420,475p' trapdata/api/api.py
printf '\nPipelineChoice usages in trapdata/api/api.py:\n'
rg -n "PipelineChoice|PipelineResponse|pipeline:" trapdata/api/api.py -C 3
printf '\nProject files mentioning PipelineResponse or pipeline examples:\n'
rg -n "PipelineResponse|PipelineResponse_|pipeline" . -g '*.py' | head -200
printf '\nDependency versions:\n'
if [ -f pyproject.toml ]; then sed -n '1,220p' pyproject.toml; fi
if [ -f requirements.txt ]; then rg -n 'pydantic|pip|poetry|uv' requirements.txt || true; fiRepository: RolnickLab/ami-data-companion
Length of output: 26386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import enum
PipelineChoice = enum.Enum("PipelineChoice", "a b c")
print("Field first positional argument accepts enum class:", PipelineChoice)
print("Example value:", PipelineChoice.a)
print("Enum is subclass of Enum:", isinstance(PipelineChoice, enum.EnumMeta))
PYRepository: RolnickLab/ami-data-companion
Length of output: 306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
import enum
from pydantic import BaseModel, Field
PIPELINE_CHOICES = {"a": "a"}
_pipeline_choices = dict(zip(PIPELINE_CHOICES.keys(), list(PIPELINE_CHOICES.keys())))
PipelineChoice = enum.Enum("PipelineChoice", _pipeline_choices)
class Base(BaseModel):
pipeline: PipelineChoice
class Required(Base):
pipeline: PipelineChoice = Field(...)
class Current(Base):
pipeline: PipelineChoice = Field(PipelineChoice)
def model_json(field_type):
return field_type.model_json_schema().get("properties", {}).get("pipeline", {})
print("Base default json_schema_extra:", json.dumps(model_json(Base), indent=2))
print("Required default json_schema_extra:", json.dumps(model_json(Required), indent=2))
print("Current default json_schema_extra:", json.dumps(model_json(Current), indent=2))
print("Base required:", "pipeline" in Base.model_json_schema().get("required", []))
print("Required required:", "pipeline" in Required.model_json_schema().get("required", []))
print("Current required:", "pipeline" in Current.model_json_schema().get("required", []))
print("Current model_dump Current with enum value:", Current(PipelineChoice.a).model_dump())
try:
print("Current model_dump empty:", Current().model_dump())
except Exception as e:
print("Current model_dump empty raised", type(e).__name__ + ":", e)
PYRepository: RolnickLab/ami-data-companion
Length of output: 881
Make PipelineResponse.pipeline required instead of defaulting to the enum class.
pydantic.Field(PipelineChoice, ...) makes the field optional with a non-value (PipelineChoice) as its default, while PipelineRequest.pipeline is already constructed with Field(...) and remains required. Use the same Field(...) pattern so the response model preserves the required pipeline contract explicitly.
🤖 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 `@trapdata/api/api.py` around lines 455 - 459, Update PipelineResponse.pipeline
to use the required-field pattern already used by PipelineRequest.pipeline,
replacing the PipelineChoice default with an explicit required Field(...) while
preserving its description and examples.
| ) | ||
| return detection | ||
|
|
||
| for image_id, image_output in zip(item_ids, batch_output): |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Pass an explicit strict= to zip. item_ids and batch_output are expected to be equal length; making that explicit (strict=True) turns a silent truncation into a loud error if they ever diverge (and clears the Ruff B905 warning). The same applies to the identical APIMothDetector.save_results at Line 52.
Proposed change
- for image_id, image_output in zip(item_ids, batch_output):
+ for image_id, image_output in zip(item_ids, batch_output, strict=True):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for image_id, image_output in zip(item_ids, batch_output): | |
| for image_id, image_output in zip(item_ids, batch_output, strict=True): |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 113-113: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
🤖 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 `@trapdata/api/models/localization.py` at line 113, Update both
`APIMothDetector.save_results` loops to call `zip` with `strict=True`, including
the loop over `item_ids` and `batch_output`, so mismatched lengths raise an
error instead of being silently truncated.
Source: Linters/SAST tools
…vers Simulating the double-detector regression, by making the config builder append the moth detector for any pipeline that does not already use it, showed that only the builder test fails. The two registry tests read the stage classes straight off PipelineDefinition, and its role validation already guarantees exactly one localization slot, so they cannot catch a builder that injects a detector of its own. The tests are worth keeping, but their docstrings claimed to guard the deployed regression, which would mislead anyone relying on them later. Each now states what it actually pins: the registry declaration and the role validation behind it, or the builder's own output. Co-Authored-By: Claude <noreply@anthropic.com>
The name and description a pipeline advertises came from its terminal classifier. That was unique only for as long as every pipeline ended in a classifier of its own. The new anybug pipeline shares MothClassifierGlobal with "global_moths_2024", so both advertised "Global Species Classifier - Aug 2024" with the same description, and an operator choosing a pipeline in the platform saw two identical entries with no way to tell which ran the YOLO26 detector. This already had to be corrected by hand on a deployment. PipelineDefinition now carries optional name and description fields, and the config builder prefers them, falling back to the terminal classifier so every existing pipeline advertises exactly what it did before. The anybug pipeline sets both, matching the name used on the deployment. Two tests cover it: one asserts no two registered pipelines resolve to the same advertised name, and fails with the colliding pair named; the other pins the prefer-then-fall-back rule through the builder itself. Both are weight-free. Co-Authored-By: Claude <noreply@anthropic.com>
Several comments still said the YOLO26 detector could not run until the weight was uploaded and ultralytics installed. Neither holds: the weight is served (HTTP 200, 5.4 MB) and ultralytics>=8.4.0 is a required dependency in pyproject.toml and uv.lock. The stale text was the apparent justification for tolerating a build failure for this pipeline in initialize_service_info, so it actively misled anyone reading that code. The tolerance itself is kept and its comment now gives the reason that does hold: one pipeline failing to build must not remove every other pipeline from /info. The lazy ultralytics import is also kept, with its rationale corrected from "the package may not be installed" to avoiding its import cost for detectors that never use it. The batching TODO is still open and now says what happens today instead of "when this detector goes live". Co-Authored-By: Claude <noreply@anthropic.com>
The comment said score_scale = 0.9 "caps the reported order confidence at 0.9". It is a multiplier, so it applies the same 10% reduction to every score, low ones included; it bounds the maximum at 0.9 only because a softmax cannot exceed 1.0. Reading it as a cap would suggest that scores below the threshold pass through untouched, which is not what happens. The multiply is deliberate and stays: it degrades the top of the range smoothly, where a hard minimum would pile most predictions at exactly 0.900 and destroy the ordering among them. Co-Authored-By: Claude <noreply@anthropic.com>
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 `@trapdata/api/tests/test_pipeline_registry.py`:
- Around line 234-235: Add function annotations throughout the test stubs:
annotate each stub constructor’s parameters to match the corresponding
production constructor types and its return as None, annotate _stub_pipeline’s
forwarded kwargs, and add -> None to the changed test functions. Apply these
updates to all indicated locations while preserving existing behavior.
🪄 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: 8b247a98-0a7a-441f-9b25-038b2b34e966
📒 Files selected for processing (5)
trapdata/api/api.pytrapdata/api/models/classification.pytrapdata/api/models/localization.pytrapdata/api/tests/test_pipeline_registry.pytrapdata/ml/models/localization.py
💤 Files with no reviewable changes (1)
- trapdata/api/models/localization.py
🚧 Files skipped from review as they are similar to previous changes (3)
- trapdata/api/models/classification.py
- trapdata/ml/models/localization.py
- trapdata/api/api.py
| def __init__(self, source_images=(), **kwargs): | ||
| pass |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add the required function type annotations.
Annotate the stub constructors’ inputs/None returns, _stub_pipeline’s forwarded kwargs, and the changed test functions’ -> None returns; mirror the corresponding production constructor types where applicable.
As per coding guidelines, “Use type hints in function signatures to document expected types without requiring extensive documentation.”
Also applies to: 250-251, 266-267, 273-273, 287-287, 311-311, 329-329
🤖 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 `@trapdata/api/tests/test_pipeline_registry.py` around lines 234 - 235, Add
function annotations throughout the test stubs: annotate each stub constructor’s
parameters to match the corresponding production constructor types and its
return as None, annotate _stub_pipeline’s forwarded kwargs, and add -> None to
the changed test functions. Apply these updates to all indicated locations while
preserving existing behavior.
Source: Coding guidelines
Ultralytics restores the checkpoint's training imgsz (1024) at predict time via _reset_ckpt_args, so this is a no-op against the current pin. It guards against a future Ultralytics release dropping that restore, which would silently fall back to the 640 predict default and cut recall on small insects. Co-Authored-By: Claude <noreply@anthropic.com>
|
Claude says: Pushed This is a no-op against the current Ultralytics: Not in this PR: swapping the nano checkpoint for the larger YOLO26x flavor. That's blocked on uploading the trained x weights to the models bucket — only |
The Antenna batch service runs on GPU and is non-interactive, so it can afford a larger detector than the on-device nano. Point the AnyBug detector at the YOLO26x checkpoint trained on the same combined flat-bug + Fieldguide set (combined-val mAP50 0.829 vs the nano's 0.785, ~21 ms per frame vs the FasterRCNN it replaces at ~35 ms). Rename the detector's key/name/description so its detections are attributable to the x model. Because Antenna registration is add-only, a deployment that already registered the nano detector needs its pipeline's detector algorithm reset to the x key after this deploys. Co-Authored-By: Claude <noreply@anthropic.com>
Summary
Until now the processing service could run only one detector — the built-in moth detector — and a "pipeline" just chose which species classifier ran after it. This change makes a pipeline a fully described ordered list of stages: a detector, optional intermediate gate models, and a terminal classifier. That lets a pipeline pair any detector with any classifier instead of hardcoding the detector.
The first capability it unlocks is a general insect-detection pipeline (
anybug_global_moths_2024): a YOLO26 detector finds any insect, an order-level gate keeps the Lepidoptera detections (others are returned tagged with their predicted order), and the global moth model classifies those. Because this pipeline's primary home is the non-interactive GPU batch service, it runs the larger YOLO26x flavor of the detector rather than the on-device nano — more accuracy at a compute cost the batch path can absorb. The synchronous HTTP path and the asynchronous worker now run every pipeline through one shared stage engine, so what the service advertises and what the worker can actually run cannot drift apart.Scope note: the automated tests are structure and format level and run without model weights. Real detection quality and coordinate/channel correctness were validated on a dev server against real captures on GPU (see "Verified").
List of Changes
PIPELINE_CHOICESregistry of dataclass stage definitions (detector / intermediate gate(s) / terminal); the formerCLASSIFIER_CHOICESis kept as a derived, deprecated projection.anybug_global_moths_2024pipeline: detect any insect, classify the moths, tag the rest by order.InsectOrderClassifiergate (keep top order == "Lepidoptera") →MothClassifierGlobalterminal. Non-passing detections are returned tagged with their predicted order./processshare one stage-execution engine; an invariant test pins advertised == subscribed == dispatchable pipeline sets.BoundingBox.clamp_to_boundsapplied on both runners.score_scale = 0.9multiplies the reported scores only, leaving logits untouched. See caveats.anybug_global_moths_2024andglobal_moths_2024both end inMothClassifierGlobal, so both advertised "Global Species Classifier - Aug 2024".PipelineDefinitionnow carries optionalname/descriptionthat the config builder prefers, falling back to the terminal classifier so every existing pipeline is unchanged. The new pipeline advertises "Global moths with Anybug detector".anybug-yolo26x-detector-2024/ "AnyBug YOLO26x Detector 2024" so detections are attributable to the x model.imgsz=1024(the checkpoint's training size) is pinned inpredict(). Ultralytics restores it from the checkpoint today, so this is a no-op now; it guards against a future release dropping that restore and silently falling back to the 640 predict default, which would cut recall on small insects.Incidental CI repairs
Two failures on this branch came from existing test code rather than from the work above. Both are fixed here so the branch can go green, and both would fail the same way on
main.get_test_imagestruncated an unsortedglob, so which images a test received varied between checkouts. The vermont fixture set spans one trapping night: run through the detector, the 00:04 frame yields 18 detections and the 04:30 frame 8, but the 20:04 dusk frame is genuinely empty. The two tests that request a single image and then assert detections came back therefore failed whenever a checkout happened to enumerate the dusk frame first. That is a coin flip rather than anything to do with this branch — the two tests failed on the Python 3.10 job in one run, then passed on both 3.10 and 3.12 in the next run without their code or the detector changing at all. Sorting before truncating makes the selection stable and puts a frame with insects in it at the front.test_urlbegan failing everywhere. Switched to 120px, which is on that list.Verified
PipelineDefinition, whose role validation already guarantees a single detector slot, so they pin that validation rather than the builder. Each docstring now says which half it covers.Caveats and follow-ups
score_scale = 0.9is a display guard, not a calibration. The insect order classifier has no "not an insect" class, so it is as confident on a smudge or a piece of plant matter as on a real insect. It is a constant multiplier, not a cap: every score takes the same 10% reduction, low ones included, and it bounds the maximum at 0.9 only because a softmax cannot exceed 1.0. The multiply is deliberate — it degrades the top of the range smoothly, where a hard minimum would pile most predictions at exactly 0.900 and destroy the ordering among them — but it does not make the number meaningful, and the platform's score filters, exports, and best-classification selection all consume it. A real fix is either an explicit rejection class or per-algorithm calibration; that work is tracked in Explore cross-model confidence calibration for multi-model prediction selection ami-ml#75. Being a positive constant, it is argmax-invariant and changes neither the stored logits nor the Lepidoptera gate.CLASSIFIER_CHOICESview still backs the CLI, the registration module, and the tests; a later PR migrates those and deletes the shim.opencv-python5, which needs system GL libraries on a headless host; consider pinningopencv-python-headless.num_workers/prefetch, lower the batch size, pre-downscale before batching, or run a single worker instance. Tracked as a follow-up; it does not block the swap.Related
Summary by CodeRabbit