Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions semantica/vector_store/faiss_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
9 changes: 9 additions & 0 deletions semantica/vector_store/pgvector_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 <table>`` — 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:
Expand Down
9 changes: 9 additions & 0 deletions semantica/vector_store/sqlite_vec_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 <table>`` 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:
Expand Down
71 changes: 63 additions & 8 deletions semantica/vector_store/vector_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -824,6 +824,35 @@ 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" — 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:
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 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 "
f"backend {self.backend!r}."
)

def initialize_decision_pipeline(
self,
graph_store: Optional[Any] = None,
Expand Down Expand Up @@ -1452,21 +1481,47 @@ def manage_store(
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)
"""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]:
"""Collect vector store statistics."""
return {
"total_vectors": len(store.vectors),
"total_vectors": store.count(),
"dimension": store.dimension,
"backend": store.backend,
}
Loading
Loading