-
-
Notifications
You must be signed in to change notification settings - Fork 14
feat(sequences): re-run cone matching when relabeling back to wildfire_smoke - #585
feat(sequences): re-run cone matching when relabeling back to wildfire_smoke #585MateoLostanlen wants to merge 6 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## main #585 +/- ## ========================================== + Coverage 92.30% 92.31% +0.01% ========================================== Files 56 56 Lines 2585 2603 +18 ========================================== + Hits 2386 2403 +17 - Misses 199 200 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Acruve15
commented
May 14, 2026
It's an interesting PR, thx! I can do a thorough review by Monday if it's ok for you.
I want to make sure I understand what happens with the triangulation and how alerts/sequences/detections concepts are linked :)
Resolve label_sequence conflict by combining both changes: keep main's sibling-check for the non-wildfire path (skip detach/recreate when the sequence is alone in its alert, avoiding alert-id churn) and add this branch's revert-to-wildfire path (detach + re-run cone matching anchored on the sequence's own last_seen_at). Keep both unit tests.
Acruve15
commented
Jun 9, 2026
@MateoLostanlen I'll review by EOD
@Acruve15
Acruve15
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @MateoLostanlen, I added some comments, the most important one on how to handle orphan alerts :)
Can be helpful, here is the current flow for context:
sequenceDiagram
autonumber
participant U as User
participant L as label_sequence
participant CM as cone matching
participant A as AlertSequence
Note over U,A: T0: Alert#100 triangulates {seq_B, seq_C}
U->>L: label seq_B OTHER_SMOKE
L->>A: detach seq_B from Alert#100
L->>A: create lonely Alert#101 = {seq_B}
Note over A: Alert#100 = {seq_C}, Alert#101 = {seq_B}
Note over U,A: T1 (later): user reconsiders
U->>L: label seq_B WILDFIRE_SMOKE
rect rgb(255, 235, 235)
Note over L,A: pre-PR: no-op, seq_B stuck in Alert#101 forever
end
rect rgb(235, 250, 235)
L->>A: detach seq_B from Alert#101 (deleted, empty)
L->>CM: anchor on seq_B.last_seen_at
alt overlap found
CM-->>A: link seq_B to Alert#100
Note over A: Alert#100 = {seq_B, seq_C} again
else no overlap (orphan risk)
CM-->>A: returns None, no link created
Note over A: seq_B has zero alert rows
end
end
Longer term: I was thinking we could soft-delete AlertSequence rows (with unlinked_at timestamps) would give us audit history for free and is useful when debugging "why did this alert lose triangulation?"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this returns None (eg no neighbour in the window or cones don't overlap?) , the sequence has zero alert links. I suggest adding a fallback to make sure we don't have orphan alters.
alert_id = await attach_sequence_to_alert( updated, camera, cameras, sequences, alerts, reference_time=sequence.last_seen_at ) if alert_id is None: new_alert = await alerts.create( AlertCreate( organization_id=camera.organization_id, started_at=sequence.started_at, last_seen_at=sequence.last_seen_at, lat=None, lon=None, ) ) session.add(AlertSequence(alert_id=new_alert.id, sequence_id=sequence_id)) await session.commit()
If orphaning is the intended behaviour, a one-line comment is enough for me.
other_smoke), it's detached from its triangulating alert and a fresh single-sequence alert is created. Reverting the label towildfire_smokepreviously left the sequence stuck in that lonely alert, breaking triangulation. This PR makes the revert path drop the lonely alert and re-run cone matching so the sequence merges back with its overlapping neighbours.last_seen_at(notutcnow()) so relabels done hours later still find the original neighbours withinSEQUENCE_RELAXATION_SECONDS._attach_sequence_to_alert→attach_sequence_to_alertand extract_detach_sequence_from_alertsto deduplicate the detach/refresh block.