From 02e69acac77f1faca585792ca4d68bc8b116c2f5 Mon Sep 17 00:00:00 2001 From: kiyo9w Date: Mon, 28 Sep 2026 17:22:46 +0900 Subject: [PATCH] fix: require cached gallery rows to keep the enrolled order A cache key proves the store, not the rows inside the file. The old check asked only that every cached row appear somewhere in the enrolled list, so a row moved across SKUs still passed: the matrix that travelled with it would then be filed under the wrong product, and a valid key would rename a SKU. Rows now walk the enrolled list forward and never backwards, so a moved, repeated or foreign row has nowhere left to match. Views that could not be decoded stay absent without disturbing the order of the ones after them, which is what keeps a shorter cache a hit rather than a re-embed. --- recognization/code/gallery.py | 26 ++++--- recognization/code/test_gallery.py | 112 ++++++++++++++++++++++------- 2 files changed, 105 insertions(+), 33 deletions(-) diff --git a/recognization/code/gallery.py b/recognization/code/gallery.py index 5afcb33..f1c21bc 100644 --- a/recognization/code/gallery.py +++ b/recognization/code/gallery.py @@ -382,18 +382,26 @@ class ProductGallery: return None if not matrix.shape[0] or matrix.shape[0] != len(skus) or len(skus) != len(views): return None - if not np.issubdtype(matrix.dtype, np.number) or not np.isfinite(matrix).all(): - return None - # Undecodable views may be omitted; all cached rows must retain the - # exact enrolled SKU/view order or a valid key could name wrong SKUs. - rows = list(zip(skus, views)) - remaining = iter((sku_id, os.path.basename(view)) for sku_id, view in enrolled) - if not all(any(pair == cached for pair in remaining) for cached in rows): + if not np.issubdtype(matrix.dtype, np.floating) or not np.isfinite(matrix).all(): return None + # Rows walk the enrolled list forward and never backwards, so a + # moved, repeated or foreign row has nowhere left to match. Views + # that could not be decoded are simply absent: everyone after one + # keeps its order, which is what stops a valid key from renaming a + # SKU. + expected = [(sku_id, os.path.basename(view)) for sku_id, view in enrolled] + rows: List[Tuple[str, str]] = [] + position = 0 + for sku_id, view in zip(skus, views): + while position < len(expected) and expected[position] != (sku_id, view): + position += 1 + if position == len(expected): + return None + rows.append((sku_id, os.path.join(self.stock_dir, view))) + position += 1 if not self._weights_pinned(): return None - paths = [(sku, os.path.join(self.stock_dir, view)) for sku, view in rows] - return paths, matrix.astype(np.float32, copy=False) + return rows, matrix.astype(np.float32, copy=False) except (OSError, ValueError, TypeError, KeyError, IndexError): return None # absent, malformed, or raced with a checkpoint removal diff --git a/recognization/code/test_gallery.py b/recognization/code/test_gallery.py index eadd11a..57e0a2c 100644 --- a/recognization/code/test_gallery.py +++ b/recognization/code/test_gallery.py @@ -236,6 +236,31 @@ class FeatureCacheTests(unittest.TestCase): self.assertTrue(gallery.load()) return gallery, backbone + def cache_payload(self, gallery): + """The rows and matrix a loaded gallery left on disk.""" + with np.load(gallery._cache_path(), allow_pickle=False) as payload: + return ( + [str(value) for value in payload["skus"].tolist()], + [str(value) for value in payload["views"].tolist()], + payload["matrix"], + ) + + def rewrite_cache(self, gallery, skus, views, matrix): + """The same valid key with different contents: only the rows can catch it.""" + path = gallery._cache_path() + with np.load(path, allow_pickle=False) as payload: + key = str(payload["key"].item()) + np.savez(path, key=key, skus=np.array(skus), views=np.array(views), matrix=matrix) + + def re_embeds(self, first, why): + """The one right answer to a rewritten cache: build it from the pixels.""" + backbone = Backbone() + gallery = a_stored_gallery(self.directory.name, ("sku_a", "sku_b"), backbone) + self.assertTrue(gallery.load(), why) + self.assertEqual(backbone.calls, 1, f"{why} must not be served from cache") + np.testing.assert_array_equal(gallery._matrix, first._matrix) + self.assertEqual([entry.sku_id for entry in gallery._entries], ["sku_a", "sku_a", "sku_b"]) + def test_a_second_start_answers_the_same_from_the_cache(self): first, backbone = self.loaded() self.assertEqual(backbone.calls, 1) @@ -282,34 +307,73 @@ class FeatureCacheTests(unittest.TestCase): self.assertEqual(backbone.calls, 1) np.testing.assert_array_equal(second._matrix, first._matrix) - def test_a_tampered_cache_row_is_not_served(self): - # A current key says nothing about the rows inside the file. Swap them - # and the gallery must re-embed, never name a SKU by a moved row. - first, _ = self.loaded() - path = first._cache_path() - with np.load(path, allow_pickle=False) as payload: - key = str(payload["key"].item()) - skus = [str(value) for value in payload["skus"].tolist()] - views = [str(value) for value in payload["views"].tolist()] - matrix = payload["matrix"] - # The two sku_a views are adjacent, so a label swap has to cross SKUs to - # mean anything - and the matrix that stays put is the dangerous one: + def test_swapped_cache_rows_are_not_served(self): + # A current key says nothing about the rows inside the file. The two + # sku_a views are adjacent, so a swap has to cross SKUs to mean + # anything - and the matrix that moves with them is the dangerous part: # a vector that would then be filed under the wrong product. + first, _ = self.loaded() + skus, views, matrix = self.cache_payload(first) self.assertEqual(list(zip(skus, views)), [("sku_a", "sku_a.jpeg"), ("sku_a", "sku_a_1.jpeg"), ("sku_b", "sku_b.jpeg")]) - tampered = { - "reordered rows": (skus[::-1], views[::-1], matrix[::-1]), - "swapped labels": ([skus[2], skus[1], skus[0]], views, matrix), - "another store's row": (skus[1:] + ["other_store"], views + ["other_store.jpeg"], matrix), + self.rewrite_cache(first, skus[::-1], views[::-1], matrix[::-1]) + self.re_embeds(first, "rows swapped with their vectors") + + def test_mislabelled_cache_rows_are_not_served(self): + # The views and the matrix stay where they are, so every row names a + # product it was not embedded for. + first, _ = self.loaded() + skus, views, matrix = self.cache_payload(first) + self.rewrite_cache(first, [skus[2], skus[1], skus[0]], views, matrix) + self.re_embeds(first, "labels swapped around the views") + + def test_a_duplicated_cache_row_is_not_served(self): + # The first view written twice: one image would answer for two products. + first, _ = self.loaded() + skus, views, matrix = self.cache_payload(first) + self.rewrite_cache(first, skus[:1] * 2 + skus[2:], views[:1] * 2 + views[2:], matrix) + self.re_embeds(first, "a row repeated") + + def test_a_foreign_sku_row_is_not_served(self): + # A row from another store's catalog: a SKU this gallery never enrolled. + first, _ = self.loaded() + skus, views, matrix = self.cache_payload(first) + self.rewrite_cache(first, skus[1:] + ["other_store"], views[1:] + ["other_store.jpeg"], matrix) + self.re_embeds(first, "another store's row") + + def test_a_cache_may_omit_a_view_it_could_not_decode(self): + # The writer drops a view it could not read and keeps everyone else's + # order, so such a cache is still this store's - and still worth serving + # rather than re-embedding the whole catalog on every start. + first, _ = self.loaded() + skus, views, matrix = self.cache_payload(first) + self.rewrite_cache(first, skus[:1] + skus[2:], views[:1] + views[2:], matrix[[0, 2]]) + + backbone = Backbone() + gallery = a_stored_gallery(self.directory.name, ("sku_a", "sku_b"), backbone) + self.assertTrue(gallery.load()) + self.assertEqual(backbone.calls, 0, "a readable shorter cache is still a hit") + self.assertEqual([entry.sku_id for entry in gallery._entries], ["sku_a", "sku_b"]) + np.testing.assert_array_equal(gallery._matrix, first._matrix[[0, 2]]) + + def test_a_nonnumeric_cache_matrix_is_not_served(self): + # Only the float matrix _cache_write produces is a cache. A complex + # matrix is finite and casts without complaint, so it answers with a + # different matrix than the one on disk; integer, boolean and text + # matrices are not embeddings at all. + first, _ = self.loaded() + skus, views, matrix = self.cache_payload(first) + others = { + "complex": matrix.astype(np.complex128), + "timedelta": np.zeros_like(matrix, dtype="timedelta64[s]"), + "integer": (matrix * 8).astype(np.int32), + "boolean": np.ones_like(matrix, dtype=bool), + "text": np.zeros(matrix.shape, dtype="