From f03faee2de4904c9c9ce6f98a911593c773763fd Mon Sep 17 00:00:00 2001 From: Mateo Date: Tue, 5 May 2026 17:11:10 +0200 Subject: [PATCH 1/3] fix: keep alert id stable when labeling a solo sequence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a sequence labeled non-wildfire is the only one in its alert, the previous logic detached it, deleted the alert (now empty), and created an identical replacement — changing the alert id for no gain. Multi-sequence alerts still split so the FP no longer contributes to triangulation. --- src/app/api/api_v1/endpoints/sequences.py | 61 ++++++++++++++--------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/src/app/api/api_v1/endpoints/sequences.py b/src/app/api/api_v1/endpoints/sequences.py index 8bc8c7e5..eabf5cf3 100644 --- a/src/app/api/api_v1/endpoints/sequences.py +++ b/src/app/api/api_v1/endpoints/sequences.py @@ -258,30 +258,45 @@ async def label_sequence( updated = await sequences.update(sequence_id, payload) - # If sequence is labeled as non-wildfire, remove it from alerts and refresh those alerts - if payload.is_wildfire is not None and payload.is_wildfire != AnnotationType.WILDFIRE_SMOKE: - alert_ids_res = await session.exec( - select(AlertSequence.alert_id).where(AlertSequence.sequence_id == sequence_id) - ) - alert_ids = list(alert_ids_res.all()) - if alert_ids: - delete_links: Any = delete(AlertSequence).where(cast(Any, AlertSequence.sequence_id) == sequence_id) - await session.exec(delete_links) - await session.commit() - for aid in alert_ids: - await _refresh_alert_state(aid, session, alerts) - # Create a fresh alert for this sequence alone - camera = cast(Camera, await cameras.get(sequence.camera_id, strict=True)) - 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, - ) + if payload.is_wildfire is None or payload.is_wildfire == AnnotationType.WILDFIRE_SMOKE: + return updated + + alert_ids_res = await session.exec( + select(AlertSequence.alert_id).where(AlertSequence.sequence_id == sequence_id) + ) + alert_ids = list(alert_ids_res.all()) + + # If the sequence is the only one in all of its alerts, leave them as-is — + # detaching and recreating would just churn the alert id for no benefit. + if alert_ids: + siblings_stmt: Any = ( + select(AlertSequence.sequence_id) + .where(cast(Any, AlertSequence.alert_id).in_(alert_ids)) + .where(AlertSequence.sequence_id != sequence_id) + .limit(1) ) - session.add(AlertSequence(alert_id=new_alert.id, sequence_id=sequence_id)) + siblings_res = await session.exec(siblings_stmt) + if siblings_res.first() is None: + return updated + + delete_links: Any = delete(AlertSequence).where(cast(Any, AlertSequence.sequence_id) == sequence_id) + await session.exec(delete_links) await session.commit() + for aid in alert_ids: + await _refresh_alert_state(aid, session, alerts) + + # Create a fresh alert for this sequence alone + camera = cast(Camera, await cameras.get(sequence.camera_id, strict=True)) + 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() return updated From dcbcbc334233c11b37eb05f37d9ca08190d55636 Mon Sep 17 00:00:00 2001 From: Mateo Date: Tue, 5 May 2026 17:19:29 +0200 Subject: [PATCH 2/3] style: collapse label_sequence alert_ids exec call to one line --- src/app/api/api_v1/endpoints/sequences.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/app/api/api_v1/endpoints/sequences.py b/src/app/api/api_v1/endpoints/sequences.py index eabf5cf3..e988c969 100644 --- a/src/app/api/api_v1/endpoints/sequences.py +++ b/src/app/api/api_v1/endpoints/sequences.py @@ -261,9 +261,7 @@ async def label_sequence( if payload.is_wildfire is None or payload.is_wildfire == AnnotationType.WILDFIRE_SMOKE: return updated - alert_ids_res = await session.exec( - select(AlertSequence.alert_id).where(AlertSequence.sequence_id == sequence_id) - ) + alert_ids_res = await session.exec(select(AlertSequence.alert_id).where(AlertSequence.sequence_id == sequence_id)) alert_ids = list(alert_ids_res.all()) # If the sequence is the only one in all of its alerts, leave them as-is — From 2bdc5075cc030135eb02df5f42024367ca3ce84d Mon Sep 17 00:00:00 2001 From: Mateo Date: Tue, 5 May 2026 17:19:35 +0200 Subject: [PATCH 3/3] test: cover label_sequence solo-alert no-churn behavior Adds an integration test asserting the alert id is preserved when labeling a single-sequence alert as non-wildfire, and a unit test for the same path. Updates the existing multi-sequence unit test to mock the new siblings probe. --- src/tests/endpoints/test_sequences.py | 115 +++++++++++++++++++++++++- 1 file changed, 111 insertions(+), 4 deletions(-) diff --git a/src/tests/endpoints/test_sequences.py b/src/tests/endpoints/test_sequences.py index 5860e155..12844980 100644 --- a/src/tests/endpoints/test_sequences.py +++ b/src/tests/endpoints/test_sequences.py @@ -447,6 +447,61 @@ async def test_sequence_label_updates_alerts(async_client: AsyncClient, detectio assert new_alert_row.lon is None +@pytest.mark.asyncio +async def test_sequence_label_keeps_solo_alert(async_client: AsyncClient, detection_session: AsyncSession): + # A sequence alone in its alert should keep that alert as-is when labeled non-wildfire. + camera = await detection_session.get(Camera, 1) + assert camera is not None + now = utcnow() + seq = Sequence( + camera_id=camera.id, + pose_id=None, + camera_azimuth=180.0, + sequence_azimuth=170.0, + cone_angle=5.0, + is_wildfire=None, + started_at=now - timedelta(seconds=30), + last_seen_at=now - timedelta(seconds=10), + ) + detection_session.add(seq) + await detection_session.commit() + await detection_session.refresh(seq) + + alert = Alert( + organization_id=camera.organization_id, + lat=1.0, + lon=2.0, + started_at=seq.started_at, + last_seen_at=seq.last_seen_at, + ) + detection_session.add(alert) + await detection_session.commit() + await detection_session.refresh(alert) + detection_session.add(AlertSequence(alert_id=alert.id, sequence_id=seq.id)) + await detection_session.commit() + + auth = pytest.get_token( + pytest.user_table[0]["id"], pytest.user_table[0]["role"].split(), pytest.user_table[0]["organization_id"] + ) + original_alert_id = alert.id + + resp = await async_client.patch( + f"/sequences/{seq.id}/label", + json={"is_wildfire": SequenceLabel(is_wildfire="other_smoke").is_wildfire}, + headers=auth, + ) + assert resp.status_code == 200, resp.text + + alerts_res = await detection_session.exec(select(Alert).execution_options(populate_existing=True)) + alerts_rows = alerts_res.all() + assert [row.id for row in alerts_rows] == [original_alert_id] + + mappings_res = await detection_session.exec( + select(AlertSequence.alert_id, AlertSequence.sequence_id).execution_options(populate_existing=True) + ) + assert {(aid, sid) for aid, sid in mappings_res.all()} == {(original_alert_id, seq.id)} + + @pytest.mark.asyncio async def test_delete_sequence_cleans_alerts_and_detections(async_client: AsyncClient, detection_session: AsyncSession): camera = await detection_session.get(Camera, 1) @@ -619,11 +674,12 @@ async def test_unit_label_sequence_as_other_smoke_refreshes_alert( mock_alerts_crud = AsyncMock() mock_alerts_crud.create.return_value = MagicMock(id=99) # New alert created - # Mock for session.exec to return an alert_id + # Mock for session.exec to return an alert_id, then a sibling sequence id, then delete mock_session = AsyncMock() mock_session.add = MagicMock() # .add is synchronous mock_exec_result = MagicMock() mock_exec_result.all.return_value = [101] # Belongs to alert 101 + mock_exec_result.first.return_value = 2 # Sibling sequence id exists in alert 101 mock_session.exec.return_value = mock_exec_result mock_token_payload = TokenPayload(sub=1, scopes=[UserRole.AGENT], organization_id=1) @@ -644,9 +700,8 @@ async def test_unit_label_sequence_as_other_smoke_refreshes_alert( mock_sequences_crud.get.assert_called_once_with(1, strict=True) mock_sequences_crud.update.assert_called_once_with(1, payload) - # Verify it was removed from the old alert and the alert was refreshed - # Two session.exec calls: one to get alert_ids, one to delete links - assert mock_session.exec.call_count == 2 + # Three session.exec calls: alert_ids lookup, siblings probe, delete links + assert mock_session.exec.call_count == 3 mock_refresh_alert_state.assert_called_once_with(101, mock_session, mock_alerts_crud) # Verify a new alert was created for this sequence @@ -657,6 +712,58 @@ async def test_unit_label_sequence_as_other_smoke_refreshes_alert( assert updated_sequence.is_wildfire == AnnotationType.OTHER_SMOKE +@pytest.mark.asyncio +@patch("app.api.api_v1.endpoints.sequences._refresh_alert_state", new_callable=AsyncMock) +async def test_unit_label_sequence_solo_alert_keeps_alert( + mock_refresh_alert_state: AsyncMock, +): + """Labeling a sequence as non-wildfire when it is alone in its alert must not + detach it, refresh the alert, or create a replacement — the alert id must stay stable.""" + mock_sequence = Sequence(id=1, camera_id=1, is_wildfire=None, started_at=utcnow(), last_seen_at=utcnow()) + + mock_sequences_crud = AsyncMock() + mock_sequences_crud.get.return_value = mock_sequence + mock_sequences_crud.update.return_value = Sequence( + id=1, + camera_id=1, + is_wildfire=AnnotationType.OTHER_SMOKE, + started_at=mock_sequence.started_at, + last_seen_at=mock_sequence.last_seen_at, + ) + + mock_cameras_crud = AsyncMock() + mock_alerts_crud = AsyncMock() + + # Two exec calls: alert_ids -> [101], siblings -> None (no other sequence in the alert) + alert_ids_result = MagicMock() + alert_ids_result.all.return_value = [101] + siblings_result = MagicMock() + siblings_result.first.return_value = None + + mock_session = AsyncMock() + mock_session.add = MagicMock() + mock_session.exec.side_effect = [alert_ids_result, siblings_result] + + mock_token_payload = TokenPayload(sub=1, scopes=[UserRole.ADMIN], organization_id=1) + payload = SequenceLabel(is_wildfire=AnnotationType.OTHER_SMOKE) + + updated_sequence = await label_sequence( + payload=payload, + sequence_id=1, + cameras=mock_cameras_crud, + sequences=mock_sequences_crud, + alerts=mock_alerts_crud, + session=mock_session, + token_payload=mock_token_payload, + ) + + assert mock_session.exec.call_count == 2 + mock_refresh_alert_state.assert_not_called() + mock_alerts_crud.create.assert_not_called() + mock_session.add.assert_not_called() + assert updated_sequence.is_wildfire == AnnotationType.OTHER_SMOKE + + @pytest.mark.asyncio async def test_unit_label_sequence_as_wildfire_smoke_does_not_refresh(): """Verify that labeling a sequence as wildfire smoke does NOT trigger an alert refresh."""