making it clean

semantica/vector_store/qdrant_store.py

@@ -606,17 +606,15 @@ class QdrantStore:
 
     def iter_all(self, batch_size: int = 500):
         """
-        Iterate over every stored point using Qdrant's native scroll cursor.
+        Iterate over every stored point using Qdrant's scroll cursor.
 
-        Qdrant paginates by point-ID cursor, not by row offset, so this is
-        exposed instead of scan_vectors(offset, limit). An integer passed to
-        scroll()'s offset is a point ID rather than a rank, so there is no way
-        to seek to "the Nth record" without walking from the start.
-        VectorStore.iter_vectors() prefers this method when it is present.
+        Paginates by point-ID cursor rather than row offset, which is why this
+        exists instead of scan_vectors(offset, limit). An integer offset is a
+        point ID, not a rank.
 
-        Assumes a single unnamed vector per point, matching how insert_vectors()
-        writes them and how get_vector() reads them back. Collections configured
-        with named or multi-vectors are not handled here.
+        Assumes a single unnamed vector per point, as insert_vectors() and
+        get_vector() already do. Named and multi-vector collections are not
+        handled.
 
         Args:
             batch_size: Points to request per scroll call
@@ -625,11 +623,8 @@ class QdrantStore:
             Result dicts with 'id', 'metadata', and 'vector', in scroll order
 
         Raises:
-            ProcessingError: If the collection or client is not initialized.
-                Errors are raised rather than swallowed because a scan that
-                silently yields nothing is indistinguishable from an empty
-                source, which would let a caller such as `store migrate`
-                report success having copied nothing (issue #1083).
+            ProcessingError: If the collection or client is not initialized, or
+                if the scan cannot advance past a full page.
         """
         if self.collection is None or self.client is None or not QDRANT_AVAILABLE:
             raise ProcessingError(
@@ -653,13 +648,21 @@ class QdrantStore:
                     "vector": np.array(rec.vector) if rec.vector is not None else None,
                 }
 
-            # The final page can carry records while already reporting no next
-            # cursor, so those records are yielded above before stopping here.
-            # Calling scroll() again with offset=None would restart from the
-            # beginning rather than continue past the end.
-            if next_offset is None or not records:
+            # A final page can carry records alongside a null cursor, so they
+            # are yielded above before stopping. Passing offset=None back to
+            # scroll() would restart from the beginning, not continue.
+            if next_offset is None:
                 return
 
+            # A cursor without records means the scan cannot advance, which is
+            # truncation rather than completion.
+            if not records:
+                raise ProcessingError(
+                    "Qdrant returned an empty page alongside a continuation "
+                    "cursor, so the scan cannot advance. Refusing to return a "
+                    "truncated scan."
+                )
+
     def delete_vectors(
         self, point_ids: List[Union[str, int]], **options
     ) -> Dict[str, Any]:

semantica/vector_store/vector_store.py

