fix mypy and add docstring

The recording maintainer only loaded review items whose end_time was after the oldest cached segment, so once an item closed in the database the segments in its post_capture window no longer matched it. With continuous recording off and no motion after the item, everything past roughly the review cutoff_time was deleted from the cache instead of being kept for the full post_capture. The query bound now subtracts the camera's largest post_capture, and the per-item check still decides the exact cutoff.

docker/main/rootfs/usr/local/nginx/conf/nginx.conf

@@ -176,12 +176,6 @@ http {
             add_header Cache-Control "public";
             autoindex on;
             root /media/frigate;
-
-            # review thumbnails are rewritten in place while a review is active,
-            # so a cached file size truncates the response
-            location /clips/review/ {
-                open_file_cache off;
-            }
         }
 
         location /cache/ {

frigate/config/camera/record.py

@@ -224,6 +224,14 @@ class RecordConfig(FrigateBaseModel):
             self.detections.pre_capture,
         )
 
+    @property
+    def event_post_capture(self) -> int:
+        """Largest post-capture window used to fetch candidate reviews."""
+        return max(
+            self.alerts.post_capture,
+            self.detections.post_capture,
+        )
+
     def get_review_pre_capture(self, severity: SeverityEnum) -> int:
         if severity == SeverityEnum.alert:
             return self.alerts.pre_capture

frigate/record/cleanup.py

@@ -198,56 +198,52 @@ class RecordingCleanup(threading.Thread):
         deleted_recordings = set()
         kept_recordings: list[tuple[float, float]] = []
         for recording in recordings:
-            modes: set[RetainModeEnum] = set()
-            # Now look for every reason to keep this recording segment
+            keep = False
+            mode = None
+            # Now look for a reason to keep this recording segment
             for idx in range(review_start, len(reviews)):
                 review = reviews[idx]
                 severity = review.severity
                 pre_capture = config.record.get_review_pre_capture(severity)
                 post_capture = config.record.get_review_post_capture(severity)
 
-                # stop once no later review's pre capture can reach this segment
-                if (
-                    review.start_time - config.record.event_pre_capture
-                    > recording.end_time
-                ):
+                # if the review starts in the future, stop checking reviews
+                # and let this recording segment expire
+                if review.start_time - pre_capture > recording.end_time:
+                    keep = False
                     break
 
-                if review.start_time - pre_capture > recording.end_time:
-                    continue
-
-                # collect every review that is in progress or ends after the
-                # recording starts, since their windows can overlap
+                # if the review is in progress or ends after the recording starts, keep it
+                # and stop looking at reviews
                 if (
                     review.end_time is None
                     or review.end_time + post_capture >= recording.start_time
                 ):
