Skip to content

fix(vector-stores): surface retrieval failures to the API caller - #39516

Merged
mateo-berri merged 10 commits into
litellm_internal_stagingfrom
litellm_vector_store_surface_retrieval_failure
Sep 11, 2026
Merged

mateo-berri merged 10 commits into
litellm_internal_stagingfrom
litellm_vector_store_surface_retrieval_failure

Conversation

@mateo-berri

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

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • A broken vector store still returns 200
  • The answer silently skips the knowledge base
  • The caller has no way to tell
  • The catch-all log line names no store id

How it solves it:

  • Failed searches ride provider_specific_fields, next to search_results
  • /v1/responses carries the same list as a top-level response field
  • New vector_store_search_failure_mode: error fails closed instead
  • Default stays permissive, so nothing changes silently
  • The catch-all now names the requested vector store ids

User Flow

Before: a developer whose support bot grounds answers in a knowledge base gets confident, wrong answers after that knowledge base breaks, and nothing in the response says so

  1. They register their knowledge base once with POST https://litellm-domain/vector_store/new and it starts answering grounded questions
  2. Someone deletes the underlying index, so the knowledge base no longer exists
  3. They send POST https://litellm-domain/v1/chat/completions with "vector_store_ids": ["their-store"] and a question only the knowledge base can answer
  4. They get back HTTP 200 with "I don't have access to that runbook", written from the model's own knowledge
  5. They read provider_specific_fields on the message and find only {"refusal": null}: no store id, no error, nothing naming the knowledge base
  6. They read all 21 x-litellm-* response headers and none of them mentions a vector store either
  7. Their bot ships the ungrounded answer to an end user, and the only evidence anything went wrong is a WARNING line in the proxy's own logs, which their application never sees

After: the same request comes back naming the store that failed, and an operator who would rather fail than answer ungrounded can turn the failure into a 400

  1. They send the same POST https://litellm-domain/v1/chat/completions with "vector_store_ids": ["their-store"]
  2. They still get HTTP 200 with the same ungrounded answer, so no existing deployment changes behavior
  3. provider_specific_fields on the message now also carries vector_store_search_failures, a list naming the store id, its provider, and the provider's error text
  4. Their bot checks that field, sees the knowledge base was skipped, and holds the answer back instead of shipping it
  5. The same is true on POST https://litellm-domain/v1/responses, where vector_store_search_failures arrives as a top-level field on the response
  6. A proxy admin who wants the request to fail instead sets vector_store_search_failure_mode: error under litellm_settings and restarts the proxy
  7. The same POST now returns HTTP 400 with "type": "invalid_request_error" and a message naming every store that failed and why, on both endpoints
  8. A request whose knowledge base is healthy is untouched in both modes: HTTP 200, grounded answer, search_results as before, and no failures field
  9. If that admin misspells the value, the proxy logs a warning naming the supported modes and keeps answering as it does today, so a typo never turns into failed requests

Design: why a response field plus an opt-in error

Three signals were on the table, and the answer is different for the two things a caller might want.

For "tell me it happened", the response field is right. The hook already owns exactly that lane: successful searches ride provider_specific_fields.search_results today, so failures riding provider_specific_fields.vector_store_search_failures need no new concept and no new plumbing. It is purely additive: same status code, same content, same existing fields, so it ships on by default without a knob.

Both endpoints the hook actually runs on carry it. On chat completions it sits under provider_specific_fields on the message and on the streaming delta, next to search_results. /v1/responses has no such container, so it rides the response object itself as a top-level vector_store_search_failures, which ResponsesAPIResponse already allows and already serializes. search_results is deliberately not added there: it would put whole retrieved documents on a surface that has never carried them, which is a bigger change than this fix needs.

A header was rejected. The hook also runs on the pure SDK path (litellm.acompletion), where there are no response headers at all, so a header would leave SDK callers exactly as blind as they are today. It also cannot carry per-store structured data without inventing an encoding that each surface would have to re-implement.

For "don't answer without the knowledge base", only an error works, and that is a real behavior change, so it is opt-in: litellm_settings.vector_store_search_failure_mode defaults to annotate (today's behavior) and can be set to error. Failing closed by default would turn one bad knowledge base into failed requests for everyone who has one configured. The error is a BadRequestError subclass, so the caller gets HTTP 400 with "type": "invalid_request_error", matching what OpenAI returns for a bad vector store id, and 400s are never retried and never cool a deployment down. litellm_settings keys are set on the litellm module with no allowlist, so a typo like erorr would otherwise reach the exhaustive match: an unrecognized value falls back to annotate and logs a warning naming the supported modes, so a misconfiguration degrades to today's behavior instead of failing every vector-store request.

Relevant issues

Linear ticket

Resolves LIT-6809

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

Real OpenAI gpt-5.6 for chat and text-embedding-3-small for the query embeddings, real AWS S3 Vectors for the knowledge base, real spend on every leg.

Shared setup, run once against each proxy:

# config.yaml
model_list:
  - model_name: gpt-5.6
    litellm_params:
      model: openai/gpt-5.6
      api_key: os.environ/OPENAI_API_KEY
  - model_name: lit6809-embeddings
    litellm_params:
      model: openai/text-embedding-3-small
      api_key: os.environ/OPENAI_API_KEY
general_settings:
  master_key: sk-lit6809
  store_model_in_db: true

config_error.yaml is the same file plus:

litellm_settings:
  vector_store_search_failure_mode: error

config_typo.yaml is the same file with vector_store_search_failure_mode: erorr, the misconfiguration case.

The knowledge base is a made-up "Project Halcyon" runbook whose only distinctive fact is that the Reykjavik shard freeze hold must be exactly 900 seconds, so a grounded answer proves retrieval ran and an ungrounded one proves it did not.

# ingest the runbook, creating the healthy store
curl -s $PROXY/v1/rag/ingest -H "Authorization: Bearer sk-lit6809" \
  -F file=@lit6809-doc.txt \
  -F 'request={"ingest_options":{"embedding":{"model":"lit6809-embeddings"},"vector_store":{"custom_llm_provider":"s3_vectors","vector_bucket_name":"lit6750-qa-vectors","aws_region_name":"eu-central-1","embedding_model":"lit6809-embeddings"}}}'
# {"id":"ingest_e2d19604-...","status":"completed","vector_store_id":"lit6750-qa-vectors:litellm-index-ac0bfb1a",...}

# register a store whose index was never created: valid config, valid credentials, no index
curl -s $PROXY/vector_store/new -H "Authorization: Bearer sk-lit6809" -H 'Content-Type: application/json' \
  -d '{"vector_store_id":"lit6750-qa-vectors:lit6809-index-never-created","custom_llm_provider":"s3_vectors","litellm_params":{"aws_region_name":"eu-central-1","embedding_model":"lit6809-embeddings"}}'

Q='In the Project Halcyon runbook, what exact hold duration must the Reykjavik shard freeze use? Answer in one short sentence.'

Before (0e537d2, this PR's merge base against litellm_vector_store_hook_router_injection)

Re-run on the /live-pr-risk rig at litellm_internal_staging df3b8a6 (two uvicorn workers, its own Postgres): every case below answers the same way there, so what ships without this PR is what these cases show.

Case 1: healthy store, chat completions

  1. curl -s -w '\nHTTP %{http_code}\n' $PROXY/v1/chat/completions -H "Authorization: Bearer sk-lit6809" -H 'Content-Type: application/json' -d '{"model":"gpt-5.6","vector_store_ids":["lit6750-qa-vectors:litellm-index-ac0bfb1a"],"messages":[{"role":"user","content":"'"$Q"'"}]}'
  2. HTTP 200, grounded: "content":"The Reykjavik shard freeze must use a hold duration of exactly 900 seconds." with provider_specific_fields carrying search_results

Case 2: broken store, chat completions, default settings

  1. Same command with "vector_store_ids":["lit6750-qa-vectors:lit6809-index-never-created"]
  2. HTTP 200, ungrounded: "content":"I don’t have access to the Project Halcyon runbook; please provide the relevant excerpt."
  3. "provider_specific_fields":{"refusal":null}: nothing names the store, the provider, or the error
  4. All 21 x-litellm-* headers are present and normal (x-litellm-response-cost: 0.0016920000000000001, x-litellm-attempted-retries: 0), and grep -c -i vector_store over every response header returns 0

Case 3: broken store, streaming chat completions, default settings

  1. Same command with "stream":true, piped through grep -E 'vector_store_search_failures|"error"'
  2. (no chunk names a vector store failure)

Case 4: broken store, chat completions, vector_store_search_failure_mode: error

  1. Proxy restarted with --config config_error.yaml; the proxy log confirms setting litellm.vector_store_search_failure_mode=error
  2. Same broken-store command
  3. HTTP 200, ungrounded: "content":"I don’t have access to the Project Halcyon runbook, so I can’t verify the Reykjavik shard freeze hold duration.". The setting does not exist yet, so asking to fail closed changes nothing

Case 5: broken store, /v1/responses, vector_store_search_failure_mode: error

  1. curl -s -w '\nHTTP %{http_code}\n' $PROXY/v1/responses -H "Authorization: Bearer sk-lit6809" -H 'Content-Type: application/json' -d '{"model":"gpt-5.6","vector_store_ids":["lit6750-qa-vectors:lit6809-index-never-created"],"input":"'"$Q"'"}'
  2. HTTP 200, ungrounded: "text":"I don’t have access to the Project Halcyon runbook to verify the Reykjavik shard freeze hold duration."

Case 6: broken store, /v1/responses, default settings

  1. Same command against the default-settings proxy
  2. HTTP 200, ungrounded: "text":"I don’t have access to the Project Halcyon runbook to verify the exact hold duration."
  3. grep -c vector_store_search_failures and grep -c -i vector_store on the response body both return 0: nothing in the body names the store

Case 7: healthy store, /v1/responses, default settings

  1. Same command with "vector_store_ids":["lit6750-qa-vectors:litellm-index-ac0bfb1a"]
  2. HTTP 200, grounded: The Reykjavik shard freeze must use a hold duration of exactly 900 seconds., so retrieval does run on this surface
  3. grep -c search_results on the response body returns 0: /v1/responses already carries neither the results nor, after this PR, the failures

After (c4d09a3: the PR merged onto staging df3b8a6, plus 376f74c, which rebinds the failure mode before the match so basedpyright stops counting the assert_never arm, and the tip commit itself, which swaps two [] defaults for () in a debug log so LIT002 stays under the ceiling staging ratcheted down)

Same rig, head side, two uvicorn workers and its own Postgres, litellm.__file__ asserted inside the PR worktree before boot. Every case below was re-run at this tip.

Case 1: healthy store, chat completions

  1. Same command
  2. HTTP 200, grounded: "content":"The Reykjavik shard freeze must use a hold duration of exactly 900 seconds." with search_results unchanged

Case 2: broken store, chat completions, default settings

  1. Same command
  2. HTTP 200, still ungrounded, and now the message carries the failure:
"provider_specific_fields": {
  "refusal": null,
  "vector_store_search_failures": [
    {
      "vector_store_id": "lit6750-qa-vectors:lit6809-index-never-created",
      "custom_llm_provider": "s3_vectors",
      "error": "litellm.NotFoundError: S3_vectorsException - {\"message\":\"The specified index could not be found\"}"
    }
  ]
}

Case 3: broken store, streaming chat completions, default settings

  1. Same command with "stream":true
  2. A chunk arrives carrying the same failure on the delta:
data: {"id":"chatcmpl-EKbSuLXqaPbMK1FbBwWXpsERJJpuo","created":1788579285,"model":"gpt-5.6","object":"chat.completion.chunk","choices":[{"finish_reason":"stop","index":0,"delta":{"provider_specific_fields":{"vector_store_search_failures":[{"vector_store_id":"lit6750-qa-vectors:lit6809-index-never-created","custom_llm_provider":"s3_vectors","error":"litellm.NotFoundError: S3_vectorsException - {\"message\":\"The specified index could not be found\"}"}]}}}],"service_tier":"default","obfuscation":"3c3Rg851bQj"}

Case 4: broken store, chat completions, vector_store_search_failure_mode: error

  1. Proxy restarted with --config config_error.yaml
  2. Same broken-store command
  3. HTTP 400:
{"error":{"message":"litellm.BadRequestError: The request could not be grounded in every configured vector store. 1 vector store search(es) failed: lit6750-qa-vectors:lit6809-index-never-created: litellm.NotFoundError: S3_vectorsException - {\"message\":\"The specified index could not be found\"}. Received Model Group=gpt-5.6\nAvailable Model Group Fallbacks=None","type":"invalid_request_error","param":null,"code":"400"}}
  1. Case 1 re-run against the same proxy still returns HTTP 200 with the grounded answer, so a healthy store is untouched by the knob
  2. "stream":true on the same broken-store command is also HTTP 400 with the same body, before any SSE event opens
  3. A request naming both stores (["...litellm-index-ac0bfb1a","...lit6809-index-never-created"]) is HTTP 400 too: error means every configured store must answer, which is the semantics the setting name promises
  4. The spend log for each 400 is one failure row with spend = 0 and one text-embedding-3-small row for the query embedding: the model call never happens, so nothing is billed for the answer that was not served

Case 5: broken store, /v1/responses, vector_store_search_failure_mode: error

  1. Same command
  2. HTTP 400 with the same body, so the fail-closed signal is not chat-completions-only
  3. Same for "stream":true on /v1/responses, and for a model that reaches /v1/responses through the chat-completions bridge (claude-sonnet-5, real Anthropic): HTTP 400, Received Model Group=claude-sonnet-5

Case 6: broken store, /v1/responses, default settings

  1. Same command against the default-settings proxy
  2. HTTP 200, ungrounded ("text":"I don’t have access to the Project Halcyon runbook to verify the Reykjavik shard freeze hold duration."), and the response object itself now names the failure:
"vector_store_search_failures": [
  {
    "vector_store_id": "lit6750-qa-vectors:lit6809-index-never-created",
    "custom_llm_provider": "s3_vectors",
    "error": "litellm.NotFoundError: S3_vectorsException - {\"message\":\"The specified index could not be found\"}"
  }
]
  1. Control on the same proxy, healthy store: HTTP 200, grounded ("The Reykjavik shard freeze must use a hold duration of exactly 900 seconds."), and no vector_store_search_failures key at all, so a healthy store leaves this surface byte-for-byte as it was
  2. The same broken-store request through the chat-completions bridge (claude-sonnet-5) carries the identical top-level vector_store_search_failures, non-streaming; on the stream it rides the final response.completed event under response.provider_specific_fields: [{"vector_store_id": "lit6750-qa-vectors:lit6809-index-never-created", "custom_llm_provider": "s3_vectors", "error": "litellm.NotFoundError: S3_vectorsException - {\"message\":\"The specified index could not be found\"}"}]

Case 7: broken store, chat completions, misspelled vector_store_search_failure_mode: erorr

  1. Proxy restarted with --config config_typo.yaml, whose only difference is the typo
  2. Same broken-store command
  3. HTTP 200 with the annotation on "I don’t have access to the Project Halcyon runbook, so I can’t determine the required hold duration.", so a typo degrades to the permissive default instead of failing every vector-store request:
"provider_specific_fields": {
  "refusal": null,
  "vector_store_search_failures": [
    {
      "vector_store_id": "lit6750-qa-vectors:lit6809-index-never-created",
      "custom_llm_provider": "s3_vectors",
      "error": "litellm.NotFoundError: S3_vectorsException - {\"message\":\"The specified index could not be found\"}"
    }
  ]
}
  1. The proxy log names the bad value and the supported set:
21:39:22 - LiteLLM:WARNING: vector_store_pre_call_hook.py:458 - Unsupported vector_store_search_failure_mode='erorr', falling back to 'annotate'. Supported modes: annotate, error
  1. litellm_settings has no allowlist, so the same typo reaches litellm.vector_store_search_failure_mode at the merge base too, where the setting simply does not exist and Case 2's un-augmented HTTP 200 is what comes back (re-run on the base side of the rig with the same config_typo.yaml: HTTP 200, provider_specific_fields is {"refusal": null}, and its log has no such warning)

Case 8: everything else the hook reaches, default settings

  1. Healthy store with "stream":true: 19 chunks on both sides, grounded, search_results on the delta, no failures key, so the streaming happy path is byte-for-byte unchanged
  2. Both stores on one request ("vector_store_ids":["...litellm-index-ac0bfb1a","...lit6809-index-never-created"]): HTTP 200, grounded on the healthy store (search_results: 1), and vector_store_search_failures names the broken one, so one dead store no longer hides behind a live one
  3. Key lifecycle on both sides: POST /key/generate, a broken-store chat call with that key (annotated on the head, bare on the base), GET /key/info, POST /key/delete, then the same call returns HTTP 401, so the annotation rides the request without touching auth
  4. POST /v1/messages with the broken store: HTTP 200 and no failures on either side, streaming or not, because that surface never runs the vector store hook today (see Caveats)
  5. Spend rows per successful annotated call are identical to the base: one acompletion (or aresponses) row, one asearch row, one aembedding row

Type

🐛 Bug Fix

Caveats (if any)

Low

  • Streaming /v1/responses on a native OpenAI model carries the failure only under error mode

    • Measured on the broken store at this tip with gpt-5.6: 35 SSE events, grep -c vector_store_search_failures returns 0
    • A model that reaches /v1/responses through the chat-completions bridge (claude-sonnet-5 here) does carry it, on the response.completed event under response.provider_specific_fields, because the bridge builds that event from the annotated chat response
    • litellm/responses/streaming_iterator.py yields the response.completed event to the client and only then runs the post-call hook, and what it hands the hook is the stream event rather than a ResponsesAPIResponse, so by the time anything could annotate it the caller already has it
    • Annotating it would mean holding the final event back behind a hook call on every responses stream, which is a wider change than this ticket and lands on every hook, not just this one
    • Non-streaming /v1/responses and both chat-completions modes are unaffected, and error mode raises before the stream opens, so a caller who cannot tolerate an ungrounded answer is covered on every surface
  • A background: true response carries the failures on the POST, not on the later GET

    • Verified at this tip: the synchronous POST returns "status":"queued" already carrying vector_store_search_failures, so the caller is told at request time
    • GET /v1/responses/{id} rebuilds the response from the provider's stored copy, which never held a LiteLLM annotation, so the field is absent there
    • Nothing is lost: the caller was told on the call that did the retrieval
  • /v1/responses carries the failures but still not search_results

    • That field has never been on this surface, and adding it would put whole retrieved documents into a response shape that never had them
    • Out of scope here: the ticket is about failures reaching the caller, not about widening what a successful search returns
  • The machine-readable code reaches the SDK caller but not the proxy caller

    • litellm.acompletion raises VectorStoreSearchError with e.code == "vector_store_search_failed" and the same code in e.body
    • Over HTTP the proxy wraps it in ProxyException, whose to_dict() emits only message, type, param and code, and it puts the status there, so a proxy caller sees "code": "400"
    • That is the shape of every LiteLLM proxy error, not something this PR introduces, and the method carries a # NOTE: DO NOT MODIFY THIS in litellm/proxy/_types.py, so changing it would touch every error the proxy returns
    • Proxy callers key off the 400 plus the message, which names every failed vector_store_id
  • The error message repeats the provider's own text back to the caller

    • By design: the caller has to know why grounding failed, and the same authenticated caller already receives the store's full document text through search_results on a healthy call
    • Redacting it would leave a 400 that says a store failed without saying which or why, which is the state this PR exists to fix
  • /v1/messages ignores vector_store_ids entirely today

    • A healthy store returns an ungrounded answer there, before and after this PR
    • Out of scope here; worth its own ticket
  • Under error mode, router fallbacks retry the search before giving up

    • Measured at this tip with fallbacks: [{gpt-5.6: [gpt-5.6-fallback]}]: the 400 still comes back, its message carries the fallback chain (Available Model Group Fallbacks=['gpt-5.6-fallback'], then Error doing the fallback: with the same text), and the spend log shows two text-embedding-3-small rows per request instead of one
    • That is the router treating the 400 like any other pre-call failure, which is what every raise from this hook has always done; the fallback deployment cannot help when the store itself is broken, so the extra call is one query embedding, never a model call
    • Teaching the router to skip fallbacks for this one exception is a router change, not a vector store one, so it stays out of this PR
  • One red check at this tip is a fleet-wide staging failure, tracked elsewhere

  • The new setting is not in the docs site yet

    • Docs live in a separate repo, so it needs a follow-up docs PR

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
  • c4d09a3 passes /live-pr-risk

Note

Medium Risk
Changes pre-call hook behavior and response shape for all vector-store-grounded requests; opt-in error mode can turn previously successful calls into HTTP 400s, but default annotate is additive only.

Overview
When a configured vector store search fails, callers no longer get a silent HTTP 200 with no signal that grounding was skipped.

The pre-call hook now records per-store failures (vector_store_id, provider, error text) alongside successful search_results. By default (vector_store_search_failure_mode: annotate), behavior stays permissive: the LLM still runs, but failures appear as vector_store_search_failures on chat completion provider_specific_fields (and streaming deltas) and as a top-level field on /v1/responses. Setting vector_store_search_failure_mode: error in litellm_settings raises the new VectorStoreSearchError (HTTP 400, invalid_request_error) so the request fails closed when any configured store cannot be searched.

Invalid failure-mode values fall back to annotate with a warning. Hook error logging now includes the requested vector_store_ids. Context injection is refactored to aggregate outcomes across multiple stores without dropping failed ones silently.

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

A vector store search that fails is swallowed by the pre-call hook, so the
request goes to the model with an un-augmented prompt and the caller gets a
200 answering from the model's own knowledge with no way to tell the
knowledge base was skipped.

Failed searches now ride the same channel their successes already use: a
vector_store_search_failures entry on provider_specific_fields naming the
store id, provider, and error. That is additive and always on. For callers
who would rather fail than answer ungrounded, litellm_settings
vector_store_search_failure_mode: error raises VectorStoreSearchError (400)
instead; the default stays annotate, today's permissive behavior.

The hook's outer catch-all also now names the requested vector store ids in
its log line, and only wraps the augmentation itself, so the fail-closed
raise is not swallowed by it.
@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes vector-store retrieval failures visible to API callers while preserving permissive default behavior.

  • Records structured per-store failures alongside successful retrieval results.
  • Annotates chat-completion and Responses API responses in the default mode.
  • Adds an opt-in error mode that fails closed when any configured vector-store search fails.
  • Falls back safely to annotation mode for unsupported configuration values.
  • Adds regression coverage for successful, failed, mixed-store, streaming, and Responses API paths.

Confidence Score: 5/5

The PR appears safe to merge; no new actionable issues or outstanding previous findings remain.

The invalid-mode failure was corrected by validating configuration and falling back to annotation, the request-state concern was withdrawn after confirming the established metadata channel, and the remaining typing thread was manually resolved. No changes were made after the previous review, and the current diff has no confirmed rule violations or merge-blocking behavior.

Important Files Changed

Filename Overview
litellm/integrations/vector_store_integrations/vector_store_pre_call_hook.py Aggregates search outcomes, propagates failure metadata, supports fail-closed behavior, and safely handles invalid failure-mode configuration.
litellm/exceptions.py Adds a typed bad-request exception containing structured vector-store failures and an SDK-facing error code.
litellm/types/vector_stores.py Defines the supported failure modes and immutable structured failure metadata.
litellm/init.py Exports the new exception and establishes annotation as the backward-compatible default mode.
tests/test_litellm/integrations/vector_store_integrations/test_vector_store_pre_call_hook.py Adds focused mocked regression tests without weakening existing coverage or introducing network dependencies.

Reviews (6): Last reviewed commit: "fix(vector_stores): default the search-c..." | Re-trigger Greptile

Comment thread litellm/integrations/vector_store_integrations/vector_store_pre_call_hook.py Outdated
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.73684% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...r_store_integrations/vector_store_pre_call_hook.py 94.01% 7 Missing ⚠️

📢 Thoughts on this report? Let us know!

verbose_logger.exception("Error in VectorStorePreCallHook: %s", e)
# Return original parameters on error
return model, messages, non_default_params
verbose_logger.debug(

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.

Low: Backend exception details exposed to callers

str(search_error) is returned in normal and streaming completion responses and included in the opt-in 400 message. An authenticated caller can use failing searches to disclose details from provider SDKs and direct clients, such as internal Valkey/Milvus addresses or backend error text; log the original exception server-side, but expose a stable sanitized message or error code here.

Suggested change
verbose_logger.debug(
error="Vector store search failed",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Provider error text already reaches callers via the direct vector store search endpoint, and litellm has no redaction knob. Hiding it defeats the ticket: nobody could tell which store broke.

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.

Thanks for the detail. I can't automatically confirm this is safe to dismiss, so I'm leaving the thread open for a maintainer to make the call.

@veria-ai

veria-ai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

PR overview

This pull request changes vector-store retrieval error handling so search failures are surfaced to API callers in both normal and streaming completion responses.

One low-impact information-disclosure issue remains open: authenticated callers who trigger failed searches may receive raw backend exception details, potentially revealing internal service addresses or provider error text. No issues have yet been addressed, so caller-facing errors should be sanitized while full details remain in server-side logs.

Open issues (1)

Fixed/addressed: 0 · PR risk: 4/10

…_injection' into litellm_vector_store_surface_retrieval_failure
…recognized

litellm_settings keys are set on the litellm module with no allowlist, so a typo
in vector_store_search_failure_mode reached assert_never and turned every
vector-store request into a 500. Validate the configured value and fall back to
the permissive default with a warning naming the supported modes.
@mateo-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

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

Copy link
Copy Markdown
Contributor Author

@greptileai

@mateo-berri

Copy link
Copy Markdown
Contributor Author

bugbot run

@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.

Stale Bugbot comment from a previous run.

Base automatically changed from litellm_vector_store_hook_router_injection to litellm_internal_staging September 3, 2026 21:36
basedpyright counts a named capture after an exhaustive match under
reportUnnecessaryComparison, which pushed the merged tree one over the
budget; the wildcard case with assert_never on the bound subject is the
shape the rest of the codebase uses
…itellm_vector_store_surface_retrieval_failure
@mateo-berri
mateo-berri requested a review from a team September 5, 2026 00:28
@mateo-berri mateo-berri added run-ci and removed run-ci labels Sep 5, 2026
@codspeed

codspeed Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_vector_store_surface_retrieval_failure (c4d09a3) with litellm_internal_staging (df3b8a6)

Open in CodSpeed

@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.

Stale Bugbot comment from a previous run.

@mateo-berri mateo-berri added run-ci and removed run-ci labels Sep 5, 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 c4d09a3. Configure here.

@mateo-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

@tin-berri tin-berri 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.

LGTM

@mateo-berri
mateo-berri merged commit 5c82182 into litellm_internal_staging Sep 11, 2026
229 of 233 checks passed
@mateo-berri
mateo-berri deleted the litellm_vector_store_surface_retrieval_failure branch September 11, 2026 18:46
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