Look up images by image_id when pairing with detections#153
Conversation
📝 WalkthroughWalkthrough
ChangesDetection image pairing
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/db/models/detections.py`:
- Around line 344-345: Update the loop pairing image_ids with
detected_objects_data to reject mismatched sequence lengths instead of silently
truncating via zip. Use strict zip behavior if supported by the project’s Python
version, or validate equal lengths before iteration, while preserving the
existing image lookup and processing logic.
- Around line 341-345: Validate that every requested ID in image_ids exists in
the fetched images set before the processing loop indexes images_by_id. In the
detection creation flow, compare image_ids against images_by_id keys and fail
fast with a clear error identifying missing IDs, while preserving the existing
loop for valid inputs.
🪄 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: f60a3669-e9eb-4a3f-870d-02db56874e00
📒 Files selected for processing (1)
trapdata/db/models/detections.py
| images_by_id = {img.id: img for img in images} | ||
|
|
||
| # Process all images and create objects | ||
| for image, detected_objects in zip(images, detected_objects_data): | ||
| for image_id, detected_objects in zip(image_ids, detected_objects_data): | ||
| image = images_by_id[image_id] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== detections.py around target lines ==\n'
sed -n '280,390p' trapdata/db/models/detections.py
printf '\n== search for image ID validation / missing IDs handling ==\n'
rg -n "missing.*image|image_ids|images_by_id|KeyError|domain error|not found|stale|deleted image|validate.*image" trapdata/db/models -S
printf '\n== outline of detections.py ==\n'
ast-grep outline trapdata/db/models/detections.py --view expandedRepository: RolnickLab/ami-data-companion
Length of output: 8422
🏁 Script executed:
set -euo pipefail
sed -n '280,390p' trapdata/db/models/detections.py
rg -n "missing.*image|image_ids|images_by_id|KeyError|domain error|not found|stale|deleted image|validate.*image" trapdata/db/models -SRepository: RolnickLab/ami-data-companion
Length of output: 4810
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('trapdata/db/models/detections.py')
text = p.read_text()
lines = text.splitlines()
for i in range(280, 391):
if i <= len(lines):
print(f"{i:4}: {lines[i-1]}")
PY
printf '\n---\n'
rg -n "missing.*image|image_ids|images_by_id|KeyError|domain error|not found|stale|deleted image|validate.*image" trapdata/db/models -SRepository: RolnickLab/ami-data-companion
Length of output: 5481
🏁 Script executed:
printf 'probe\n'Repository: RolnickLab/ami-data-companion
Length of output: 173
🏁 Script executed:
set -euo pipefail
printf '\n== callers of save_detected_objects ==\n'
rg -n "save_detected_objects\(" -S .
printf '\n== get_monitoring_session_image_ids definition ==\n'
sed -n '300,360p' trapdata/db/models/events.py
printf '\n== surrounding uses of image ids in events.py ==\n'
sed -n '140,190p' trapdata/db/models/events.pyRepository: RolnickLab/ami-data-companion
Length of output: 4378
🏁 Script executed:
set -euo pipefail
printf '\n== trapdata/ml/models/localization.py around save_detected_objects call ==\n'
sed -n '150,230p' trapdata/ml/models/localization.py
printf '\n== trapdata/api/queries.py save_detected_objects wrapper ==\n'
sed -n '1,80p' trapdata/api/queries.py
printf '\n== any other image-id list producer/consumer around detections ==\n'
rg -n "get_monitoring_session_image_ids|image_ids|detected_objects_data" trapdata -SRepository: RolnickLab/ami-data-companion
Length of output: 11206
🏁 Script executed:
set -euo pipefail
printf '\n== custom exceptions / domain error patterns ==\n'
rg -n "class .*Error|raise .*Error|ValueError|LookupError|NotFound|Missing.*Error|domain error" trapdata -S
printf '\n== any helper for missing IDs / validation ==\n'
rg -n "validate.*id|missing ids|missing.*ids|check.*ids|raise.*missing" trapdata -SRepository: RolnickLab/ami-data-companion
Length of output: 2796
Validate requested image IDs before indexing. images_by_id[image_id] will raise KeyError if the query returns a subset of image_ids; compare the requested IDs to the fetched set and fail fast with a clear error before creating detections.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 344-344: 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/db/models/detections.py` around lines 341 - 345, Validate that every
requested ID in image_ids exists in the fetched images set before the processing
loop indexes images_by_id. In the detection creation flow, compare image_ids
against images_by_id keys and fail fast with a clear error identifying missing
IDs, while preserving the existing loop for valid inputs.
| for image_id, detected_objects in zip(image_ids, detected_objects_data): | ||
| image = images_by_id[image_id] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section with line numbers
sed -n '300,390p' trapdata/db/models/detections.py
# Search for image_ids / detected_objects_data usage and any length validations
rg -n "image_ids|detected_objects_data|zip\\(" trapdata/db/models/detections.py trapdata -g '!**/__pycache__/**'Repository: RolnickLab/ami-data-companion
Length of output: 10086
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '150,220p' trapdata/ml/models/localization.py
printf '\n---- API QUERIES ----\n'
sed -n '1,90p' trapdata/api/queries.py
printf '\n---- DETECTIONS LENGTH CHECK SEARCH ----\n'
rg -n "len\\(.*image_ids|len\\(.*detected_objects_data|strict=True|mismatch" trapdata/db/models/detections.py trapdata/api/queries.py trapdata/ml/models/localization.pyRepository: RolnickLab/ami-data-companion
Length of output: 6268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "save_detected_objects\\(" trapdataRepository: RolnickLab/ami-data-companion
Length of output: 355
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '120,220p' trapdata/ml/models/base.py
printf '\n---- LOCALIZATION CLASS CONTEXT ----\n'
sed -n '1,220p' trapdata/ml/models/localization.pyRepository: RolnickLab/ami-data-companion
Length of output: 11683
Reject mismatched ID and payload lengths. zip() will silently drop extra detections if these sequences ever drift out of sync, so use strict=True or check lengths before iterating.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 344-344: 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/db/models/detections.py` around lines 344 - 345, Update the loop
pairing image_ids with detected_objects_data to reject mismatched sequence
lengths instead of silently truncating via zip. Use strict zip behavior if
supported by the project’s Python version, or validate equal lengths before
iteration, while preserving the existing image lookup and processing logic.
Source: Linters/SAST tools
1 participant
Fixes #152
Summary by CodeRabbit