-                    modes.add(
+                    keep = True
+                    mode = (
                         alerts_retain_mode
                         if review.severity == "alert"
                         else detections_retain_mode
                     )
+                    break
 
                 # if the review ends before this recording segment starts, skip
-                # it on future segments too. reviews and recordings are sorted,
-                # but an earlier review with a longer post capture may still
-                # overlap, so only skip while nothing has overlapped yet
-                elif not modes:
+                # this review and check the next review for an overlap.
+                # since the review and recordings are sorted, we can skip review
+                # that end before the previous recording segment started on future segments
+                if review.end_time + post_capture < recording.start_time:
                     review_start = idx
 
-            # Delete recordings unless an overlapping review's retention mode keeps them
-            if not any(
-                mode == RetainModeEnum.all
+            # Delete recordings outside of the retention window or based on the retention mode
+            if (
+                not keep
                 or (
                     mode == RetainModeEnum.motion
-                    and (
-                        recording.motion != 0
-                        or recording.objects != 0
-                        or recording.dBFS != 0
-                    )
+                    and recording.motion == 0
+                    and recording.objects == 0
+                    and recording.dBFS == 0
                 )
-                or (mode == RetainModeEnum.active_objects and recording.objects != 0)
-                for mode in modes
+                or (mode == RetainModeEnum.active_objects and recording.objects == 0)
             ):
                 recording_path = Path(recording.path)
                 recording_path.unlink(missing_ok=True)

frigate/record/maintainer.py

@@ -471,9 +471,14 @@ class RecordingMaintainer(threading.Thread):
         for key, recordings in grouped_recordings.items():
             camera, stream_type = key
 
-            # get all reviews with the end time after the start of the oldest
-            # cache file or with end_time None; shared across stream types
+            # get all reviews whose post capture reaches the oldest cache file
+            # or with end_time None; shared across stream types
             if camera not in reviews_by_camera:
+                post_capture = (
+                    self.config.cameras[camera].record.event_post_capture
+                    if camera in self.config.cameras
+                    else 0
+                )
                 reviews_by_camera[camera] = (
                     ReviewSegment.select(
                         ReviewSegment.start_time,
@@ -484,7 +489,10 @@ class RecordingMaintainer(threading.Thread):
                     .where(
                         ReviewSegment.camera == camera,
                         (ReviewSegment.end_time == None)
-                        | (ReviewSegment.end_time >= min_start_per_camera[camera]),
+                        | (
+                            ReviewSegment.end_time
+                            >= min_start_per_camera[camera] - post_capture
+                        ),
                     )
                     .order_by(ReviewSegment.start_time)
                 )
@@ -759,40 +767,39 @@ class RecordingMaintainer(threading.Thread):
 
         # we fell through the continuous / motion check, so we need to check the review items
         # if the cached segment overlaps with the review items:
-        modes: set[RetainModeEnum] = set()
+        overlaps = False
         for review in reviews:
             severity = SeverityEnum[review.severity]
 
-            # stop once no later review item's pre capture can reach this segment
-            if (
-                review.start_time - record_config.event_pre_capture
-            ) > end_time.timestamp():
-                break
-
+            # if the review item starts in the future, stop checking review items
+            # and remove this segment
             if (
                 review.start_time - record_config.get_review_pre_capture(severity)
             ) > end_time.timestamp():
-                continue
+                overlaps = False
+                break
 
-            # collect every review item that is in progress or ends after the
-            # recording starts, since their windows can overlap
+            # if the review item is in progress or ends after the recording starts, keep it
+            # and stop looking at review items
             if (
                 review.end_time is None
                 or (review.end_time + record_config.get_review_post_capture(severity))
                 >= start_time.timestamp()
             ):
-                modes.add(
-                    alerts_retain_mode
-                    if review.severity == "alert"
-                    else detections_retain_mode
-                )
+                overlaps = True
+                break
+
+        if overlaps:
+            record_mode = (
+                alerts_retain_mode
+                if review.severity == "alert"
+                else detections_retain_mode
+            )
 
-        if modes:
             if segment_stats is None:
                 segment_stats = self.segment_stats(camera, start_time, end_time)
 
-            # keep the segment if any overlapping review item's mode keeps it
-            if any(not segment_stats.should_discard_segment(m) for m in modes):
+            if not segment_stats.should_discard_segment(record_mode):
                 # move from cache to recordings immediately
                 return await self.move_segment(
                     camera,

frigate/test/test_maintainer.py

@@ -6,6 +6,8 @@ import unittest
 from pathlib import Path
 from unittest.mock import AsyncMock, MagicMock, patch
 
+from peewee import SqliteDatabase
+
 # Mock complex imports before importing maintainer, saving originals so we can
 # restore them after import and avoid polluting sys.modules for other tests.
 _MOCKED_MODULES = [
@@ -20,6 +22,7 @@ for name in _MOCKED_MODULES:
 
 # Now import the class under test
 from frigate.config import FrigateConfig, RetainModeEnum  # noqa: E402
+from frigate.models import ReviewSegment  # noqa: E402
 from frigate.record.maintainer import RecordingMaintainer, SegmentInfo  # noqa: E402
 
 # Restore original modules (or remove mock if there was no original)
@@ -81,6 +84,53 @@ class TestMaintainer(unittest.IsolatedAsyncioTestCase):
                             f"Expected a single warning for unexpected files, got {len(matching)}",
                         )
 
+    async def test_ended_review_stays_eligible_through_post_capture(self):
+        config = MagicMock(spec=FrigateConfig)
+        camera_config = MagicMock()
+        camera_config.record.sub.enabled = False
+        camera_config.record.event_post_capture = 60
+        config.cameras = {"test_cam": camera_config}
+
+        maintainer = RecordingMaintainer(config, MagicMock())
+        maintainer.requestor = MagicMock()
+        maintainer.recordings_publisher = MagicMock()
+        maintainer.validate_and_move_segment = AsyncMock(return_value=None)
+
+        db = SqliteDatabase(":memory:")
+        with db.bind_ctx([ReviewSegment]):
+            db.create_tables([ReviewSegment])
+
+            # oldest cached segment starts 40s after the review ended
+            segment_start = datetime.datetime(
+                2026, 10, 19, 13, 1, 40, tzinfo=datetime.UTC
+            )
+            ReviewSegment.create(
+                id="review",
+                camera="test_cam",
+                start_time=segment_start.timestamp() - 50,
+                end_time=segment_start.timestamp() - 40,
+                severity="alert",
+                thumb_path="",
+                data={},
+            )
+
+            with tempfile.TemporaryDirectory() as cache_dir:
+                name = segment_start.strftime("%Y%m%d%H%M%S%z")
+                Path(cache_dir, f"test_cam@{name}.mp4").touch()
+
+                with (
+                    patch("frigate.record.maintainer.CACHE_DIR", cache_dir),
+                    patch(
+                        "frigate.record.maintainer.psutil.process_iter", return_value=[]
+                    ),
+                ):
+                    await maintainer.move_files()
+
+            reviews = maintainer.validate_and_move_segment.call_args.args[1]
+            self.assertEqual(
+                [segment_start.timestamp() - 40], [r.end_time for r in reviews]
+            )
+
     async def test_drops_quiet_segment_when_only_motion_retention(self):
         # Regression: when motion retention is enabled but a segment has no
         # motion and no review overlaps it, the segment must still be dropped.
@@ -127,116 +177,6 @@ class TestMaintainer(unittest.IsolatedAsyncioTestCase):
         self.assertIsNone(result)
         maintainer.drop_segment.assert_called_once_with(cache_path)
 
-    async def test_quiet_segment_kept_by_later_overlapping_alert(self):
-        config = MagicMock(spec=FrigateConfig)
-
-        camera_config = MagicMock()
-        camera_config.record.enabled = True
-        camera_config.record.continuous.days = 0
-        camera_config.record.motion.days = 0
-        camera_config.record.alerts.retain.mode = RetainModeEnum.all
-        camera_config.record.detections.retain.mode = RetainModeEnum.motion
-        camera_config.record.event_pre_capture = 60
-        camera_config.record.get_review_pre_capture.return_value = 60
-        camera_config.record.get_review_post_capture.return_value = 60
-        config.cameras = {"test_cam": camera_config}
-
-        maintainer = RecordingMaintainer(config, MagicMock())
-
-        now = datetime.datetime.now(datetime.UTC)
-        start_time = now - datetime.timedelta(seconds=20)
-        end_time = now - datetime.timedelta(seconds=10)
-        cache_path = "/tmp/cache/test_cam@20260417150000+0000.mp4"
-
-        maintainer.end_time_cache = {
-            cache_path: (end_time, 10.0, False, None, None, None, [])
-        }
-        maintainer.object_recordings_info["test_cam"] = [(now.timestamp(), [], [], [])]
-        maintainer.audio_recordings_info["test_cam"] = []
-
-        maintainer.drop_segment = MagicMock()
-        maintainer.move_segment = AsyncMock(return_value=None)
-        maintainer.recordings_publisher = MagicMock()
-
-        # the detection's post capture and the alert's window both cover the segment
-        detection = MagicMock()
-        detection.severity = "detection"
-        detection.start_time = start_time.timestamp() - 50
-        detection.end_time = start_time.timestamp() - 40
-        alert = MagicMock()
-        alert.severity = "alert"
-        alert.start_time = start_time.timestamp() - 10
-        alert.end_time = start_time.timestamp()
-
-        await maintainer.validate_and_move_segment(
-            "test_cam",
-            reviews=[detection, alert],
-            recording={
-                "start_time": start_time,
-                "cache_path": cache_path,
-                "stream_type": "main",
-            },
-        )
-
-        maintainer.move_segment.assert_awaited_once()
-        maintainer.drop_segment.assert_not_called()
-
-    async def test_quiet_segment_kept_by_later_alert_with_longer_pre_capture(self):
-        config = MagicMock(spec=FrigateConfig)
-
-        camera_config = MagicMock()
-        camera_config.record.enabled = True
-        camera_config.record.continuous.days = 0
-        camera_config.record.motion.days = 0
-        camera_config.record.alerts.retain.mode = RetainModeEnum.all
-        camera_config.record.detections.retain.mode = RetainModeEnum.all
-        camera_config.record.event_pre_capture = 30
-        camera_config.record.get_review_pre_capture.side_effect = lambda s: (
-            30 if s == "alert" else 5
-        )
-        camera_config.record.get_review_post_capture.return_value = 5
-        config.cameras = {"test_cam": camera_config}
-
-        maintainer = RecordingMaintainer(config, MagicMock())
-
-        now = datetime.datetime.now(datetime.UTC)
-        start_time = now - datetime.timedelta(seconds=20)
-        end_time = now - datetime.timedelta(seconds=10)
-        cache_path = "/tmp/cache/test_cam@20260417150000+0000.mp4"
-
-        maintainer.end_time_cache = {
-            cache_path: (end_time, 10.0, False, None, None, None, [])
-        }
-        maintainer.object_recordings_info["test_cam"] = [(now.timestamp(), [], [], [])]
-        maintainer.audio_recordings_info["test_cam"] = []
-
-        maintainer.drop_segment = MagicMock()
-        maintainer.move_segment = AsyncMock(return_value=None)
-        maintainer.recordings_publisher = MagicMock()
-
-        # the detection's pre capture starts after the segment, the alert's reaches it
-        detection = MagicMock()
-        detection.severity = "detection"
-        detection.start_time = end_time.timestamp() + 10
-        detection.end_time = end_time.timestamp() + 12
-        alert = MagicMock()
-        alert.severity = "alert"
-        alert.start_time = end_time.timestamp() + 20
-        alert.end_time = end_time.timestamp() + 25
-
-        await maintainer.validate_and_move_segment(
-            "test_cam",
-            reviews=[detection, alert],
-            recording={
-                "start_time": start_time,
-                "cache_path": cache_path,
-                "stream_type": "main",
-            },
-        )
-
-        maintainer.move_segment.assert_awaited_once()
-        maintainer.drop_segment.assert_not_called()
-
     async def test_defers_review_overlap_segment_until_metadata_catches_up(self):
         # Regression: a segment overlapping an active_objects review must not
         # be dropped while detection metadata lags behind the segment end,
@@ -249,7 +189,6 @@ class TestMaintainer(unittest.IsolatedAsyncioTestCase):
         camera_config.record.continuous.days = 7
         camera_config.record.motion.days = 0
         camera_config.record.alerts.retain.mode = RetainModeEnum.active_objects
-        camera_config.record.event_pre_capture = 5
         camera_config.record.get_review_pre_capture.return_value = 5
         camera_config.record.get_review_post_capture.return_value = 5
         config.cameras = {"test_cam": camera_config}

frigate/test/test_record_cleanup_reviews.py

@@ -1,142 +0,0 @@
-"""Tests for review-based retention in recording cleanup."""
-
-import datetime
-import unittest
-from unittest.mock import MagicMock
-
-from peewee import SqliteDatabase
-
-from frigate.config import FrigateConfig
-from frigate.models import Previews, Recordings, ReviewSegment, UserReviewStatus
-from frigate.record.cleanup import RecordingCleanup
-
-
-class TestRecordingCleanupReviewOverlap(unittest.TestCase):
-    def setUp(self):
-        self.db = SqliteDatabase(":memory:")
-        models = [Previews, Recordings, ReviewSegment, UserReviewStatus]
-        self.db.bind(models)
-        self.db.create_tables(models)
-        self.base = datetime.datetime.now().timestamp() - 3600
-
-    def tearDown(self):
-        self.db.close()
-
-    def _build_cleanup(self, alerts: dict, detections: dict) -> RecordingCleanup:
-        config = FrigateConfig(
-            **{
-                "mqtt": {"host": "mqtt"},
-                "cameras": {
-                    "front_door": {
-                        "ffmpeg": {
-                            "inputs": [
-                                {
-                                    "path": "rtsp://10.0.0.1:554/video",
-                                    "roles": ["detect", "record"],
-                                }
-                            ]
-                        },
-                        "record": {
-                            "enabled": True,
-                            "continuous": {"days": 0},
-                            "motion": {"days": 0},
-                            "alerts": alerts,
-                            "detections": detections,
-                        },
-                    }
-                },
-            }
-        )
-        return RecordingCleanup(config, MagicMock())
-
-    def _insert_review(self, id: str, severity: str, start: float, end: float):
-        ReviewSegment.create(
-            id=id,
-            camera="front_door",
-            start_time=self.base + start,
-            end_time=self.base + end,
-            severity=severity,
-            thumb_path=f"/media/frigate/clips/review/{id}.webp",
-            data={},
-        )
-
-    def _insert_recording(self, id: str, start: float):
-        Recordings.create(
-            id=id,
-            camera="front_door",
-            path=f"/media/frigate/recordings/{id}.mp4",
-            start_time=self.base + start,
-            end_time=self.base + start + 10,
-            duration=10,
-            motion=0,
-            objects=0,
-            dBFS=0,
-            segment_size=0,
-            stream_type="main",
-        )
-
-    def test_quiet_segment_kept_by_later_overlapping_alert(self):
-        cleanup = self._build_cleanup(
-            alerts={"pre_capture": 60, "post_capture": 60, "retain": {"mode": "all"}},
-            detections={"post_capture": 60, "retain": {"mode": "motion"}},
-        )
-        self._insert_review("detection", "detection", 90, 100)
-        self._insert_review("alert", "alert", 130, 140)
-        self._insert_recording("quiet", 140)
-
-        cleanup.expire_recordings()
-
-        assert Recordings.get_or_none(Recordings.id == "quiet") is not None
-
-    def test_quiet_segment_dropped_when_no_overlapping_mode_keeps_it(self):
-        cleanup = self._build_cleanup(
-            alerts={
-                "pre_capture": 60,
-                "post_capture": 60,
-                "retain": {"mode": "motion"},
-            },
-            detections={"post_capture": 60, "retain": {"mode": "motion"}},
-        )
-        self._insert_review("detection", "detection", 90, 100)
-        self._insert_review("alert", "alert", 130, 140)
-        self._insert_recording("quiet", 140)
-
-        cleanup.expire_recordings()
-
-        assert Recordings.get_or_none(Recordings.id == "quiet") is None
-
-    def test_longer_pre_capture_survives_an_earlier_shorter_review(self):
-        # the detection starts after the segment even with its pre capture,
-        # but the later alert's longer pre capture still reaches it
-        cleanup = self._build_cleanup(
-            alerts={"pre_capture": 30, "retain": {"mode": "all"}},
-            detections={"pre_capture": 5, "retain": {"mode": "all"}},
-        )
-        self._insert_review("detection", "detection", 10, 12)
-        self._insert_review("alert", "alert", 20, 25)
-        self._insert_recording("early", -6)
-
-        cleanup.expire_recordings()
-
-        assert Recordings.get_or_none(Recordings.id == "early") is not None
-
-    def test_longer_post_capture_survives_a_later_shorter_review(self):
-        # the alert's window ends before the detection's, so skipping past
-        # it must not skip the detection for later segments
-        cleanup = self._build_cleanup(
-            alerts={"post_capture": 5, "retain": {"mode": "motion"}},
-            detections={"post_capture": 60, "retain": {"mode": "all"}},
-        )
-        self._insert_review("detection", "detection", 90, 100)
-        self._insert_review("alert", "alert", 110, 115)
-        self._insert_recording("first", 125)
-        self._insert_recording("second", 135)
-
-        cleanup.expire_recordings()
-
-        assert Recordings.get_or_none(Recordings.id == "first") is not None
-        assert Recordings.get_or_none(Recordings.id == "second") is not None
-
-
-if __name__ == "__main__":
-    unittest.main()