Add vector store retrieve list update delete - #23435
Conversation
- Add vector_store_retrieve/list/update/delete handlers in llm_http_handler - Fix AsyncHTTPHandler.get() timeout arg (not supported) - Fix update/delete URL (api_base already includes /vector_stores) - Clean metadata for update to avoid UserAPIKeyAuth JSON serialization Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds vector store retrieve, list, update, and delete operations across the full litellm stack: new HTTP handler methods in Key issues found:
Confidence Score: 2/5
|
| Filename | Overview |
|---|---|
| litellm/llms/custom_httpx/llm_http_handler.py | Adds 536 lines of new vector store handlers (retrieve, list, update, delete — both sync and async). The sync update and sync delete handlers silently drop the timeout parameter (async counterparts pass it correctly). Several previously flagged issues remain: timeout missing from retrieve/list handlers, list/delete return raw response.json() without provider transformation, and inline import of add_openai_metadata inside both update handlers. |
| litellm/proxy/vector_store_endpoints/endpoints.py | Adds four new FastAPI endpoints: GET /vector_stores/{id} (retrieve), GET /vector_stores (list), POST /vector_stores/{id} (update), DELETE /vector_stores/{id} (delete). All follow the established proxy request-processing pattern and correctly delegate to the router via route_type. No significant new issues beyond what's already flagged in the handlers. |
| litellm/vector_stores/main.py | Adds aretrieve/retrieve, alist/list, aupdate/update, adelete/delete public functions following the existing create/search patterns. The list function name shadows Python's built-in (previously flagged). The alist function references list via partial(list, ...) which refers to the newly-defined list function — this is consistent but confusing due to the naming collision. |
| litellm/router.py | Registers the new vector store endpoints (retrieve, list, update, delete) in the Router, adds corresponding call_types to the factory function dispatch logic, and imports list from vector_stores/main. The list name collision carries over here as the imported name. |
| tests/test_new_vector_store_endpoints.py | Adds mock-based tests for all four new operations. Tests are placed in tests/ instead of tests/test_litellm/ (previously flagged). The sync tests (test_vector_store_list_with_pagination, test_vector_store_update_with_expires_after) patch the module-level function name, which may not intercept calls that go through the Router's factory function if it holds a direct reference to the original function object. |
Sequence Diagram
sequenceDiagram
participant Client
participant ProxyEndpoint as proxy/vector_store_endpoints
participant Router as litellm.Router
participant VectorStoreMain as vector_stores/main.py
participant HTTPHandler as BaseLLMHTTPHandler
participant Provider as OpenAI / Provider API
Client->>ProxyEndpoint: GET /vector_stores/{id}
ProxyEndpoint->>Router: avector_store_retrieve(vector_store_id)
Router->>VectorStoreMain: aretrieve(...)
VectorStoreMain->>VectorStoreMain: loop.run_in_executor(retrieve)
VectorStoreMain->>HTTPHandler: vector_store_retrieve_handler(_is_async=True)
HTTPHandler->>HTTPHandler: async_vector_store_retrieve_handler()
HTTPHandler->>Provider: GET {api_base}/{vector_store_id}
Provider-->>HTTPHandler: HTTP 200 response
HTTPHandler-->>VectorStoreMain: VectorStoreCreateResponse
VectorStoreMain-->>Router: VectorStoreCreateResponse
Router-->>ProxyEndpoint: response
ProxyEndpoint-->>Client: JSON
Client->>ProxyEndpoint: GET /vector_stores
ProxyEndpoint->>Router: avector_store_list(limit, order, ...)
Router->>VectorStoreMain: alist(...)
VectorStoreMain->>HTTPHandler: vector_store_list_handler(_is_async=True)
HTTPHandler->>Provider: GET {api_base}?limit=N&order=desc
Provider-->>HTTPHandler: HTTP 200 response
HTTPHandler-->>Client: response.json() [raw dict]
Client->>ProxyEndpoint: POST /vector_stores/{id}
ProxyEndpoint->>Router: avector_store_update(vector_store_id, ...)
Router->>VectorStoreMain: aupdate(...)
VectorStoreMain->>HTTPHandler: vector_store_update_handler(_is_async=True)
HTTPHandler->>Provider: POST {api_base}/{id} JSON body
Provider-->>HTTPHandler: HTTP 200 response
HTTPHandler-->>Client: VectorStoreCreateResponse
Client->>ProxyEndpoint: DELETE /vector_stores/{id}
ProxyEndpoint->>Router: avector_store_delete(vector_store_id)
Router->>VectorStoreMain: adelete(...)
VectorStoreMain->>HTTPHandler: vector_store_delete_handler(_is_async=True)
HTTPHandler->>Provider: DELETE {api_base}/{id}
Provider-->>HTTPHandler: HTTP 200 response
HTTPHandler-->>Client: response.json() [raw dict]
Last reviewed commit: 5927345
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -0,0 +1,364 @@ | |||
| """ | |||
There was a problem hiding this comment.
Test file placed in wrong directory
Per the pre-submission checklist and repository convention (rule for tests/test_litellm/), tests should be added under tests/test_litellm/, not the root tests/ directory. These tests correctly use mocking (AsyncMock, patch), so they're eligible to live in tests/test_litellm/ — they just need to be moved there so CI picks them up as part of the standard unit test suite (make test-unit).
Rule Used: What: prevent any tests from being added here that... (source)
| kwargs["alist"] = True | ||
|
|
||
| if custom_llm_provider is None: | ||
| custom_llm_provider = "openai" |
There was a problem hiding this comment.
list shadows Python's built-in list
Naming this function list overwrites the Python built-in list type for the rest of the module. While it works at runtime (Python resolves names at call time), it means any code after this definition in the same module that attempts to use the built-in list type (e.g. list[str], isinstance(x, list)) will silently use this function instead. It also makes the alist implementation above confusing — partial(list, ...) appears to be calling the built-in when in fact it refers to this decorated function.
Consider renaming to vector_store_list (matching the pattern used in the Router) to avoid the shadowing:
| custom_llm_provider = "openai" | |
| def vector_store_list( |
| ) | ||
|
|
||
| def vector_store_retrieve_handler( |
There was a problem hiding this comment.
Timeout not forwarded to HTTP client
The timeout parameter is accepted but silently dropped — it is never passed to async_httpx_client.get(...). This means long-running retrieve calls will use the httpx client's default timeout instead of the caller-specified value. The same issue exists in the sync vector_store_retrieve_handler and in both sync/async vector_store_list_handler variants.
| ) | |
| def vector_store_retrieve_handler( | |
| response = await async_httpx_client.get( | |
| url=url, headers=headers, timeout=timeout | |
| ) |
| ) | ||
|
|
||
| url = f"{api_base}/{vector_store_id}" | ||
|
|
There was a problem hiding this comment.
Timeout not forwarded in sync retrieve handler
The timeout parameter is silently dropped here too.
| response = sync_httpx_client.get(url=url, headers=headers, timeout=timeout) |
| "api_base": api_base, | ||
| "headers": headers, | ||
| "params": params, |
There was a problem hiding this comment.
list and delete handlers bypass provider response transformation
async_vector_store_list_handler, vector_store_list_handler, async_vector_store_delete_handler, and vector_store_delete_handler all return response.json() directly — a raw dict. In contrast, retrieve and update go through vector_store_provider_config.transform_create_vector_store_response(response=response), which applies provider-specific parsing and validation.
Returning raw response.json() means:
- No provider-specific transformation is applied (e.g. field renaming for non-OpenAI providers).
- The return type is inconsistent — callers have no guarantee about the shape of the result.
If a typed transformation for list/delete responses doesn't exist yet, consider adding transform_list_vector_stores_response / transform_delete_vector_store_response methods to BaseVectorStoreConfig, or at minimum apply response.raise_for_status() before returning the dict to surface HTTP errors early.
| vector_store_update_optional_params: VectorStoreCreateOptionalRequestParams, | ||
| vector_store_provider_config: BaseVectorStoreConfig, |
There was a problem hiding this comment.
Inline import inside hot-path method
from litellm.utils import add_openai_metadataThis inline import runs on every call to async_vector_store_update_handler (and the sync counterpart). Python caches module imports, so the overhead is small, but it is non-idiomatic and hard to spot during refactors. Move this import to the top of the file alongside the existing litellm.utils imports.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
@greptile re-review with updated commits |
| response = sync_httpx_client.post( | ||
| url=url, headers=headers, json=request_body |
There was a problem hiding this comment.
Timeout dropped in sync vector_store_update_handler
The timeout parameter is silently ignored in the synchronous POST path. The async counterpart (async_vector_store_update_handler) correctly forwards it via timeout=timeout, but the sync path does not:
response = sync_httpx_client.post(
url=url, headers=headers, json=request_body
)This means callers using the sync path will always use the httpx default timeout regardless of what they pass.
| response = sync_httpx_client.post( | |
| url=url, headers=headers, json=request_body | |
| response = sync_httpx_client.post( | |
| url=url, headers=headers, json=request_body, timeout=timeout | |
| ) |
| ) | ||
|
|
||
| try: | ||
| response = sync_httpx_client.delete(url=url, headers=headers) |
There was a problem hiding this comment.
Timeout dropped in sync vector_store_delete_handler
The timeout parameter is silently ignored in the synchronous DELETE path. The async counterpart correctly passes timeout=timeout to async_httpx_client.delete(...), but the sync path does not:
response = sync_httpx_client.delete(url=url, headers=headers)| response = sync_httpx_client.delete(url=url, headers=headers) | |
| response = sync_httpx_client.delete(url=url, headers=headers, timeout=timeout) |
…trieve-list-update-delete Add vector store retrieve list update delete
2 participants
Relevant issues
Fixes LIT-2180
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes