fix(llm): drop local/* in-process embedding models from generation fallbacks - #332
Conversation
…llbacks
A local/* in-process embedding model (e.g. local/nomic-embed-text-v1.5) has no
litellm completion route — it is served in-process by _litellm_embedding.py. If
one lands in a text-generation fallback list, litellm raises BadRequestError("LLM
Provider NOT provided") deep inside its fallback ladder (observed in prod as
Sentry PYTHON-FASTAPI-CV, ~1290 events). _build_completion_params now filters
local/* entries alongside the existing primary-model dedup, so a stray embedding
model is skipped instead of crashing the call.
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe completion fallback builder now excludes ChangesCompletion fallback filtering
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…back filter The blanket local/* drop in _build_completion_params is only safe because the local provider has no generation model (_PROVIDER_DEFAULTS["local"].generation is None). Assert that contract so a future local generation model fails CI and forces the fallback filter to be revisited instead of silently dropping a legitimate local generation fallback.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
Filters
local/*in-process embedding models out of text-generation fallback lists so they can never be handed to litellm's completion fallback ladder.Problem
local/*models (e.g.local/nomic-embed-text-v1.5) are in-process ONNX embedding models —_litellm_embedding.pyroutes them toNomicEmbedder/LocalEmbedderand they have no litellm completion route. If such a model ends up in a text-generationfallback_modelslist,_build_completion_paramspasses it straight into litellm'sfallbacks, and when the primary model fails, litellm tries the embedding model viaasync_completion_with_fallbacks→get_llm_provider→BadRequestError: LLM Provider NOT provided. You passed model=local/nomic-embed-text-v1.5.This surfaced in production as Sentry PYTHON-FASTAPI-CV (~1,290 events). The existing filter at
_build_completion_paramsonly deduped the primary model (m != actual_model); there was no guard against non-generation model names.Fix
_build_completion_paramsnow dropslocal/*entries alongside the existing primary-model dedup:local/is the sole in-process embedding prefix (embedding_service.py), and no generation model uses it, so this is safe and targeted — a stray embedding model in the fallback list is now skipped instead of crashing the call.Test
Added
test_local_embedding_model_is_dropped_from_fallbacks: alocal/nomic-embed-text-v1.5+gpt-5-minifallback list resolves to["gpt-5-mini"]. All existing fallback tests still pass (10 passed).Summary by CodeRabbit
Bug Fixes
local/*embedding models from the fallback ladder, avoiding provider routing errors when mixed with valid generation models.Tests
local/*models are filtered out while valid fallbacks are preserved.localprovider is embedding-only and excluded from generation-capable provider defaults.