feat(alerts/export): expand CSV with sequence + camera context (#591)#593
Open
Acruve15 wants to merge 10 commits into
Open
feat(alerts/export): expand CSV with sequence + camera context (#591)#593Acruve15 wants to merge 10 commits into
Acruve15 wants to merge 10 commits into
Conversation
Long format: one row per (alert, sequence). Alerts with no sequences still emit one row with empty sequence/camera cells. New columns: alert_started_at_date, alert_started_at_time, alert_duration_seconds, alert_triangulated_lat/lon, organization_id, sequence_id/started_at/last_seen_at/triangulated_azimuth/label, pose_id, camera_id, camera_name. is_wildfire is collapsed to wildfire / other / unknown via a dict mapper. Tests will be updated in the next commit.
Only camera_name is read in the CSV; selecting (Camera.id, Camera.name) and returning Dict[int, str] avoids loading columns we ignore.
Alerts are born from sequences and delete_alert wipes AlertSequence rows before the alert itself, so an alert with 0 sequences shouldn't exist outside a brief delete window. The defensive branch was dead code; if such an alert ever appears in production it's a data-integrity issue worth noticing rather than papering over with blank cells.
Update the seven existing export tests to the 16-column header, swap the positional CSV parser for csv.DictReader so assertions look up by column name. Each existing test now attaches a sequence to its alerts via a new focused _attach_sequence helper (alerts always have ≥1 sequence in production). Three new tests cover the long-format semantics: - emits_one_row_per_sequence: 3 sequences -> 3 rows, ASC by started_at - wildfire_label_mapping: WILDFIRE_SMOKE/OTHER_SMOKE/None -> wildfire/other/unknown - camera_name_resolution: rows pick the correct camera_name
Import _ALERT_EXPORT_COLUMNS from the endpoint module instead of maintaining a copy. In happy_path, replace nine per-cell asserts with a single dict equality on the first row (pytest renders the diff cleanly, and the equality implicitly covers the full column set). Keep the explicit header assertion only in empty_range, where it's the test's raison d'etre.
Scratch drafts and other ephemeral notes live under tmp/ during local work and shouldn't ship in PR diffs.
Replace the per-test boilerplate (datetime anchor, JWT token build, the HTTP request + status + parse triplet) with three fixtures and one helper: - export_base_dt: anchor datetime shared by 7 tests - org1_admin_auth / org1_agent_auth: token headers shared by 8 tests - _get_export: wraps the GET + 200 assert + CSV parse used by 6 tests Parametrize test_alerts_export_wildfire_label_mapping over all four (is_wildfire, expected_label) pairs. This also closes a coverage hole: the previous version exercised WILDFIRE_SMOKE / OTHER_SMOKE / None but not the AnnotationType.OTHER -> "other" mapping.
… tests Five of the export tests were exercising _iter_alerts_csv behavior (header, null coords, row-per-sequence ordering, wildfire label, camera-name lookup) through the full HTTP route + JWT + DB stack. Move them to plain unit tests that call _iter_alerts_csv directly: - no async, no detection_session, no async_client, no fixtures - Alert / Sequence instances built with small in-memory helpers - failures point at the serializer, not the route Keep five integration tests that genuinely exercise route wiring (happy path with content headers, SQL date filter, JWT org scoping, 422 validation, 401 auth required).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #593 +/- ##
==========================================
- Coverage 90.11% 90.10% -0.01%
==========================================
Files 55 55
Lines 2498 2517 +19
==========================================
+ Hits 2251 2268 +17
- Misses 247 249 +2
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:
|
The iterator was building alert-level cells, sequence-level cells (with wildfire label and camera-name lookups), and running the CSV streaming buffer in one body. Extract _alert_cells and _sequence_cells as pure helpers next to the data they shape, factor the buffer drain into a local closure, and leave _iter_alerts_csv as the orchestrator it should be: header, iterate, drain.
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
GET /alerts/exportfrom the 5-column alert-only CSV shipped in feat: add CSV export endpoint for alerts (#521) #577 to a 16-column long format with one row per(alert, sequence)pair, picking up sequence + camera context for analysts who want to slice past alerts in Excel/Sheets without going back to the API._fetch_sequences_by_alert_idshelper; adds a focused_fetch_camera_names_by_idsthat selects only(id, name). A_WILDFIRE_LABELSdict collapsesSequence.is_wildfire→wildfire/other/unknown.Departures from the issue spec
Both decided after looking at how the file actually gets used (Excel/Sheets pivots). Happy to revert either if @MateoLostanlen prefers the original shape.
|-separated multi-value cells. The issue proposedid1|id2|id3packed cells positionally aligned acrosssequence_*andcamera_*columns. That shape is fragile (a|in acamera_namesilently breaks parsing) and kills Excel's filter + pivot onsequence_label/camera_name. One row per(alert, sequence)is the spreadsheet-native way to encode a 1:N relation; the droppedsequence_count/camera_countcolumns are trivialCOUNTIFs.alert_lat/alert_lon→alert_triangulated_lat/alert_triangulated_lon(pre-answers the "why is it empty?" question raised in the issue thread).sequence_azimuth→sequence_triangulated_azimuth(theSequencemodel also hascamera_azimuth, the cone center; the prefix removes the ambiguity).Schema
Each row joins one alert with one of its sequences. Alert-level cells repeat across the alert's rows; sequences within an alert are sorted ASC by
started_at.alert_id,alert_started_at_date,alert_started_at_time,alert_last_seen_at,alert_duration_seconds,alert_triangulated_lat,alert_triangulated_lon,organization_idsequence_id,sequence_started_at,sequence_last_seen_at,sequence_triangulated_azimuth,sequence_label,pose_idcamera_id,camera_namesequence_labelmapsAnnotationTypeto a 3-bucket label:WILDFIRE_SMOKE → wildfire,OTHER_SMOKE/OTHER → other,None → unknown.Tests
Split into two clearly-labeled sections:
Unit tests for
_iter_alerts_csv(sync, in-memoryAlert/Sequence, no DB / HTTP / auth):started_atAnnotationType | Nonecases (catches the previously-uncoveredAnnotationType.OTHERmapping)camera_nameresolved per sequence fromcamera_names_by_idIntegration tests for
GET /alerts/export(route → JWT → DB):attachmentdisposition, full first-row dict equality)to_date < from_date→ 422Test plan
pytestruff 0.11.9 checkandruff 0.11.9 format --checkpass locally (repo pins 0.11.9 in pre-commit)curlagainst a dev stack with a valid token; open the CSV in Sheets and confirm filter / pivot work onsequence_labelandcamera_name