@@ -867,12 +867,9 @@ class VectorStore:
         """
         Iterate over every stored vector, one page at a time.
 
-        Backends whose native pagination is cursor based (Qdrant, Pinecone,
-        Milvus, Weaviate) cannot honestly implement the positional
-        scan_vectors(offset, limit) contract, so they expose iter_all()
-        instead and it is preferred here when present. Backends with real
-        positional access (inmemory, FAISS, SQLite-vec, PgVector) fall
-        through to the offset loop below.
+        Cursor-based backends expose iter_all() because they cannot support a
+        positional offset; it takes precedence when present. Everything else
+        falls through to the scan_vectors() offset loop.
 
         Args:
             batch_size: Number of vectors to fetch per underlying call

tests/vector_store/test_qdrant_store.py

@@ -35,7 +35,7 @@ def _store_with_scroll(*pages):
 
 @patch("semantica.vector_store.qdrant_store.QDRANT_AVAILABLE", True)
 def test_iter_all_threads_cursor_across_pages():
-    """The next call must continue from the previous page's next_page_offset."""
+    """The next call continues from the previous page's cursor."""
     store = _store_with_scroll(
         ([_record(1), _record(2)], "cursor-1"),
         ([_record(3)], None),
@@ -53,11 +53,8 @@ def test_iter_all_threads_cursor_across_pages():
 
 @patch("semantica.vector_store.qdrant_store.QDRANT_AVAILABLE", True)
 def test_iter_all_yields_final_page_that_reports_no_next_cursor():
-    """Qdrant can return records and a null cursor on the same page.
-
-    Those records must still be yielded. Treating a null cursor as "stop
-    before this page" would silently drop the tail of every scan.
-    """
+    """Records and a null cursor can arrive together; those records must still
+    be yielded or every scan loses its tail."""
     store = _store_with_scroll(([_record(1), _record(2)], None))
 
     result = list(store.iter_all(batch_size=10))
@@ -97,21 +94,18 @@ def test_iter_all_empty_collection_yields_nothing():
 
 
 @patch("semantica.vector_store.qdrant_store.QDRANT_AVAILABLE", True)
-def test_iter_all_stops_on_empty_page_even_with_a_cursor():
-    """Defensive: an empty page ends the scan rather than looping forever."""
+def test_iter_all_raises_on_empty_page_with_a_cursor():
+    """An empty page with a cursor left set cannot advance, so returning here
+    would hand back a partial scan that reads as a complete one."""
     store = _store_with_scroll(([], "cursor-that-never-clears"))
 
-    assert list(store.iter_all()) == []
-    assert store.client.scroll.call_count == 1
+    with pytest.raises(ProcessingError, match="cannot advance"):
+        list(store.iter_all())
 
 
 @patch("semantica.vector_store.qdrant_store.QDRANT_AVAILABLE", True)
 def test_iter_all_raises_when_collection_not_initialized():
-    """Must fail loudly, not yield nothing.
-
-    An empty scan is indistinguishable from an empty source, which would let
-    `store migrate` report success having copied nothing (issue #1083).
-    """
+    """Must fail loudly: an empty scan reads the same as an empty source."""
     store = QdrantStore()
 
     with pytest.raises(ProcessingError, match="Collection not initialized"):

tests/vector_store/test_vector_manager_persistent.py

@@ -140,11 +140,7 @@ class _NonScanningBackendStore:
 
 
 class _IterAllBackendStore:
-    """Fake cursor-based backend store exposing iter_all() but not scan_vectors().
-
-    Mirrors qdrant/pinecone/milvus/weaviate, which cannot honour a positional
-    offset and therefore expose native iteration instead.
-    """
+    """Fake cursor-based store: iter_all() only, no usable scan_vectors()."""
 
     def __init__(self, items):
         self._items = items
@@ -230,12 +226,7 @@ class VectorStoreScanVectorsTests(unittest.TestCase):
 # ---------------------------------------------------------------------------
 
 class VectorStoreIterAllDispatchTests(unittest.TestCase):
-    """iter_vectors() prefers a backend's native iter_all() when present.
-
-    Cursor-based backends cannot implement scan_vectors(offset, limit)
-    honestly, so they expose iter_all() instead and iter_vectors() routes to
-    it rather than walking offsets.
-    """
+    """iter_vectors() prefers a backend's native iter_all() when present."""
 
     def _persistent_store(self, backend_store, backend_name="qdrant"):
         store = VectorStore(backend="inmemory", dimension=2)
@@ -268,9 +259,7 @@ class VectorStoreIterAllDispatchTests(unittest.TestCase):
         self.assertEqual(list(store.iter_vectors(batch_size=2)), items)
 
     def test_iter_vectors_falls_back_when_iter_all_not_callable(self):
-        # A mis-shaped adapter exposing a non-callable ``iter_all`` must not be
-        # invoked; the offset path still has to work. Mirrors the count()
-        # precedent in _MisShapedBackendStore.
+        # Mirrors the count() precedent in _MisShapedBackendStore.
         items = [{"id": "a", "vector": None, "metadata": {}}]
         store = self._persistent_store(_MisShapedIterAllBackendStore(items))
 
@@ -286,9 +275,7 @@ class VectorStoreIterAllDispatchTests(unittest.TestCase):
         self.assertEqual([item["metadata"] for item in collected], [{"type": "a"}])
 
     def test_iter_vectors_propagates_iter_all_errors(self):
-        # A scan that silently yields nothing is indistinguishable from an
-        # empty source, which would let `store migrate` report success having
-        # copied nothing (issue #1083).
+        # Silently yielding nothing would read as an empty source (#1083).
         class _FailingIterAll:
             def iter_all(self, batch_size=500):
                 raise ProcessingError("backend unreachable")