feat(detections): accept optional crop image for faster smoke checks#575
Open
MateoLostanlen wants to merge 9 commits intomainfrom
Open
feat(detections): accept optional crop image for faster smoke checks#575MateoLostanlen wants to merge 9 commits intomainfrom
MateoLostanlen wants to merge 9 commits intomainfrom
Conversation
Adds a nullable crop_bucket_key column to detections (model + migration),
extends POST /detections/ to accept an optional `crop` file alongside the
main frame, and exposes the resulting presigned URL as `crop_url` on
GET /detections/{id}/url. Cameras that do not send a crop continue to
work unchanged.
Also updates pyroclient.create_detection to accept an optional `crop`
argument and aligns update_last_image / update_pose_image content types
with the JPEG payloads actually sent.
PR #572 collapses prior migrations into a single baseline revision 9700bbccb2f1. Repoint this migration's down_revision so it applies cleanly on top of the new baseline.
…th tests
Move the pose ownership check ahead of any S3 upload so a forbidden
request with another camera's pose_id no longer leaves the main image
(or, after this branch, the optional crop) orphaned in the bucket.
Add tests covering the new crop behavior:
- crop upload persists crop_bucket_key on the detection,
- omitting crop leaves crop_bucket_key null,
- GET /detections/{id}/url returns crop_url when a crop is present and
null otherwise,
- a 403 from a foreign pose_id never invokes upload_file (verified via
monkeypatched stub).
DET_TABLE in conftest gains an explicit crop_bucket_key=None so the
existing detection JSON-equality assertions still match.
The platform consumes detections through GET /sequences/{id}/detections
(not the per-detection /url endpoint), so the crop_url addition needs to
land on that response too. Extend DetectionWithUrl with an optional
crop_url and populate it from crop_bucket_key when present.
Add a with_crop query parameter (default true) so callers that don't
need crops can skip the extra S3 head requests that get_public_url
performs. Mirror the parameter on pyroclient.fetch_sequences_detections.
Tests: assert crop_url is presigned when a crop exists, that with_crop=
false suppresses it, and that the existing equality check now also
strips crop_url alongside url.
… bytes upload_file derives keys from camera_id, second-precision timestamp, and the SHA-256 prefix of the bytes. When a crop happens to be byte-identical to the frame (e.g. a full-frame crop sent in the same request), both calls produced the same key, the second upload overwrote the first, and crop_url ended up pointing at the frame. Add an optional key_prefix parameter to upload_file and call the crop upload with key_prefix="crop_". Regression test asserts the two keys remain distinct when the bytes match.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #575 +/- ##
==========================================
- Coverage 88.49% 88.49% -0.01%
==========================================
Files 51 51
Lines 2182 2190 +8
==========================================
+ Hits 1931 1938 +7
- Misses 251 252 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rts, prefix assertion Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…307a1d6d490d The previous down_revision (9700bbccb2f1) does not exist in the migration history, so `alembic upgrade head` failed at backend boot and the docker healthcheck never went green. Tests pass because they use SQLModel.metadata.create_all and bypass alembic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…al head 307a1d6d490d" This reverts commit af48653.
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
1 participant
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
cropupload toPOST /detectionsso cameras can ship a tight, pre-cropped view of the smoke alongside the full frame. The goal is to give reviewers a faster signal when triaging sequences — they can confirm or dismiss a smoke from the crop without waiting on the full frame.Detectionvia a newcrop_bucket_keycolumn (alembic migration chained onto the current baseline) and surface it ascrop_urlonGET /detections/{id}/urlandGET /sequences/{id}/detections./sequences/{id}/detectionsacceptswith_crop=falseto skip the extra presign round-trips when the consumer doesn't need crops.Notable changes
endpoints/detections.py: authorize the pose before uploading to S3 so a 403 no longer leaves an orphan object in the bucket. Crop upload happens only after authorization passes.services/storage.py:upload_filetakes akey_prefixso frame and crop with identical bytes (same SHA prefix + same second) don't collide on the same bucket key. Crop keys are prefixedcrop_.client/pyroclient:create_detection(...)gains acrop: bytes | Nonekwarg;fetch_sequences_detections(...)gainswith_crop: bool = True. Also fixed the camera/pose image uploads to sendimage/jpeginstead of mislabelling JPEG bytes asimage/png.