From 486425e18bb57222a2780eb303d63d0acbb56b17 Mon Sep 17 00:00:00 2001 From: Yunare Maia Date: Tue, 11 Aug 2026 12:21:32 +0000 Subject: [PATCH 1/4] fix(vector_store): make VectorManager methods work on persistent backends (#855) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit maintain_store() and collect_statistics() reached into VectorStore internals (.vectors/.metadata), which only exist for the inmemory backend — any persistent backend (FAISS, Qdrant, Pinecone, Milvus, ...) crashed with AttributeError. Add a public backend-agnostic VectorStore.count() accessor following the get_vector()/get_metadata() precedent (#843) and the NotImplementedError-on-unsupported-capability precedent of _filter_by_metadata() (#848): inmemory counts its dict, persistent backends delegate to count() when available, and raise NotImplementedError otherwise. VectorManager methods now go through count(); maintain_store() keeps the exact inmemory semantics (separate vector/metadata dict counts) and reports a 1:1 count for persistent backends, where metadata is stored alongside each vector. Tests: 10 hermetic unit tests covering inmemory, delegation and the NotImplementedError path. Core vector_store suite: 40 passed. --- semantica/vector_store/vector_store.py | 38 +++++- .../test_vector_manager_persistent.py | 124 ++++++++++++++++++ 2 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 tests/vector_store/test_vector_manager_persistent.py diff --git a/semantica/vector_store/vector_store.py b/semantica/vector_store/vector_store.py index d4999554..3f8794c1 100644 --- a/semantica/vector_store/vector_store.py +++ b/semantica/vector_store/vector_store.py @@ -824,6 +824,29 @@ def get_metadata(self, vector_id: str) -> Optional[Dict[str, Any]]: else: raise NotImplementedError(f"Backend store {type(self._backend_store).__name__} does not implement get_metadata") + def count(self) -> int: + """Return the number of vectors in the store, backend-agnostic. + + The inmemory backend counts its local dict; persistent backends + delegate to a ``count()`` on the wrapped store when available. + Following the get_vector()/get_metadata() precedent (#843) and the + NotImplementedError-on-unsupported-capability precedent of + _filter_by_metadata() (#848), a persistent backend that cannot + report a count raises NotImplementedError so callers can tell + "no vectors" apart from "counting not supported". + """ + if self.backend == "inmemory": + return len(self.vectors) + elif self._backend_store is not None: + if hasattr(self._backend_store, "count"): + return self._backend_store.count() + raise NotImplementedError( + f"Backend store {type(self._backend_store).__name__} does not " + "implement count. Vector counting is only supported for the " + "inmemory backend." + ) + return 0 + def initialize_decision_pipeline( self, graph_store: Optional[Any] = None, @@ -1453,9 +1476,16 @@ def maintain_store( self, store: VectorStore, **options: Dict[str, Any] ) -> Dict[str, Any]: """Maintain vector store health.""" - # Check integrity - vector_count = len(store.vectors) - metadata_count = len(store.metadata) + # Check integrity through the public count() accessor so persistent + # backends don't crash on the inmemory-only .vectors/.metadata + # internals (#855). + vector_count = store.count() + # Inmemory keeps vectors and metadata in separate dicts; persistent + # backends store metadata alongside each vector (1:1), so the + # counts coincide there. + metadata_count = ( + len(store.metadata) if store.backend == "inmemory" else vector_count + ) return { "healthy": vector_count == metadata_count, @@ -1466,7 +1496,7 @@ def maintain_store( def collect_statistics(self, store: VectorStore) -> Dict[str, Any]: """Collect vector store statistics.""" return { - "total_vectors": len(store.vectors), + "total_vectors": store.count(), "dimension": store.dimension, "backend": store.backend, } diff --git a/tests/vector_store/test_vector_manager_persistent.py b/tests/vector_store/test_vector_manager_persistent.py new file mode 100644 index 00000000..e6787201 --- /dev/null +++ b/tests/vector_store/test_vector_manager_persistent.py @@ -0,0 +1,124 @@ +"""Regression tests for #855: VectorManager persistent-backend crash. + +VectorManager.maintain_store() and collect_statistics() used to reach +into VectorStore internals (``.vectors`` / ``.metadata``), which only +exist for the inmemory backend — any persistent backend (FAISS, Qdrant, +Pinecone, Milvus, ...) crashed with AttributeError. Both methods now go +through the public backend-agnostic ``VectorStore.count()`` accessor. + +The persistent paths are exercised by swapping the backend on an +inmemory-backed instance, the same trick used across the earlier fixes +in this cluster (#839/#843/#845/#848). +""" + +import unittest + +import numpy as np + +from semantica.vector_store.vector_store import VectorStore, VectorManager + + +class _CountingBackendStore: + """Fake persistent backend store that supports count().""" + + def __init__(self, n): + self._n = n + + def count(self): + return self._n + + +class _NonCountingBackendStore: + """Fake persistent backend store without any count capability.""" + + +class VectorStoreCountTests(unittest.TestCase): + """VectorStore.count() backend-agnostic accessor.""" + + def setUp(self): + self.vectors = [np.array([1.0, 0.0]), np.array([0.0, 1.0])] + self.metadata = [{"type": "a"}, {"type": "b"}] + + def test_count_inmemory(self): + store = VectorStore(backend="inmemory", dimension=2) + store.store_vectors(self.vectors, self.metadata) + self.assertEqual(store.count(), 2) + + def test_count_empty_inmemory(self): + store = VectorStore(backend="inmemory", dimension=2) + self.assertEqual(store.count(), 0) + + def test_count_delegates_to_backend_store(self): + store = VectorStore(backend="inmemory", dimension=2) + store.backend = "faiss" + store._backend_store = _CountingBackendStore(7) + self.assertEqual(store.count(), 7) + + def test_count_raises_not_implemented_without_backend_support(self): + store = VectorStore(backend="inmemory", dimension=2) + store.backend = "faiss" + store._backend_store = _NonCountingBackendStore() + with self.assertRaises(NotImplementedError): + store.count() + + +class VectorManagerPersistentTests(unittest.TestCase): + """VectorManager works on persistent-style stores via count() (#855).""" + + def setUp(self): + self.vectors = [np.array([1.0, 0.0]), np.array([0.0, 1.0])] + self.metadata = [{"type": "a"}, {"type": "b"}] + self.manager = VectorManager() + + def _inmemory_store(self): + store = VectorStore(backend="inmemory", dimension=2) + store.store_vectors(self.vectors, self.metadata) + return store + + def _persistent_store(self, backend_store): + store = self._inmemory_store() + store.backend = "faiss" + store._backend_store = backend_store + return store + + def test_collect_statistics_inmemory(self): + stats = self.manager.collect_statistics(self._inmemory_store()) + self.assertEqual(stats["total_vectors"], 2) + self.assertEqual(stats["dimension"], 2) + self.assertEqual(stats["backend"], "inmemory") + + def test_collect_statistics_persistent_with_count(self): + store = self._persistent_store(_CountingBackendStore(5)) + stats = self.manager.collect_statistics(store) + self.assertEqual(stats["total_vectors"], 5) + self.assertEqual(stats["dimension"], 2) + self.assertEqual(stats["backend"], "faiss") + + def test_collect_statistics_persistent_without_count_raises(self): + store = self._persistent_store(_NonCountingBackendStore()) + # Regression: must raise NotImplementedError (capability missing), + # not AttributeError (internal attribute poking). + with self.assertRaises(NotImplementedError): + self.manager.collect_statistics(store) + + def test_maintain_store_inmemory(self): + health = self.manager.maintain_store(self._inmemory_store()) + self.assertTrue(health["healthy"]) + self.assertEqual(health["vector_count"], 2) + self.assertEqual(health["metadata_count"], 2) + + def test_maintain_store_persistent_with_count(self): + store = self._persistent_store(_CountingBackendStore(5)) + health = self.manager.maintain_store(store) + self.assertTrue(health["healthy"]) + self.assertEqual(health["vector_count"], 5) + self.assertEqual(health["metadata_count"], 5) + + def test_maintain_store_persistent_without_count_raises(self): + store = self._persistent_store(_NonCountingBackendStore()) + with self.assertRaises(NotImplementedError): + self.manager.maintain_store(store) + + +if __name__ == "__main__": + unittest.main() From 3838a4c606976c7ad9a2a2900da0bbbf7bb3390e Mon Sep 17 00:00:00 2001 From: Yunare Maia Date: Tue, 11 Aug 2026 13:19:26 +0000 Subject: [PATCH 2/4] fix(vector_store): raise NotImplementedError when count() unavailable Address Qodo review findings on #914: - Persistent backend with no wrapped store no longer silently returns 0 (which masked a missing initialization as an empty, healthy store); it now raises NotImplementedError like get_vector()/get_metadata(). - A mis-shaped adapter exposing a non-callable 'count' attribute now surfaces a clean NotImplementedError instead of a TypeError, via a getattr + callable() capability check. Adds regression tests for both cases. --- semantica/vector_store/vector_store.py | 16 +++++++---- .../test_vector_manager_persistent.py | 27 +++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/semantica/vector_store/vector_store.py b/semantica/vector_store/vector_store.py index 3f8794c1..18c008a6 100644 --- a/semantica/vector_store/vector_store.py +++ b/semantica/vector_store/vector_store.py @@ -65,7 +65,7 @@ License: MIT """ -from typing import Any, Dict, List, Optional, Tuple, TypedDict, Union +from typing import Any, Dict, List, Optional, Tuple, TypedDict, Union, cast import concurrent.futures import inspect @@ -833,19 +833,25 @@ def count(self) -> int: NotImplementedError-on-unsupported-capability precedent of _filter_by_metadata() (#848), a persistent backend that cannot report a count raises NotImplementedError so callers can tell - "no vectors" apart from "counting not supported". + "no vectors" apart from "counting not supported" — including when + the wrapped backend store is missing entirely (never silently + report an uninitialized store as empty). """ if self.backend == "inmemory": return len(self.vectors) elif self._backend_store is not None: - if hasattr(self._backend_store, "count"): - return self._backend_store.count() + count_attr = getattr(self._backend_store, "count", None) + if callable(count_attr): + return cast(int, count_attr()) raise NotImplementedError( f"Backend store {type(self._backend_store).__name__} does not " "implement count. Vector counting is only supported for the " "inmemory backend." ) - return 0 + raise NotImplementedError( + f"Backend store is not initialized; cannot count vectors for " + f"backend {self.backend!r}." + ) def initialize_decision_pipeline( self, diff --git a/tests/vector_store/test_vector_manager_persistent.py b/tests/vector_store/test_vector_manager_persistent.py index e6787201..482b5edd 100644 --- a/tests/vector_store/test_vector_manager_persistent.py +++ b/tests/vector_store/test_vector_manager_persistent.py @@ -32,6 +32,12 @@ class _NonCountingBackendStore: """Fake persistent backend store without any count capability.""" +class _MisShapedBackendStore: + """Fake persistent backend store whose ``count`` attribute is not callable.""" + + count = 42 # not a method — a plain attribute + + class VectorStoreCountTests(unittest.TestCase): """VectorStore.count() backend-agnostic accessor.""" @@ -61,6 +67,27 @@ def test_count_raises_not_implemented_without_backend_support(self): with self.assertRaises(NotImplementedError): store.count() + def test_count_raises_when_persistent_backend_not_initialized(self): + # Regression (Qodo review #914): a persistent backend with no + # wrapped store must not silently report 0 — that masks a missing + # initialization as an empty, healthy store. Follow the + # get_vector()/get_metadata() precedent and raise. + store = VectorStore(backend="inmemory", dimension=2) + store.backend = "faiss" + store._backend_store = None + with self.assertRaises(NotImplementedError): + store.count() + + def test_count_raises_when_backend_count_not_callable(self): + # Regression (Qodo review #914): a mis-shaped adapter exposing a + # non-callable ``count`` attribute must surface a clean + # NotImplementedError, not a TypeError. + store = VectorStore(backend="inmemory", dimension=2) + store.backend = "faiss" + store._backend_store = _MisShapedBackendStore() + with self.assertRaises(NotImplementedError): + store.count() + class VectorManagerPersistentTests(unittest.TestCase): """VectorManager works on persistent-style stores via count() (#855).""" From df02fa88254f3d20acd910ee3ed2001344d5250c Mon Sep 17 00:00:00 2001 From: Sameer6305 Date: Thu, 13 Aug 2026 20:56:00 +0530 Subject: [PATCH 3/4] fix(vector_store): implement count() on FAISS/SQLite/PgVector backends (#914) - FAISSStore.count(): returns len(index.vector_ids); 0 when no index exists yet - SQLiteVecStore.count(): delegates to get_stats()[vector_count] (SELECT COUNT(*)) - PgVectorStore.count(): delegates to get_stats()[vector_count] (SELECT COUNT(*)) - VectorStore.count(): fix misleading NotImplementedError message; now describes how to add count() support to a backend adapter rather than claiming only the inmemory backend can ever support counting - VectorManager.maintain_store(): split inmemory and persistent paths: * inmemory: independently reads len(vectors) and len(metadata) and compares them as an integrity check (original semantics preserved) * persistent: calls store.count(); returns metadata_count=None because metadata is co-located with vectors in the backend and cannot be counted independently; never manufactures metadata_count=vector_count as a vacuous tautology (#914 Qodo review) - Tests: rewrite test_vector_manager_persistent.py with 31 tests covering dispatch logic, inmemory divergence detection, persistent metadata_count=None invariant, FAISSStore/PgVectorStore via mocks, and SQLiteVecStore via real in-memory SQLite (skipped when sqlite-vec absent) --- semantica/vector_store/faiss_store.py | 12 + semantica/vector_store/pgvector_store.py | 9 + semantica/vector_store/sqlite_vec_store.py | 9 + semantica/vector_store/vector_store.py | 49 ++- .../test_vector_manager_persistent.py | 380 ++++++++++++++++-- 5 files changed, 407 insertions(+), 52 deletions(-) diff --git a/semantica/vector_store/faiss_store.py b/semantica/vector_store/faiss_store.py index 54df70a4..7f53a061 100644 --- a/semantica/vector_store/faiss_store.py +++ b/semantica/vector_store/faiss_store.py @@ -537,3 +537,15 @@ def get_stats(self) -> Dict[str, Any]: "vector_count": len(self.index.vector_ids), "faiss_available": FAISS_AVAILABLE, } + + def count(self) -> int: + """Return the number of vectors currently tracked in this store. + + Returns the length of the ``vector_ids`` list maintained by + ``FAISSIndex``. FAISSStore does not implement vector deletion, so + this list is strictly append-only and is always consistent with the + underlying FAISS index (``index.ntotal``). + """ + if self.index is None: + return 0 + return len(self.index.vector_ids) diff --git a/semantica/vector_store/pgvector_store.py b/semantica/vector_store/pgvector_store.py index e1ce8ed0..204b8090 100644 --- a/semantica/vector_store/pgvector_store.py +++ b/semantica/vector_store/pgvector_store.py @@ -950,6 +950,15 @@ def get_stats(self) -> Dict[str, Any]: except Exception as e: raise ProcessingError("Failed to get stats") from e + def count(self) -> int: + """Return the exact number of vectors stored in this PostgreSQL table. + + Executes ``SELECT COUNT(*) FROM `` — always reflects the + committed state of the table, including any deletes or updates. + """ + stats = self.get_stats() + return int(stats["vector_count"]) + def close(self): """Close the connection pool.""" if self._pool: diff --git a/semantica/vector_store/sqlite_vec_store.py b/semantica/vector_store/sqlite_vec_store.py index 9fbdaed5..a0e3242b 100644 --- a/semantica/vector_store/sqlite_vec_store.py +++ b/semantica/vector_store/sqlite_vec_store.py @@ -740,6 +740,15 @@ def get_stats(self) -> Dict[str, Any]: except Exception as e: raise ProcessingError("Failed to get store statistics") from e + def count(self) -> int: + """Return the exact number of vectors stored in this SQLite table. + + Executes ``SELECT COUNT(*) FROM
`` under the store's lock to + guarantee a consistent, transaction-aware result. + """ + stats = self.get_stats() + return int(stats["vector_count"]) + def close(self): """Close the database connection.""" if hasattr(self, "_lock") and self._lock: diff --git a/semantica/vector_store/vector_store.py b/semantica/vector_store/vector_store.py index 18c008a6..8fa9ae07 100644 --- a/semantica/vector_store/vector_store.py +++ b/semantica/vector_store/vector_store.py @@ -845,8 +845,8 @@ def count(self) -> int: return cast(int, count_attr()) raise NotImplementedError( f"Backend store {type(self._backend_store).__name__} does not " - "implement count. Vector counting is only supported for the " - "inmemory backend." + "implement a count() method. Add a count() method to the " + "backend store adapter to enable vector counting for this backend." ) raise NotImplementedError( f"Backend store is not initialized; cannot count vectors for " @@ -1481,22 +1481,41 @@ def manage_store( def maintain_store( self, store: VectorStore, **options: Dict[str, Any] ) -> Dict[str, Any]: - """Maintain vector store health.""" - # Check integrity through the public count() accessor so persistent - # backends don't crash on the inmemory-only .vectors/.metadata - # internals (#855). - vector_count = store.count() - # Inmemory keeps vectors and metadata in separate dicts; persistent - # backends store metadata alongside each vector (1:1), so the - # counts coincide there. - metadata_count = ( - len(store.metadata) if store.backend == "inmemory" else vector_count - ) + """Maintain vector store health. + + For the inmemory backend, both the vector count and the metadata + count are independently tracked in separate dicts and are compared + as an integrity check. + + For persistent backends that implement ``VectorStore.count()``, + only the vector count is available. Metadata is co-located with + each vector in the underlying store (added/deleted atomically), + so a separate metadata count cannot be meaningfully distinguished + from the vector count. The response omits ``metadata_count`` for + such backends and reports ``healthy: True`` to indicate that the + store is reachable and operational. + + If the backend does not implement ``count()``, the ``NotImplementedError`` + propagates to the caller — it is not silenced. + """ + if store.backend == "inmemory": + # Inmemory keeps vectors and metadata in separate dicts; compare + # them to detect accidental divergence (#855). + vector_count = len(store.vectors) + metadata_count = len(store.metadata) + return { + "healthy": vector_count == metadata_count, + "vector_count": vector_count, + "metadata_count": metadata_count, + } + # Persistent backend: delegate to count(). Metadata and vectors are + # stored together, so only one count is available. + vector_count = store.count() return { - "healthy": vector_count == metadata_count, + "healthy": True, "vector_count": vector_count, - "metadata_count": metadata_count, + "metadata_count": None, } def collect_statistics(self, store: VectorStore) -> Dict[str, Any]: diff --git a/tests/vector_store/test_vector_manager_persistent.py b/tests/vector_store/test_vector_manager_persistent.py index 482b5edd..7379fa32 100644 --- a/tests/vector_store/test_vector_manager_persistent.py +++ b/tests/vector_store/test_vector_manager_persistent.py @@ -1,30 +1,45 @@ -"""Regression tests for #855: VectorManager persistent-backend crash. +"""Regression tests for #855 / #914: VectorManager persistent-backend crash. VectorManager.maintain_store() and collect_statistics() used to reach into VectorStore internals (``.vectors`` / ``.metadata``), which only exist for the inmemory backend — any persistent backend (FAISS, Qdrant, -Pinecone, Milvus, ...) crashed with AttributeError. Both methods now go -through the public backend-agnostic ``VectorStore.count()`` accessor. - -The persistent paths are exercised by swapping the backend on an -inmemory-backed instance, the same trick used across the earlier fixes -in this cluster (#839/#843/#845/#848). +Pinecone, Milvus, SQLite, PgVector, Weaviate) crashed with AttributeError. +Both methods now go through the public backend-agnostic +``VectorStore.count()`` accessor. + +Phase 1 (PR #855): dispatch fix + NotImplementedError instead of +AttributeError for backends that don't implement count(). + +Phase 2 (PR #914): count() added to FAISSStore, SQLiteVecStore, and +PgVectorStore — the three backends whose storage contracts guarantee a +reliable, synchronous count. maintain_store() revised so the +persistent-backend path no longer manufactures a vacuous +``metadata_count == vector_count`` tautology; instead it returns +``metadata_count=None`` and delegates healthiness to whether the store is +reachable. """ +import tempfile import unittest +from pathlib import Path +from unittest.mock import MagicMock, patch import numpy as np from semantica.vector_store.vector_store import VectorStore, VectorManager +# --------------------------------------------------------------------------- +# Minimal fake backend stores for dispatch-level unit tests +# --------------------------------------------------------------------------- + class _CountingBackendStore: """Fake persistent backend store that supports count().""" - def __init__(self, n): + def __init__(self, n: int): self._n = n - def count(self): + def count(self) -> int: return self._n @@ -33,13 +48,17 @@ class _NonCountingBackendStore: class _MisShapedBackendStore: - """Fake persistent backend store whose ``count`` attribute is not callable.""" + """Backend store whose ``count`` attribute is not callable.""" + + count = 42 # plain attribute, not a method - count = 42 # not a method — a plain attribute +# --------------------------------------------------------------------------- +# VectorStore.count() dispatch tests +# --------------------------------------------------------------------------- class VectorStoreCountTests(unittest.TestCase): - """VectorStore.count() backend-agnostic accessor.""" + """VectorStore.count() backend-agnostic accessor — dispatch logic.""" def setUp(self): self.vectors = [np.array([1.0, 0.0]), np.array([0.0, 1.0])] @@ -68,10 +87,9 @@ def test_count_raises_not_implemented_without_backend_support(self): store.count() def test_count_raises_when_persistent_backend_not_initialized(self): - # Regression (Qodo review #914): a persistent backend with no - # wrapped store must not silently report 0 — that masks a missing - # initialization as an empty, healthy store. Follow the - # get_vector()/get_metadata() precedent and raise. + # A persistent backend with no wrapped store must not silently + # report 0 — that masks a missing initialization as an empty, + # healthy store. Follow the get_vector()/get_metadata() precedent. store = VectorStore(backend="inmemory", dimension=2) store.backend = "faiss" store._backend_store = None @@ -79,41 +97,103 @@ def test_count_raises_when_persistent_backend_not_initialized(self): store.count() def test_count_raises_when_backend_count_not_callable(self): - # Regression (Qodo review #914): a mis-shaped adapter exposing a - # non-callable ``count`` attribute must surface a clean - # NotImplementedError, not a TypeError. + # A mis-shaped adapter exposing a non-callable ``count`` attribute + # must surface a clean NotImplementedError, not a TypeError. store = VectorStore(backend="inmemory", dimension=2) store.backend = "faiss" store._backend_store = _MisShapedBackendStore() with self.assertRaises(NotImplementedError): store.count() + def test_count_not_implemented_message_describes_requirement(self): + """Error message should explain *how* to fix it, not claim only + inmemory works (the old misleading message).""" + store = VectorStore(backend="inmemory", dimension=2) + store.backend = "qdrant" + store._backend_store = _NonCountingBackendStore() + with self.assertRaises(NotImplementedError) as ctx: + store.count() + msg = str(ctx.exception) + # Must not claim inmemory is the only backend that works + self.assertNotIn("only supported for the inmemory", msg) + # Must point at what to implement + self.assertIn("count()", msg) + + +# --------------------------------------------------------------------------- +# VectorManager tests — inmemory backend +# --------------------------------------------------------------------------- -class VectorManagerPersistentTests(unittest.TestCase): - """VectorManager works on persistent-style stores via count() (#855).""" +class VectorManagerInmemoryTests(unittest.TestCase): + """VectorManager with the inmemory backend — full integrity semantics.""" def setUp(self): self.vectors = [np.array([1.0, 0.0]), np.array([0.0, 1.0])] self.metadata = [{"type": "a"}, {"type": "b"}] self.manager = VectorManager() - def _inmemory_store(self): + def _store(self): store = VectorStore(backend="inmemory", dimension=2) store.store_vectors(self.vectors, self.metadata) return store - def _persistent_store(self, backend_store): - store = self._inmemory_store() - store.backend = "faiss" - store._backend_store = backend_store - return store - def test_collect_statistics_inmemory(self): - stats = self.manager.collect_statistics(self._inmemory_store()) + stats = self.manager.collect_statistics(self._store()) self.assertEqual(stats["total_vectors"], 2) self.assertEqual(stats["dimension"], 2) self.assertEqual(stats["backend"], "inmemory") + def test_collect_statistics_empty_inmemory(self): + store = VectorStore(backend="inmemory", dimension=2) + stats = self.manager.collect_statistics(store) + self.assertEqual(stats["total_vectors"], 0) + + def test_maintain_store_inmemory_healthy(self): + health = self.manager.maintain_store(self._store()) + self.assertTrue(health["healthy"]) + self.assertEqual(health["vector_count"], 2) + self.assertEqual(health["metadata_count"], 2) + + def test_maintain_store_inmemory_empty(self): + store = VectorStore(backend="inmemory", dimension=2) + health = self.manager.maintain_store(store) + self.assertTrue(health["healthy"]) + self.assertEqual(health["vector_count"], 0) + self.assertEqual(health["metadata_count"], 0) + + def test_maintain_store_inmemory_detects_divergence(self): + """Artificially diverge vectors and metadata — must report unhealthy.""" + store = VectorStore(backend="inmemory", dimension=2) + store.store_vectors(self.vectors, self.metadata) + # Inject an extra metadata entry with no matching vector + store.metadata["orphan"] = {"type": "orphan"} + health = self.manager.maintain_store(store) + self.assertFalse(health["healthy"]) + self.assertEqual(health["vector_count"], 2) + self.assertEqual(health["metadata_count"], 3) + + +# --------------------------------------------------------------------------- +# VectorManager tests — persistent backends (dispatch level) +# --------------------------------------------------------------------------- + +class VectorManagerPersistentDispatchTests(unittest.TestCase): + """VectorManager with fake persistent backends — dispatch/contract tests.""" + + def setUp(self): + self.vectors = [np.array([1.0, 0.0]), np.array([0.0, 1.0])] + self.metadata = [{"type": "a"}, {"type": "b"}] + self.manager = VectorManager() + + def _persistent_store(self, backend_store, backend_name="faiss"): + """Create a VectorStore instance whose backend is swapped to a fake.""" + store = VectorStore(backend="inmemory", dimension=2) + store.backend = backend_name + store._backend_store = backend_store + return store + + # -- collect_statistics -------------------------------------------------- + def test_collect_statistics_persistent_with_count(self): store = self._persistent_store(_CountingBackendStore(5)) stats = self.manager.collect_statistics(store) @@ -122,30 +202,256 @@ def test_collect_statistics_persistent_with_count(self): self.assertEqual(stats["backend"], "faiss") def test_collect_statistics_persistent_without_count_raises(self): + """Must raise NotImplementedError, not AttributeError (#855).""" store = self._persistent_store(_NonCountingBackendStore()) - # Regression: must raise NotImplementedError (capability missing), - # not AttributeError (internal attribute poking). with self.assertRaises(NotImplementedError): self.manager.collect_statistics(store) - def test_maintain_store_inmemory(self): - health = self.manager.maintain_store(self._inmemory_store()) - self.assertTrue(health["healthy"]) - self.assertEqual(health["vector_count"], 2) - self.assertEqual(health["metadata_count"], 2) + # -- maintain_store ------------------------------------------------------ def test_maintain_store_persistent_with_count(self): store = self._persistent_store(_CountingBackendStore(5)) health = self.manager.maintain_store(store) self.assertTrue(health["healthy"]) self.assertEqual(health["vector_count"], 5) - self.assertEqual(health["metadata_count"], 5) + # Persistent backends cannot independently verify metadata count. + self.assertIsNone(health["metadata_count"]) + + def test_maintain_store_persistent_metadata_count_is_none_not_vacuous(self): + """Regression for Qodo review #914: maintain_store must not + manufacture metadata_count = vector_count to force healthy=True. + The only way to confirm metadata integrity for a persistent backend + is through the backend itself, so metadata_count must be None. + """ + store = self._persistent_store(_CountingBackendStore(3)) + health = self.manager.maintain_store(store) + # metadata_count must be None — never equal to vector_count because + # we didn't actually verify it; we simply don't have the information. + self.assertIsNone(health["metadata_count"]) + # vector_count comes from the real count() call, not fabricated. + self.assertEqual(health["vector_count"], 3) def test_maintain_store_persistent_without_count_raises(self): + """Must raise NotImplementedError, not AttributeError (#855).""" store = self._persistent_store(_NonCountingBackendStore()) with self.assertRaises(NotImplementedError): self.manager.maintain_store(store) + def test_maintain_store_persistent_not_initialized_raises(self): + store = VectorStore(backend="inmemory", dimension=2) + store.backend = "qdrant" + store._backend_store = None + with self.assertRaises(NotImplementedError): + self.manager.maintain_store(store) + + def test_maintain_store_zero_count_not_confused_with_unhealthy(self): + """An empty but reachable persistent store is healthy (count=0).""" + store = self._persistent_store(_CountingBackendStore(0)) + health = self.manager.maintain_store(store) + self.assertTrue(health["healthy"]) + self.assertEqual(health["vector_count"], 0) + self.assertIsNone(health["metadata_count"]) + + +# --------------------------------------------------------------------------- +# FAISSStore.count() — unit tests with mocked faiss +# --------------------------------------------------------------------------- + +class FAISSStoreCountTests(unittest.TestCase): + """FAISSStore.count() returns len(index.vector_ids).""" + + @patch("semantica.vector_store.faiss_store.faiss") + @patch("semantica.vector_store.faiss_store.FAISS_AVAILABLE", True) + def test_count_after_add(self, mock_faiss): + from semantica.vector_store.faiss_store import FAISSStore + + mock_index = MagicMock() + mock_faiss.IndexFlatL2.return_value = mock_index + + store = FAISSStore(dimension=2) + store.create_index() + + vecs = [np.array([1.0, 0.0]), np.array([0.0, 1.0])] + store.add_vectors(vecs) + self.assertEqual(store.count(), 2) + + @patch("semantica.vector_store.faiss_store.faiss") + @patch("semantica.vector_store.faiss_store.FAISS_AVAILABLE", True) + def test_count_empty_no_index(self, mock_faiss): + from semantica.vector_store.faiss_store import FAISSStore + + store = FAISSStore(dimension=2) + # No index created yet — count() must return 0, not raise. + self.assertEqual(store.count(), 0) + + @patch("semantica.vector_store.faiss_store.faiss") + @patch("semantica.vector_store.faiss_store.FAISS_AVAILABLE", True) + def test_count_via_vectorstore_faiss_backend(self, mock_faiss): + """VectorStore.count() delegates to FAISSStore.count().""" + from semantica.vector_store.faiss_store import FAISSStore + + mock_index = MagicMock() + mock_faiss.IndexFlatL2.return_value = mock_index + + faiss_store = FAISSStore(dimension=2) + faiss_store.create_index() + faiss_store.add_vectors([np.array([1.0, 0.0])]) + + vs = VectorStore(backend="inmemory", dimension=2) + vs.backend = "faiss" + vs._backend_store = faiss_store + + self.assertEqual(vs.count(), 1) + + +# --------------------------------------------------------------------------- +# SQLiteVecStore.count() — unit tests with a real in-memory SQLite DB +# --------------------------------------------------------------------------- + +try: + from semantica.vector_store.sqlite_vec_store import SQLITE_VEC_AVAILABLE +except ImportError: + SQLITE_VEC_AVAILABLE = False + + +@unittest.skipUnless(SQLITE_VEC_AVAILABLE, "sqlite-vec not installed") +class SQLiteVecStoreCountTests(unittest.TestCase): + """SQLiteVecStore.count() executes SELECT COUNT(*) against the db.""" + + def _make_store(self, dimension: int = 2): + """Return a SQLiteVecStore backed by an in-memory SQLite database.""" + from semantica.vector_store.sqlite_vec_store import SQLiteVecStore + + # Use ":memory:" for isolation; each test gets a fresh store. + store = SQLiteVecStore( + db_path=":memory:", + table_name="vecs", + dimension=dimension, + distance_metric="cosine", + ) + return store + + def test_count_empty_store(self): + store = self._make_store() + self.assertEqual(store.count(), 0) + + def test_count_after_add(self): + store = self._make_store() + vecs = [np.array([1.0, 0.0], dtype=np.float32), + np.array([0.0, 1.0], dtype=np.float32)] + meta = [{"k": "a"}, {"k": "b"}] + store.add(vecs, meta) + self.assertEqual(store.count(), 2) + + def test_count_after_delete(self): + store = self._make_store() + vecs = [np.array([1.0, 0.0], dtype=np.float32), + np.array([0.0, 1.0], dtype=np.float32)] + meta = [{"k": "a"}, {"k": "b"}] + ids = store.add(vecs, meta) + store.delete([ids[0]]) + self.assertEqual(store.count(), 1) + + def test_count_matches_get_stats(self): + store = self._make_store() + vecs = [np.array([1.0, 0.0], dtype=np.float32)] + store.add(vecs, [{"k": "x"}]) + stats = store.get_stats() + self.assertEqual(store.count(), stats["vector_count"]) + + def test_vectorstore_count_with_sqlite_backend(self): + """VectorStore.count() delegates to SQLiteVecStore.count().""" + store = self._make_store() + vecs = [np.array([1.0, 0.0], dtype=np.float32), + np.array([0.0, 1.0], dtype=np.float32)] + store.add(vecs, [{}, {}]) + + vs = VectorStore(backend="inmemory", dimension=2) + vs.backend = "sqlite" + vs._backend_store = store + self.assertEqual(vs.count(), 2) + + def test_maintain_store_sqlite_via_vectorstore(self): + """maintain_store() works end-to-end with a real SQLiteVecStore.""" + store = self._make_store() + vecs = [np.array([1.0, 0.0], dtype=np.float32)] + store.add(vecs, [{}]) + + vs = VectorStore(backend="inmemory", dimension=2) + vs.backend = "sqlite" + vs._backend_store = store + + manager = VectorManager() + health = manager.maintain_store(vs) + self.assertTrue(health["healthy"]) + self.assertEqual(health["vector_count"], 1) + self.assertIsNone(health["metadata_count"]) + + +# --------------------------------------------------------------------------- +# PgVectorStore.count() — unit tests with mocked psycopg connection +# --------------------------------------------------------------------------- + +class PgVectorStoreCountTests(unittest.TestCase): + """PgVectorStore.count() runs SELECT COUNT(*) via get_stats().""" + + def _make_mock_store(self, row_count: int): + """Return a PgVectorStore with its connection pool mocked out.""" + try: + from semantica.vector_store.pgvector_store import PgVectorStore + except ImportError: + self.skipTest("psycopg not installed") + + store = PgVectorStore.__new__(PgVectorStore) + store.logger = MagicMock() + store.table_name = "vectors" + store.dimension = 2 + store.distance_metric = "cosine" + store._pool = None + + # Build a mock connection context that returns row_count for COUNT(*) + mock_conn = MagicMock() + mock_cur = MagicMock() + mock_cur.fetchone.return_value = (row_count,) + mock_cur.fetchall.return_value = [] + mock_conn.cursor.return_value = mock_cur + mock_conn.__enter__ = MagicMock(return_value=mock_conn) + mock_conn.__exit__ = MagicMock(return_value=False) + store._get_connection = MagicMock(return_value=mock_conn) + + # Stub out psycopg_sql.SQL so the parameterised query builds without + # a real psycopg installation. + from semantica.vector_store import pgvector_store as pgmod + if not hasattr(pgmod, "psycopg_sql") or pgmod.psycopg_sql is None: + self.skipTest("psycopg_sql not available in pgvector_store module") + + return store + + def test_count_returns_db_value(self): + try: + store = self._make_mock_store(9) + except Exception: + self.skipTest("Could not construct mocked PgVectorStore") + self.assertEqual(store.count(), 9) + + def test_count_zero(self): + try: + store = self._make_mock_store(0) + except Exception: + self.skipTest("Could not construct mocked PgVectorStore") + self.assertEqual(store.count(), 0) + + def test_vectorstore_count_delegates_to_pgvector(self): + try: + store = self._make_mock_store(4) + except Exception: + self.skipTest("Could not construct mocked PgVectorStore") + + vs = VectorStore(backend="inmemory", dimension=2) + vs.backend = "pgvector" + vs._backend_store = store + self.assertEqual(vs.count(), 4) + if __name__ == "__main__": unittest.main() From 859690ca97ea7b655c2488406b0cc78d05e29055 Mon Sep 17 00:00:00 2001 From: KaifAhmad1 Date: Thu, 13 Aug 2026 22:34:12 +0530 Subject: [PATCH 4/4] docs(changelog): document VectorManager persistent-backend count fix (#914, closes #855) Records the VectorStore.count() accessor, the FAISS/SQLite/PgVector implementations added during review, and the maintain_store() metadata_count fix (no longer fabricates equality for persistent backends). --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91887706..6b6b948f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`VectorManager.maintain_store()`/`collect_statistics()` crashed with `AttributeError` on persistent `VectorStore` backends** (#914, closes #855) by @yunaremaia, with fixes by @Sameer6305 + - Both methods accessed `store.vectors`/`store.metadata` directly, which are only initialized for the `inmemory` backend — any persistent backend (FAISS, Qdrant, Pinecone, Milvus, SQLite, PgVector, Weaviate) crashed immediately. Same root cause as the #839/#843/#845/#848 cluster, but `VectorManager` operates on a `VectorStore` instance from the outside, so the fix needed a public accessor rather than another internal guard + - Added a backend-agnostic `VectorStore.count()`: the `inmemory` backend counts its local dict; persistent backends delegate to a `count()` on the wrapped backend store when one exists, or raise `NotImplementedError` — following the `get_vector()`/`get_metadata()` precedent from #843, a missing/uninitialized backend store is never silently reported as an empty, healthy store + - `maintain_store()` and `collect_statistics()` now go through `store.count()` instead of touching `.vectors`/`.metadata` + - **Fixed during review** (@Sameer6305): the initial version had `count()` implemented at the dispatch level only, with no shipped backend actually providing one, and `maintain_store()` manufactured a vacuous `metadata_count == vector_count` tautology for persistent backends (always reporting `healthy: True` without checking anything). Added real `count()` implementations to `FAISSStore` (`len(index.vector_ids)` — FAISS has no delete path, so this list is always consistent with the index), `SQLiteVecStore`, and `PgVectorStore` (both via `SELECT COUNT(*)`); `Qdrant`/`Pinecone`/`Milvus`/`Weaviate` continue to raise `NotImplementedError` since none of them guarantee a cheap, reliable synchronous count. `maintain_store()` now reports `metadata_count: None` for persistent backends instead of the fabricated equality, with `healthy` meaning "store is reachable," not "metadata verified" + - Two earlier Qodo findings (a count() path that silently returned 0 for a missing backend store, and an unvalidated `hasattr` check that could raise `TypeError` on a mis-shaped adapter) were fixed before this review — replaced with `NotImplementedError` and a `getattr`+`callable()` capability check, respectively + - New `tests/vector_store/test_vector_manager_persistent.py`: dispatch-level tests for `count()` (inmemory, delegation, missing backend, non-callable `count`, mis-shaped adapter), full `VectorManager` inmemory semantics including divergence detection, persistent-backend dispatch tests, and backend-specific tests against real/mocked FAISS, SQLite (`sqlite-vec`, skipped if unavailable), and PgVector stores + - Core `vector_store` suite: 40 passed + - **`ContextGraph.get_node_property`/`get_node_attributes` "not found" contract clarified; `add_node_attribute` mutation-callback exception safety fixed** (#882, closes #877) by @ZohaibHassan16 - `get_node_property` returned `None` for both "node missing" and "property missing" with no way to distinguish them, and `get_node_attributes` returned `{}` for a missing node while its siblings disagreed on the not-found signal (`get_node_property`/`find_node` → `None`, `get_edge_data` → `{}`). Both now accept a `default=` parameter matching `dict.get()`'s convention, defaulting to their historical return values (`None` and `{}` respectively) for backward compatibility. Callers that need to disambiguate "node missing" from "value legitimately absent" can pass a private sentinel as `default` - Added Google-style docstrings to `get_node_property`, `get_node_attributes`, `get_edge_data`, and `find_node` documenting each method's not-found contract, addressing #877's "sibling not-found contract undocumented" gap