mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-16 04:29:31 +00:00
fix(vector_stores): stop sharing one list across default-constructed registries (#38879)
* fix(vector_stores): stop sharing one list across default-constructed registries VectorStoreIndexRegistry and VectorStoreRegistry took a mutable list as their default argument. Python evaluates that once at class definition, so every registry built without arguments shared one list. The proxy builds one that way during startup, so an upsert into it leaked into every registry created after, and into the class default itself. Default to None and build a fresh list per instance. A caller-supplied list is still used as given, including an empty one, so nothing that passes its own list changes behaviour. Fixes #38874 * test(vector_stores): annotate locals Final and mutate registries through their public API The leak tests now go through add_vector_store_to_registry and upsert_vector_store_index rather than appending to the registry's list directly, which is closer to how the proxy hits the bug. * fix(vector_stores): drop the last mutable list default and identity-only tests --------- Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
This commit is contained in:
parent
ad83e3d3c8
commit
391296ffdd
2 changed files with 44 additions and 13 deletions
|
|
@ -2,6 +2,7 @@
|
|||
import json
|
||||
from collections.abc import Mapping
|
||||
from datetime import datetime, timezone
|
||||
from itertools import chain
|
||||
from typing import (
|
||||
TYPE_CHECKING,
|
||||
Any, # noqa: TID251 # untyped non_default_params dict is the only source of the unknown key type
|
||||
|
|
@ -36,8 +37,10 @@ _DB_ROW: Final = TypeAdapter(Mapping[str, object], config=ConfigDict(strict=True
|
|||
|
||||
|
||||
class VectorStoreIndexRegistry:
|
||||
def __init__(self, vector_store_indexes: list[LiteLLM_ManagedVectorStoreIndex] = []):
|
||||
self.vector_store_indexes: list[LiteLLM_ManagedVectorStoreIndex] = vector_store_indexes
|
||||
def __init__(self, vector_store_indexes: list[LiteLLM_ManagedVectorStoreIndex] | None = None):
|
||||
self.vector_store_indexes: list[LiteLLM_ManagedVectorStoreIndex] = (
|
||||
vector_store_indexes if vector_store_indexes is not None else []
|
||||
)
|
||||
|
||||
def get_vector_store_indexes(self) -> list[LiteLLM_ManagedVectorStoreIndex]:
|
||||
"""
|
||||
|
|
@ -109,8 +112,8 @@ class VectorStoreIndexRegistry:
|
|||
|
||||
|
||||
class VectorStoreRegistry:
|
||||
def __init__(self, vector_stores: list[LiteLLM_ManagedVectorStore] = []):
|
||||
self.vector_stores: list[LiteLLM_ManagedVectorStore] = vector_stores
|
||||
def __init__(self, vector_stores: list[LiteLLM_ManagedVectorStore] | None = None):
|
||||
self.vector_stores: list[LiteLLM_ManagedVectorStore] = vector_stores if vector_stores is not None else []
|
||||
self.vector_store_ids_to_vector_store_map: dict[str, LiteLLM_ManagedVectorStore] = {}
|
||||
|
||||
def _extract_tool_params(self, tool: dict) -> VectorStoreToolParams:
|
||||
|
|
@ -392,17 +395,14 @@ class VectorStoreRegistry:
|
|||
|
||||
return vector_stores_to_run
|
||||
|
||||
def _get_vector_store_ids_from_tool_calls(
|
||||
self, tools: list[dict] | None = None, vector_store_ids: list[str] = []
|
||||
) -> list[str]:
|
||||
def _get_vector_store_ids_from_tool_calls(self, tools: list[dict] | None, vector_store_ids: list[str]) -> list[str]:
|
||||
"""
|
||||
Returns the vector store ids from the tool calls
|
||||
"""
|
||||
if tools:
|
||||
for tool in tools:
|
||||
if "vector_store_ids" in tool:
|
||||
vector_store_ids.extend(tool["vector_store_ids"])
|
||||
return vector_store_ids
|
||||
return [
|
||||
*vector_store_ids,
|
||||
*chain.from_iterable(tool["vector_store_ids"] for tool in tools or () if "vector_store_ids" in tool),
|
||||
]
|
||||
|
||||
def load_vector_stores_from_config(self, vector_stores_config: list[dict]):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -15,7 +15,11 @@ from datetime import datetime, timezone
|
|||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import litellm
|
||||
from litellm.types.vector_stores import LiteLLM_ManagedVectorStore
|
||||
from litellm.types.vector_stores import (
|
||||
IndexCreateLiteLLMParams,
|
||||
LiteLLM_ManagedVectorStore,
|
||||
LiteLLM_ManagedVectorStoreIndex,
|
||||
)
|
||||
from litellm.vector_stores.main import search
|
||||
from litellm.vector_stores.vector_store_registry import VectorStoreIndexRegistry, VectorStoreRegistry
|
||||
|
||||
|
|
@ -343,3 +347,30 @@ async def test_vector_store_index_row_without_optional_columns_gets_empty_defaul
|
|||
async def test_malformed_vector_store_index_row_from_the_db_raises_a_validation_error(row: object) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
await VectorStoreIndexRegistry.get_vector_store_indexes_from_db(_database_listing_index_rows([row]))
|
||||
|
||||
|
||||
class TestRegistriesDoNotShareDefaultState:
|
||||
def test_adding_to_one_registry_leaves_the_next_one_empty(self):
|
||||
VectorStoreRegistry().add_vector_store_to_registry(
|
||||
LiteLLM_ManagedVectorStore(vector_store_id="vs-leak", custom_llm_provider="bedrock")
|
||||
)
|
||||
assert VectorStoreRegistry().vector_stores == []
|
||||
|
||||
def test_upserting_into_one_index_registry_leaves_the_next_one_empty(self):
|
||||
VectorStoreIndexRegistry().upsert_vector_store_index(
|
||||
LiteLLM_ManagedVectorStoreIndex(
|
||||
id="idx-leak",
|
||||
index_name="leak",
|
||||
litellm_params=IndexCreateLiteLLMParams(vector_store_index="vs-leak", vector_store_name="leak"),
|
||||
)
|
||||
)
|
||||
assert VectorStoreIndexRegistry().get_vector_store_indexes() == []
|
||||
|
||||
|
||||
def test_vector_store_ids_to_run_merge_the_param_and_tools_once_per_request():
|
||||
registry: Final = VectorStoreRegistry()
|
||||
first: Final = registry.get_vector_store_ids_to_run(
|
||||
{"vector_store_ids": ["vs-a"]}, tools=[{"type": "file_search", "vector_store_ids": ["vs-b", "vs-a"]}]
|
||||
)
|
||||
second: Final = registry.get_vector_store_ids_to_run({}, tools=[{"type": "file_search", "vector_store_ids": ["vs-c"]}])
|
||||
assert (first, second) == (["vs-a", "vs-b"], ["vs-c"])
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue