Skip to content

fix(vector-stores): survive a failing vector store search in the chat completions hook - #39495

Merged
mateo-berri merged 4 commits into
litellm_internal_stagingfrom
litellm_vector_store_hook_router_injection
Sep 3, 2026
Merged

fix(vector-stores): survive a failing vector store search in the chat completions hook#39495
mateo-berri merged 4 commits into
litellm_internal_stagingfrom
litellm_vector_store_hook_router_injection

Conversation

@mateo-berri

@mateo-berri mateo-berri commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Follows #39474, now merged, which moved S3 Vectors query embeddings onto the shared executor.

TLDR

Problem this solves:

  • One unreachable vector store drops every store's context
  • The failure log never says which store broke
  • With two stores, only the last one's context reaches the model

How it solves it:

  • Each store's search gets its own error handler
  • The warning names the vector store id that failed
  • Context now chains through every store that answered
  • The hook takes its Router as an injected dependency

User Flow

Before: a developer whose chat app searches several vector stores loses all retrieved context the moment one store is unreachable, and the log never says which one

  1. They POST https://litellm-domain/v1/chat/completions with "vector_store_ids": ["<store whose index was deleted>", "<healthy runbook store>"] and a question the runbook answers
  2. The reply is 200 but the model guesses: "drain the LBs / load balancers", and no search results come back with it
  3. They retry over two healthy stores, ["<runbook store>", "<rollback window store>"], asking for a fact from each, and the answer still only has the second store's fact
  4. The proxy log shows one line, "Error in VectorStorePreCallHook", with no vector store id in it, so they bisect the id list by hand to find the bad store

After: the same requests answer from every store that is reachable, and the log names the one that is not

  1. They POST https://litellm-domain/v1/chat/completions with the same "vector_store_ids": ["<store whose index was deleted>", "<healthy runbook store>"]
  2. The reply is 200 and grounded in the healthy store: "The Osaka queue must be drained before flipping DNS", with one search result attached
  3. They retry over the two healthy stores and get both facts back: the drain step from the first store and the 14 minute rollback window from the second
  4. The proxy log carries a warning naming vector_store_id=<store whose index was deleted> and the provider error next to it, so the bad store is obvious

Relevant issues

Linear ticket

Resolves LIT-6752

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

Two proxies on the same Postgres, one per side, both with 2 workers, real OpenAI models and real S3 Vectors stores in eu-central-1.

model_list:
  - model_name: gpt-5.4-mini
    litellm_params:
      model: openai/gpt-5.4-mini
      api_key: os.environ/OPENAI_API_KEY
  - model_name: qa-team-embeddings
    litellm_params:
      model: openai/text-embedding-3-small
      api_key: os.environ/OPENAI_API_KEY
general_settings:
  master_key: sk-lit6750
  store_model_in_db: true

Shared setup, run once on the head proxy: two documents ingested into their own S3 Vectors stores, plus a third store registered against an index that does not exist.

$ curl -s http://127.0.0.1:34446/v1/rag/ingest -H "Authorization: Bearer $TEAM_KEY" \
    -F file=@lit6750-doc.txt \
    -F 'request={"ingest_options":{"embedding":{"model":"qa-team-embeddings"},"vector_store":{"custom_llm_provider":"s3_vectors","vector_bucket_name":"lit6750-qa-vectors","aws_region_name":"eu-central-1","embedding_model":"qa-team-embeddings"}}}'
{"id":"ingest_...","status":"completed","vector_store_id":"lit6750-qa-vectors:litellm-index-705d1dd1","file_id":"..."}

$ curl -s http://127.0.0.1:34446/v1/rag/ingest -H "Authorization: Bearer $TEAM_KEY" \
    -F file=@lit6752-doc-b.txt \
    -F 'request={"ingest_options":{"embedding":{"model":"qa-team-embeddings"},"vector_store":{"custom_llm_provider":"s3_vectors","vector_bucket_name":"lit6750-qa-vectors","aws_region_name":"eu-central-1","embedding_model":"qa-team-embeddings"}}}'
{"id":"ingest_...","status":"completed","vector_store_id":"lit6750-qa-vectors:litellm-index-995246bd","file_id":"..."}

$ curl -s http://127.0.0.1:34446/vector_store/new -H "Authorization: Bearer sk-lit6750" \
    -H 'Content-Type: application/json' \
    -d '{"vector_store_id":"lit6750-qa-vectors:lit6752-missing-p5","custom_llm_provider":"s3_vectors","litellm_params":{"aws_region_name":"eu-central-1","embedding_model":"qa-team-embeddings"}}'
{"status":"success","message":"Vector store lit6750-qa-vectors:lit6752-missing-p5 created successfully", ...}

litellm-index-705d1dd1 holds "The Kyoto failover runbook: drain the Osaka queue before flipping DNS, then page the on-call owl." and litellm-index-995246bd holds "The Kyoto rollback window is 14 minutes, after which the failover becomes permanent." lit6752-missing-p5 points at an S3 Vectors index that was never created.

Before (066d5f0)

One unreachable store next to a healthy one

  1. Ask a question the runbook store answers, with the unreachable store first in the list
$ curl -s http://127.0.0.1:31642/v1/chat/completions -H "Authorization: Bearer $TEAM_KEY" \
    -H 'Content-Type: application/json' \
    -d '{"model":"gpt-5.4-mini","vector_store_ids":["lit6750-qa-vectors:lit6752-missing-p5","lit6750-qa-vectors:litellm-index-705d1dd1"],"messages":[{"role":"user","content":"What must be drained before flipping DNS in the Kyoto failover runbook? Answer in one sentence."}]}'
{"content": "Before flipping DNS, the application traffic/load balancer must be drained so no requests are still flowing to the old Kyoto environment.", "search_results_returned": 0}

The healthy store never gets searched, so the answer is the model's own guess and no search results come back.

  1. The proxy log names no store
00:38:58 - LiteLLM:ERROR: vector_store_pre_call_hook.py:171 - Error in VectorStorePreCallHook: litellm.NotFoundError: S3_vectorsException - {"message":"The specified index could not be found"}

Two healthy stores

  1. Ask for one fact from each store
$ curl -s http://127.0.0.1:31642/v1/chat/completions -H "Authorization: Bearer $TEAM_KEY" \
    -H 'Content-Type: application/json' \
    -d '{"model":"gpt-5.4-mini","vector_store_ids":["lit6750-qa-vectors:litellm-index-705d1dd1","lit6750-qa-vectors:litellm-index-995246bd"],"messages":[{"role":"user","content":"In the Kyoto failover: what must be drained before flipping DNS, and how long is the rollback window? Answer in one sentence covering both."}]}'
{"content": "Before flipping DNS, the old primary must be fully drained, and the rollback window is 14 minutes."}

Only the last store's fact (14 minutes) is real; the drain target is invented, because the first store's context never reached the model.

After (0e537d2)

One unreachable store next to a healthy one

  1. Same request, same two stores, against the head proxy
$ curl -s http://127.0.0.1:34446/v1/chat/completions -H "Authorization: Bearer $TEAM_KEY" \
    -H 'Content-Type: application/json' \
    -d '{"model":"gpt-5.4-mini","vector_store_ids":["lit6750-qa-vectors:lit6752-missing-p5","lit6750-qa-vectors:litellm-index-705d1dd1"],"messages":[{"role":"user","content":"What must be drained before flipping DNS in the Kyoto failover runbook? Answer in one sentence."}]}'
{"content": "The Osaka queue must be drained before flipping DNS.", "search_results_returned": 1}
  1. The proxy log names the store that failed
00:39:05 - LiteLLM:WARNING: vector_store_pre_call_hook.py:164 - Vector store search failed for vector_store_id=lit6750-qa-vectors:lit6752-missing-p5, continuing without its context: litellm.NotFoundError: S3_vectorsException - {"message":"The specified index could not be found"}

Two healthy stores

  1. Same request as before
$ curl -s http://127.0.0.1:34446/v1/chat/completions -H "Authorization: Bearer $TEAM_KEY" \
    -H 'Content-Type: application/json' \
    -d '{"model":"gpt-5.4-mini","vector_store_ids":["lit6750-qa-vectors:litellm-index-705d1dd1","lit6750-qa-vectors:litellm-index-995246bd"],"messages":[{"role":"user","content":"In the Kyoto failover: what must be drained before flipping DNS, and how long is the rollback window? Answer in one sentence covering both."}]}'
{"content": "Before flipping DNS, the Osaka queue must be drained, and the rollback window is 14 minutes."}

Both stores' facts are in the answer.

Type

🐛 Bug Fix

Caveats (if any)

Medium

  • A failed search stays non-fatal, by choice
    • Surfacing it would turn one bad store into a failed request
    • The request answers from the stores that did work
    • The warning naming the store is the operator's signal
  • The outer catch-all around the whole hook stays
    • It still swallows non-search failures with no store id

Low

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

Note

Medium Risk
Changes RAG pre-call behavior for all multi-store chat requests; failures are now non-fatal per store, which improves availability but can hide misconfiguration unless operators watch warnings.

Overview
Hardens the vector store pre-call hook for chat completions when requests list multiple vector_store_ids.

Per-store failures are caught inside the search loop: a warning logs the failing vector_store_id and the hook continues so healthy stores still run. The chat request is not aborted for one bad index.

Multi-store context now chains correctly by appending each successful search onto modified_messages instead of the original message list, so every reachable store contributes context (not only the last one).

Proxy access is refactored through an injectable ProxyRuntime (ProxyServerRuntime by default) for llm_router and prisma_client, replacing inline proxy_server imports in the hook path. New unit tests cover router injection, SDK fallback, partial failure, and the default runtime.

Reviewed by Cursor Bugbot for commit 0e537d2. Bugbot is set up for automated code reviews on this repo. Configure here.

… completions hook

One unreachable vector store used to wipe out every store's context on a
chat completion carrying vector_store_ids: the search raised, the blanket
handler returned the original messages, and the request answered with no
retrieved context at all. Each store's search now has its own handler that
warns with the vector store id and moves on to the next store.

The same loop appended every store's results to the original messages
instead of the running copy, so with two healthy stores only the last one
reached the model. It now chains through modified_messages.

The Router is injected through a ProxyRuntime protocol instead of an
in-function litellm.proxy.proxy_server import, so the hook's routing can
be driven in tests without touching proxy globals.
@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR makes vector-store searches independently recoverable and accumulates context from every successful store.

  • Injects proxy runtime dependencies for router and Prisma access.
  • Logs failed searches with their vector-store identifiers while continuing remaining searches.
  • Chains successful search context and preserves successful results for citations.
  • Adds regression coverage for router injection, SDK fallback, multiple stores, and partial failures.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
litellm/integrations/vector_store_integrations/vector_store_pre_call_hook.py Adds injectable proxy runtime access, isolates individual search failures, and accumulates context across successful vector stores without an identified blocking defect.
tests/test_litellm/integrations/vector_store_integrations/test_vector_store_pre_call_hook.py Adds strongly typed regression coverage for router selection, fallback behavior, context accumulation, warning details, and partial failures.

Reviews (4): Last reviewed commit: "Merge remote-tracking branch 'origin/lit..." | Re-trigger Greptile

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...r_store_integrations/vector_store_pre_call_hook.py 84.61% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@mateo-berri

Copy link
Copy Markdown
Contributor Author

bugbot run

@mateo-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

@mateo-berri mateo-berri added run-ci and removed run-ci labels Sep 3, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 6966a33. Configure here.

Base automatically changed from litellm_s3_vectors_query_embedding_executor to litellm_internal_staging September 3, 2026 05:23
@codspeed-hq

codspeed-hq Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_vector_store_hook_router_injection (0e537d2) with litellm_internal_staging (ecabfbd)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_internal_staging (658f506) during the generation of this report, so ecabfbd was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@mateo-berri

Copy link
Copy Markdown
Contributor Author

bugbot run

@mateo-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

@mateo-berri mateo-berri added run-ci and removed run-ci labels Sep 3, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b503bca. Configure here.

@mateo-berri mateo-berri added run-ci and removed run-ci labels Sep 3, 2026
@mateo-berri

Copy link
Copy Markdown
Contributor Author

bugbot run

@mateo-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 0e537d2. Configure here.

@mateo-berri
mateo-berri merged commit 00faaa1 into litellm_internal_staging Sep 3, 2026
127 of 130 checks passed
@mateo-berri
mateo-berri deleted the litellm_vector_store_hook_router_injection branch September 3, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants