Skip to content
Open
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
5 changes: 5 additions & 0 deletions openrag/services/storage/vector_store_searcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ async def search(
similarity_threshold: float = 0.0,
with_surrounding_chunks: bool = True,
) -> list[Chunk]:
# Collection is created lazily on first insert (#508); absent means no chunks.
if not await self._store.collection_exists(self._collection):
return []
Comment on lines +72 to +74

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Expect either no concurrent collection-drop path or explicit missing-collection handling.
rg -n -C 8 'drop_collection|release_collection|has_collection|collection_exists|VDBSearchError' openrag tests

Repository: linagora/openrag

Length of output: 50372


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Inspect the relevant searcher/store implementation and exception shape without running repo code.
sed -n '1,150p' openrag/services/storage/vector_store_searcher.py
printf '\n--- exceptions around VDBSearchError / VDBPartitionNotFound ---\n'
sed -n '35,46p' openrag/core/utils/exceptions.py
sed -n '400,412p' openrag/core/utils/exceptions.py
printf '\n--- Milvus search wrappers and collection drop/resolvers ---\n'
sed -n '800,1105p' openrag/services/storage/milvus_store.py
sed -n '880,980p' openrag/services/storage/milvus_store.py

Repository: linagora/openrag

Length of output: 23633


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Find all search_call sites around collection_exists and whether they catch missing-collection errors.
rg -n -C 4 'collection_exists\(.*collection|\.search\(|\.hybrid_search\(' openrag/services/storage/vector_store_searcher.py openrag/services/storage/milvus_store.py tests/integration/repos/test_milvus_store_integration.py tests/integration/repos/test_workspace_scoping_e2e.py

Repository: linagora/openrag

Length of output: 12540


Handle collection absence atomically in search.

collection_exists() and search() are separate calls. If drop_collection() runs concurrently, search() can still translate the missing collection failure to VDBSearchError. Make the store operation atomic, or catch the missing-collection state at VectorStoreSearcher and return an empty result.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openrag/services/storage/vector_store_searcher.py` around lines 72 - 74,
Update the search flow in VectorStoreSearcher to handle a collection being
dropped between collection_exists() and search(). Prefer an atomic store-level
search operation; otherwise catch the store’s missing-collection error around
search and return [] while preserving VDBSearchError translation for other
failures.

(embedding,) = await self._embedder.embed([query])
filters: dict[str, Any] = {"partition": partition}
if filter:
Expand Down Expand Up @@ -100,6 +103,8 @@ async def multi_query_search(
similarity_threshold: float = 0.0,
with_surrounding_chunks: bool = True,
) -> list[Chunk]:
if not await self._store.collection_exists(self._collection): # see search()
return []
embeddings = await self._embedder.embed(queries)
filters: dict[str, Any] = {"partition": partition}
if filter:
Expand Down
32 changes: 32 additions & 0 deletions tests/unit/services/storage/test_vector_store_searcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def _make_searcher(
ancestor_ids=None,
) -> tuple[VectorStoreSearcher, MagicMock, MagicMock, MagicMock]:
store = MagicMock()
store.collection_exists = AsyncMock(return_value=True)
store.search = AsyncMock(return_value=search_results or [])
store.query_chunks_by_filter = AsyncMock(return_value=filter_results or [])

Expand Down Expand Up @@ -87,6 +88,22 @@ def test_dict_to_chunk_metadata_excludes_reserved_keys():
# ---------------------------------------------------------------------------


@pytest.mark.asyncio
async def test_search_returns_empty_when_collection_does_not_exist():
"""Regression for #505-class bugs: on a fresh stack nothing has ever been
indexed, so the shared collection doesn't exist yet. search() must return
no matches instead of embedding the query and raising on the missing
collection."""
searcher, store, embedder, _ = _make_searcher()
store.collection_exists = AsyncMock(return_value=False)

result = await searcher.search(query="hello", partition=["p1"], top_k=5)

assert result == []
embedder.embed.assert_not_awaited()
store.search.assert_not_awaited()


@pytest.mark.asyncio
async def test_search_embeds_query_and_calls_store():
searcher, store, embedder, _ = _make_searcher(
Expand Down Expand Up @@ -170,6 +187,7 @@ async def test_multi_query_search_merges_filter_params():
embedder = MagicMock()
embedder.embed = AsyncMock(return_value=[_EMBED_VEC, _EMBED_VEC])
store = MagicMock()
store.collection_exists = AsyncMock(return_value=True)
store.search = AsyncMock(return_value=[])
store.query_chunks_by_filter = AsyncMock(return_value=[])
searcher = VectorStoreSearcher(vector_store=store, embedder=embedder, document_repo=MagicMock(), collection="col")
Expand Down Expand Up @@ -239,12 +257,25 @@ async def test_search_skips_surrounding_when_no_results():
# ---------------------------------------------------------------------------


@pytest.mark.asyncio
async def test_multi_query_search_returns_empty_when_collection_does_not_exist():
searcher, store, embedder, _ = _make_searcher()
store.collection_exists = AsyncMock(return_value=False)

result = await searcher.multi_query_search(queries=["q1", "q2"], partition=["p1"], top_k_per_query=5)

assert result == []
embedder.embed.assert_not_awaited()
store.search.assert_not_awaited()


@pytest.mark.asyncio
async def test_multi_query_search_embeds_all_queries():
embedder = MagicMock()
embedder.embed = AsyncMock(return_value=[_EMBED_VEC, _EMBED_VEC])

store = MagicMock()
store.collection_exists = AsyncMock(return_value=True)
store.search = AsyncMock(return_value=[])
store.query_chunks_by_filter = AsyncMock(return_value=[])

Expand All @@ -267,6 +298,7 @@ async def test_multi_query_search_deduplicates_across_queries():

# Both queries return the same chunk "1" plus a unique one each
store = MagicMock()
store.collection_exists = AsyncMock(return_value=True)
store.search = AsyncMock(
side_effect=[
[_make_row("1"), _make_row("2")],
Expand Down
Loading