From 4953c19a71fac9dbba52cd9e4bdc3c7575d6a52d Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Thu, 29 Oct 2026 09:04:16 -0500 Subject: [PATCH] Miscellaneous fixes (#24820) * keep base path in logout redirect * store manual event sub labels as verified objects Manual events created with a sub label were stored in the review item as "label: sub_label", so review previews and collapsed History cards showed a generic icon, the review label filter missed them, and the sub label never reached sub_labels for chips, notifications, or GenAI prompts. The object processor now publishes the label and sub label separately, and the review maintainer stores them as label-verified plus a sub_labels entry, the same as tracked objects. * only expire snapshots for events that still have one Snapshot expiry selected every event older than snapshot retention, including events whose snapshot was already deleted, so with snapshot retention shorter than alert/detection retention the same missing files were unlinked again every cleanup cycle until the event row expired. That was nearly free on local disk but caused constant metadata load on network shares. The expiry queries now filter on has_snapshot so each snapshot is deleted once. --- frigate/api/auth.py | 3 +- frigate/events/cleanup.py | 3 + frigate/review/maintainer.py | 24 ++--- frigate/test/test_event_cleanup_snapshots.py | 87 +++++++++++++++++++ frigate/test/test_review_flow.py | 62 +++++++++++-- .../test/test_review_manual_event_severity.py | 18 ---- frigate/track/object_processing.py | 6 +- 7 files changed, 165 insertions(+), 38 deletions(-) create mode 100644 frigate/test/test_event_cleanup_snapshots.py diff --git a/frigate/api/auth.py b/frigate/api/auth.py index c3f8f2d623..49fd641c33 100644 --- a/frigate/api/auth.py +++ b/frigate/api/auth.py @@ -934,7 +934,8 @@ def profile(request: Request): ) def logout(request: Request): auth_config: AuthConfig = request.app.frigate_config.auth - response = RedirectResponse("/login", status_code=303) + base_path = request.headers.get("x-ingress-path", "").rstrip("/") + response = RedirectResponse(f"{base_path}/login", status_code=303) response.delete_cookie(auth_config.cookie_name) return response diff --git a/frigate/events/cleanup.py b/frigate/events/cleanup.py index b5b5cbfc32..5f4716784f 100644 --- a/frigate/events/cleanup.py +++ b/frigate/events/cleanup.py @@ -93,6 +93,7 @@ class EventCleanup(threading.Thread): Event.start_time < expire_after, Event.label == event.label, Event.retain_indefinitely == False, + Event.has_snapshot == True, ) .namedtuples() .iterator() @@ -114,6 +115,7 @@ class EventCleanup(threading.Thread): Event.start_time < expire_after, Event.label == event.label, Event.retain_indefinitely == False, + Event.has_snapshot == True, ) events_to_update: list[str] = [] @@ -169,6 +171,7 @@ class EventCleanup(threading.Thread): Event.start_time < expire_after, Event.label == event.label, Event.retain_indefinitely == False, + Event.has_snapshot == True, ) .namedtuples() .iterator() diff --git a/frigate/review/maintainer.py b/frigate/review/maintainer.py index be246bf5ac..f7a665c6db 100644 --- a/frigate/review/maintainer.py +++ b/frigate/review/maintainer.py @@ -100,6 +100,14 @@ class PendingReviewSegment: if zone not in self.zones: self.zones.append(zone) + def add_manual_detection(self, info: dict[str, Any]) -> None: + """Add a manual event's label and sub label to the segment.""" + if info.get("sub_label"): + self.detections[info["event_id"]] = f"{info['label']}-verified" + self.sub_labels[info["event_id"]] = info["sub_label"] + else: + self.detections[info["event_id"]] = info["label"] + def update_frame( self, camera_config: CameraConfig, @@ -483,8 +491,6 @@ class ReviewSegmentMaintainer(threading.Thread): alerts so manual events keep their historical severity. """ review_config = self.config.cameras[camera].review - # label contains 'label: sub_label', only the label is categorized - label = label.split(": ")[0] if review_config.alerts.enabled and label in review_config.alerts.labels: return SeverityEnum.alert @@ -894,9 +900,7 @@ class ReviewSegmentMaintainer(threading.Thread): current_segment.last_detection_time = frame_time elif topic == DetectionTypeEnum.api or topic == DetectionTypeEnum.lpr: if manual_info["state"] == ManualEventState.complete: - current_segment.detections[manual_info["event_id"]] = ( - manual_info["label"] - ) + current_segment.add_manual_detection(manual_info) if topic == DetectionTypeEnum.api: severity = self.get_manual_event_severity( camera, manual_info["label"] @@ -921,9 +925,7 @@ class ReviewSegmentMaintainer(threading.Thread): self.indefinite_events[camera][manual_info["event_id"]] = ( manual_info["label"] ) - current_segment.detections[manual_info["event_id"]] = ( - manual_info["label"] - ) + current_segment.add_manual_detection(manual_info) if topic == DetectionTypeEnum.api: if ( self.get_manual_event_severity( @@ -1014,11 +1016,12 @@ class ReviewSegmentMaintainer(threading.Thread): camera, frame_time, severity, - {manual_info["event_id"]: manual_info["label"]}, + {}, {}, [], set(), ) + api_segment.add_manual_detection(manual_info) self._activate_segment(api_segment) if manual_info["state"] == ManualEventState.start: @@ -1041,11 +1044,12 @@ class ReviewSegmentMaintainer(threading.Thread): camera, frame_time, SeverityEnum.detection, - {manual_info["event_id"]: manual_info["label"]}, + {}, {}, [], set(), ) + lpr_segment.add_manual_detection(manual_info) self._activate_segment(lpr_segment) if manual_info["state"] == ManualEventState.start: diff --git a/frigate/test/test_event_cleanup_snapshots.py b/frigate/test/test_event_cleanup_snapshots.py new file mode 100644 index 0000000000..babf3daf90 --- /dev/null +++ b/frigate/test/test_event_cleanup_snapshots.py @@ -0,0 +1,87 @@ +"""Tests for snapshot expiration in event cleanup.""" + +import datetime +import unittest +from unittest.mock import MagicMock, patch + +from peewee import SqliteDatabase + +from frigate.config import FrigateConfig +from frigate.events.cleanup import EventCleanup +from frigate.models import Event, Timeline + + +class TestEventCleanupSnapshots(unittest.TestCase): + def setUp(self): + self.db = SqliteDatabase(":memory:") + models = [Event, Timeline] + self.db.bind(models) + self.db.create_tables(models) + + config = FrigateConfig( + **{ + "mqtt": {"host": "mqtt"}, + "snapshots": {"enabled": True, "retain": {"default": 14}}, + "cameras": { + "front_door": { + "ffmpeg": { + "inputs": [ + { + "path": "rtsp://10.0.0.1:554/video", + "roles": ["detect"], + }, + ] + }, + } + }, + } + ) + self.cleanup = EventCleanup(config, MagicMock(), MagicMock()) + + def tearDown(self): + self.db.close() + + def _insert_event(self, id: str, camera: str, has_snapshot: bool) -> None: + end_time = (datetime.datetime.now() - datetime.timedelta(days=20)).timestamp() + Event.create( + id=id, + label="person", + camera=camera, + start_time=end_time - 10, + end_time=end_time, + top_score=0.9, + score=0.9, + false_positive=False, + zones=[], + thumbnail="", + has_clip=True, + has_snapshot=has_snapshot, + region=[], + box=[], + area=0, + retain_indefinitely=False, + plus_id="", + model_hash="", + detector_type="cpu", + model_type="ssd", + data={}, + ) + + @patch("frigate.events.cleanup.delete_event_snapshot", return_value=True) + def test_expired_snapshots_are_only_deleted_once(self, delete_snapshot): + # events stay in the db while their clip is retained, so events whose + # snapshot already expired must not be selected again + for camera in ("front_door", "removed_camera"): + self._insert_event(f"{camera}-pending", camera, has_snapshot=True) + self._insert_event(f"{camera}-expired", camera, has_snapshot=False) + + self.cleanup.expire_snapshots() + + deleted = sorted(c.args[0].id for c in delete_snapshot.call_args_list) + assert deleted == ["front_door-pending", "removed_camera-pending"] + assert not Event.select().where(Event.has_snapshot == True).exists() + + delete_snapshot.reset_mock() + self.cleanup.expire_snapshots() + + delete_snapshot.assert_not_called() diff --git a/frigate/test/test_review_flow.py b/frigate/test/test_review_flow.py index d71884ba7f..ebf7b981b7 100644 --- a/frigate/test/test_review_flow.py +++ b/frigate/test/test_review_flow.py @@ -19,6 +19,7 @@ import numpy as np from frigate.comms.detections_updater import DetectionTypeEnum from frigate.config import FrigateConfig from frigate.review.maintainer import ReviewSegmentMaintainer +from frigate.track.object_processing import ManualEventState CAMERA = "front_door" @@ -122,13 +123,19 @@ class ReviewFlowTestCase(unittest.TestCase): def feed(self, *frames: tuple[float, list[dict[str, Any]]]) -> None: """Run the maintainer loop over the given (frame_time, objects) frames.""" - queue = [ - ( - DetectionTypeEnum.video.value, - (CAMERA, f"{CAMERA}_{frame_time}", frame_time, objects, [], []), - ) - for frame_time, objects in frames - ] + self.feed_messages( + *[ + ( + DetectionTypeEnum.video.value, + (CAMERA, f"{CAMERA}_{frame_time}", frame_time, objects, [], []), + ) + for frame_time, objects in frames + ] + ) + + def feed_messages(self, *messages: tuple[str, tuple[Any, ...]]) -> None: + """Run the maintainer loop over the given (topic, data) messages.""" + queue = list(messages) self.maintainer.stop_event = threading.Event() def next_update(timeout: float) -> Any: @@ -853,5 +860,46 @@ class TestSplitDetectionLabels(ReviewFlowTestCase): self.assertCountEqual(detection["data"]["zones"], ["yard", "driveway"]) +class TestManualEvents(ReviewFlowTestCase): + def manual_event( + self, event_id: str, frame_time: float, sub_label: str | None = None + ) -> tuple[str, tuple[Any, ...]]: + """Build a completed manual event as published by the object processor.""" + return ( + DetectionTypeEnum.api.value, + ( + CAMERA, + frame_time, + { + "state": ManualEventState.complete, + "label": "person", + "sub_label": sub_label, + "event_id": event_id, + "end_time": frame_time + 5, + }, + ), + ) + + def test_sub_label_is_verified_on_new_segment(self) -> None: + self.feed_messages(self.manual_event("m1", 1, "Bob")) + + data = self.maintainer.active_review_segments[CAMERA].get_data(False)["data"] + self.assertEqual(data["objects"], ["person-verified"]) + self.assertEqual(data["verified_objects"], ["person-verified"]) + self.assertEqual(data["sub_labels"], ["Bob"]) + + def test_sub_label_is_verified_on_active_segment(self) -> None: + self.feed( + (1, [self.tracked("dog1", "dog", 1)]), + ) + self.feed_messages( + self.manual_event("m1", 2, "Bob"), self.manual_event("m2", 2) + ) + + data = self.maintainer.active_review_segments[CAMERA].get_data(False)["data"] + self.assertCountEqual(data["objects"], ["dog", "person-verified", "person"]) + self.assertEqual(data["sub_labels"], ["Bob"]) + + if __name__ == "__main__": unittest.main() diff --git a/frigate/test/test_review_manual_event_severity.py b/frigate/test/test_review_manual_event_severity.py index 9dd61e18c5..51656e3868 100644 --- a/frigate/test/test_review_manual_event_severity.py +++ b/frigate/test/test_review_manual_event_severity.py @@ -100,24 +100,6 @@ class TestManualEventSeverity(unittest.TestCase): SeverityEnum.alert, ) - def test_sub_label_is_stripped_before_categorizing(self) -> None: - maintainer = self._make_maintainer( - """ - review: - alerts: - labels: - - person - detections: - labels: - - person -""" - ) - - self.assertEqual( - maintainer.get_manual_event_severity("front_door", "person: Bob"), - SeverityEnum.alert, - ) - def test_alert_label_is_detection_when_alerts_disabled(self) -> None: maintainer = self._make_maintainer( """ diff --git a/frigate/track/object_processing.py b/frigate/track/object_processing.py index 35773fb9ff..7f8c1d6071 100644 --- a/frigate/track/object_processing.py +++ b/frigate/track/object_processing.py @@ -635,7 +635,8 @@ class TrackedObjectProcessor(threading.Thread): if end_time else ManualEventState.start ), - "label": f"{label}: {sub_label}" if sub_label else label, + "label": label, + "sub_label": sub_label, "event_id": event_id, "end_time": end_time, }, @@ -696,7 +697,8 @@ class TrackedObjectProcessor(threading.Thread): frame_time, { "state": ManualEventState.start, - "label": f"{label}: {sub_label}" if sub_label else label, + "label": label, + "sub_label": sub_label, "event_id": event_id, "end_time": None, },