From c6d8f067be73af1c0ec4b6c5ec7ac0d81bb5dfbf Mon Sep 17 00:00:00 2001 From: Nicolas Mowen Date: Tue, 29 Sep 2026 10:18:47 -0600 Subject: [PATCH] Improve report context management --- .../post/review_descriptions.py | 22 ++++----- .../test/test_review_classification_state.py | 45 +++++++++---------- 2 files changed, 31 insertions(+), 36 deletions(-) diff --git a/frigate/data_processing/post/review_descriptions.py b/frigate/data_processing/post/review_descriptions.py index adf322f56c..a4e80cb6b3 100644 --- a/frigate/data_processing/post/review_descriptions.py +++ b/frigate/data_processing/post/review_descriptions.py @@ -310,7 +310,7 @@ class ReviewDescriptionProcessor(PostProcessorApi): primary_end = primary_seg["end_time"] primary_camera = primary_seg["camera"] contextual_items = [] - contextual_by_camera: dict[str, dict[str, Any]] = {} + seen_contextual_cameras = set() for seg in segments: seg_camera = seg["camera"] @@ -326,14 +326,12 @@ class ReviewDescriptionProcessor(PostProcessorApi): if seg_start < primary_end and primary_start < seg_end: # Avoid duplicates if same camera has multiple overlapping - # segments, but keep every segment's state changes - existing_item = contextual_by_camera.get(seg_camera) - - if existing_item is not None: - if seg["state_changes"]: - existing_item.setdefault("state_changes", []).extend( - seg["state_changes"] - ) + # segments. One with state changes is kept as its own item + # so each change stays within its item's time range. + if ( + seg_camera in seen_contextual_cameras + and not seg["state_changes"] + ): continue contextual_item = copy.deepcopy(seg["metadata"]) @@ -342,12 +340,10 @@ class ReviewDescriptionProcessor(PostProcessorApi): contextual_item["end_time"] = seg_end if seg["state_changes"]: - contextual_item["state_changes"] = list( - seg["state_changes"] - ) + contextual_item["state_changes"] = seg["state_changes"] contextual_items.append(contextual_item) - contextual_by_camera[seg_camera] = contextual_item + seen_contextual_cameras.add(seg_camera) # Add context array to primary item primary_item["context"] = contextual_items diff --git a/frigate/test/test_review_classification_state.py b/frigate/test/test_review_classification_state.py index 97429cd90f..49813b9f80 100644 --- a/frigate/test/test_review_classification_state.py +++ b/frigate/test/test_review_classification_state.py @@ -221,7 +221,7 @@ class TestSummaryContext(unittest.TestCase): return client.generate_review_summary.call_args.args[2] - def test_overlapping_context_reviews_keep_all_state_changes(self): + def test_context_state_changes_stay_with_their_review(self): events = self.summarize( [ self.row("front_door", 10, 60, 1), @@ -230,29 +230,28 @@ class TestSummaryContext(unittest.TestCase): ] ) + self.assertEqual( + [ + (item["start_time"], item["end_time"], item["state_changes"]) + for item in events[0]["context"] + ], + [ + (15, 25, ["front gate changed from closed to open"]), + (30, 40, ["front gate changed from open to closed"]), + ], + ) + + def test_later_context_review_without_changes_is_deduplicated(self): + events = self.summarize( + [ + self.row("front_door", 10, 60, 1), + self.row("driveway", 15, 25, 0, [gate_change(20.0)]), + self.row("driveway", 30, 40, 0), + ] + ) + self.assertEqual(len(events[0]["context"]), 1) - self.assertEqual( - events[0]["context"][0]["state_changes"], - [ - "front gate changed from closed to open", - "front gate changed from open to closed", - ], - ) - - def test_merging_context_does_not_leak_between_primary_events(self): - events = self.summarize( - [ - self.row("front_door", 10, 60, 1), - self.row("back_door", 12, 22, 1), - self.row("driveway", 15, 25, 0, [gate_change(20.0)]), - self.row("driveway", 30, 40, 0, [gate_change(35.0, "open", "closed")]), - ] - ) - - self.assertEqual( - events[1]["context"][0]["state_changes"], - ["front gate changed from closed to open"], - ) + self.assertEqual(events[0]["context"][0]["start_time"], 15) if __name__ == "__main__":