fix: reject crop evidence after track drift

recognization/code/live.py

@@ -162,6 +162,19 @@ def _iou(a: Sequence[int], b: Sequence[int]) -> float:
     return inter / union if union > 0 else 0.0
 
 
+def _crop_off_product(requested: Sequence[int], current: Sequence[int]) -> bool:
+    """True when a track no longer covers the box a crop was taken from.
+
+    The measure a settled label already uses: below DRIFT_IOU the track has
+    slid onto a different product, and a crop of the old one says nothing about
+    what is there now. A box that was never measured cannot vouch either, so
+    it counts as off product.
+    """
+    if len(requested) != 4 or len(current) != 4:
+        return True
+    return _iou(current, requested) < DRIFT_IOU
+
+
 class _CropRejected(Exception):
     """A returned crop that may not be used, carrying the reply to send back."""
 
@@ -806,10 +819,14 @@ class LiveSession:
                 continue
             self._crop_counter += 1
             request_id = "c%d" % self._crop_counter
+            # The detection box this crop is a photograph of. The answer is only
+            # about that product, so the box travels with the request.
+            requested_box = list(track.raw_box)
             self._crop_requests[request_id] = {
                 "request_id": request_id,
                 "track_id": track.track_id,
                 "frame_id": int(frame_id),
+                "box": requested_box,
                 "object_generation": track.object_generation,
                 "generation": self._generation,
                 "seq": seq,
@@ -824,7 +841,7 @@ class LiveSession:
                 "request_id": request_id,
                 "track_id": track.track_id,
                 "frame_id": int(frame_id),
-                "box": list(track.raw_box),
+                "box": list(requested_box),
             }
         return planned
 
@@ -944,11 +961,19 @@ class LiveSession:
             if track.last_seq < record["seq"]:
                 self._release_crop_slot(record)
                 raise _CropRejected("CROP_STALE", "the track is no longer in the requested frame")
+            if _crop_off_product(record["box"], track.raw_box):
+                # The crop is of the product the phone was asked to photograph.
+                # If the track has since slid onto a neighbour the answer would
+                # be a label for a product nobody cropped.
+                self._release_crop_slot(record)
+                raise _CropRejected("CROP_STALE", "the track moved off the product that was cropped")
             limit = self.settings.max_crop_bytes
             if not payload or len(payload) > limit:
                 self._release_crop_slot(record)
                 raise _CropRejected("CROP_TOO_LARGE", "crop is empty or over %d bytes" % limit)
-            return record, list(track.box), payload
+            # The label anchors to the box the crop was taken from, not to where
+            # the track is by the time the answer lands.
+            return record, list(record["box"]), payload
 
     def _on_classified(
         self,
@@ -975,6 +1000,12 @@ class LiveSession:
                 track.in_flight = max(0, track.in_flight - 1)
                 return
             track.in_flight = max(0, track.in_flight - 1)
+            if source == SOURCE_CROP and _crop_off_product(submitted_box, track.raw_box):
+                # The answer was measured on the box the track has since left.
+                # Dropping it leaves the track to be decided from a later frame
+                # crop instead of putting the old product's label on whatever
+                # moved into its place.
+                return
             track.attempts += 1
             # A crop is an attempt against the same budget as a frame crop, so
             # the lane cannot be used to re-roll the dice on one product.

recognization/code/test_live.py

@@ -533,6 +533,29 @@ def requested_crop(result):
     return None
 
 
+def panned(detector, x):
+    """Put the fake facing at x, the way a camera panning a shelf does.
+
+    The facing stays too narrow for the frame path, so only the crop lane can
+    decide it, and it stays under crop_below_px so a crop is asked about.
+    """
+    detector.boxes[0].update({
+        "box": [x, 100, x + 36, 220], "width": 36, "height": 120, "center": [x + 18, 160],
+    })
+
+
+class GatedMatcher(FakeMatcher):
+    """Holds every classification open until the test lets it finish."""
+
+    def __init__(self):
+        super().__init__()
+        self.gate = threading.Event()
+
+    def score_crop(self, crop, method="hybrid"):
+        self.gate.wait(2.0)
+        return super().score_crop(crop, method)
+
+
 class CropLaneTests(unittest.TestCase):
     """A full-resolution crop is extra evidence for one track, nothing else."""
 
@@ -584,6 +607,72 @@ class CropLaneTests(unittest.TestCase):
         self.assertTrue(wait_for(lambda: session._crops_accepted == 1))
         engine.shutdown()
 
+    def test_a_delayed_crop_is_accepted_while_the_track_stays_on_its_product(self):
+        """Movement alone is not staleness: the crop still shows the same product."""
+        session, engine, detector = make_crop_session()
+        panned(detector, 100)
+        process(session, 1)
+        panned(detector, 106)
+        request = requested_crop(process(session, 2))
+        self.assertIsNotNone(request, "a facing the frame cannot use is asked about")
+        # The phone answers a frame late, on a track that has panned a little.
+        panned(detector, 112)
+        moved = process(session, 3)
+        self.assertEqual(moved["tracks"][0]["track_id"], request["track_id"])
+        reply = session.submit_crop(request["request_id"], request["track_id"],
+                                    request["frame_id"], frame_bytes())
+        self.assertEqual(reply["type"], "crop_ack")
+        self.assertTrue(wait_for(lambda: process(session, 4)["stats"]["crops"]["accepted"] == 1))
+        labelled = [t for t in process(session, 5)["tracks"] if t["identity"]["state"] == "recognized"]
+        self.assertEqual(labelled[0]["identity"]["sku_id"], "sku_a")
+        engine.shutdown()
+
+    def test_a_crop_for_a_track_that_slid_onto_its_neighbour_is_refused(self):
+        """The crop photographs one product; a track that walked off it answers nothing."""
+        session, engine, detector = make_crop_session()
+        panned(detector, 100)
+        process(session, 1)
+        panned(detector, 118)
+        request = requested_crop(process(session, 2))
+        self.assertIsNotNone(request)
+        # The pan carries the track onto the next product before the phone answers.
+        panned(detector, 136)
+        moved = process(session, 3)
+        self.assertEqual(moved["tracks"][0]["track_id"], request["track_id"],
+                         "the id must survive the pan, or this refusal proves nothing")
+        reply = session.submit_crop(request["request_id"], request["track_id"],
+                                    request["frame_id"], frame_bytes())
+        self.assertEqual(reply["code"], "CROP_STALE")
+        self.assertEqual(session._tracks[request["track_id"]].in_flight, 0,
+                         "a refused crop gives the track's busy slot back")
+        result = process(session, 4)
+        self.assertEqual(result["stats"]["crops"]["accepted"], 0)
+        self.assertEqual(result["stats"]["crops"]["rejected"], 1)
+        self.assertNotEqual(result["tracks"][0]["identity"]["state"], "recognized")
+        engine.shutdown()
+
+    def test_a_crop_answered_after_the_track_left_its_product_does_not_label_it(self):
+        """A classification in flight cannot settle a track that has since moved on."""
+        matcher = GatedMatcher()
+        session, engine, detector = make_crop_session(matcher=matcher)
+        panned(detector, 100)
+        process(session, 1)
+        panned(detector, 118)
+        request = requested_crop(process(session, 2))
+        self.assertIsNotNone(request)
+        reply = session.submit_crop(request["request_id"], request["track_id"],
+                                    request["frame_id"], frame_bytes())
+        self.assertEqual(reply["type"], "crop_ack", "the track is still on the product here")
+        # The answer is still in the classifier when the pan takes the track off it.
+        panned(detector, 136)
+        process(session, 3)
+        matcher.gate.set()
+        track = session._tracks[request["track_id"]]
+        self.assertTrue(wait_for(lambda: session._tracks[request["track_id"]].in_flight == 0))
+        self.assertEqual(track.attempts, 0, "a crop of a product left behind is not an attempt")
+        self.assertIsNone(track.identity, "the neighbour must not wear the old product's label")
+        engine.shutdown()
+
     def test_a_track_is_asked_once(self):
         session, engine, _ = make_crop_session()
         process(session, 1)