Skip to content

fix(embedding): respect drop_params for unsupported dimensions parameter - #26868

Merged
mateo-berri merged 2 commits into
BerriAI:litellm_oss_stagingfrom
xr843:fix/embedding-dimensions-drop-params
Jun 2, 2026
Merged

fix(embedding): respect drop_params for unsupported dimensions parameter#26868
mateo-berri merged 2 commits into
BerriAI:litellm_oss_stagingfrom
xr843:fix/embedding-dimensions-drop-params

Conversation

@xr843

@xr843 xr843 commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #26787. The OpenAI-provider branch in get_optional_params_embeddings hard-raised UnsupportedParamsError whenever dimensions was passed to a non-text-embedding-3 model — even though the error message itself instructed users to set litellm.drop_params=True. The flag (per-call and global) had no effect, so the documented escape hatch was a dead end.

Root cause

litellm/utils.py:3304-3317 short-circuited with a raise before honoring drop_params. The same function's _check_valid_arg (lines 3252-3268) already respects litellm.drop_params is True or drop_params is True, but this OpenAI-specific check bypassed that path entirely.

Fix

Before raising, check litellm.drop_params is True or drop_params is True. If true, pop dimensions from non_default_params and continue; otherwise preserve the original raise (default behavior unchanged). Then assign optional_params = non_default_params unconditionally — when dropped, dimensions is no longer in the dict, so it doesn't leak to the OpenAI request; when supported (text-embedding-3 or in allowed_openai_params), it passes through normally.

Changes

  • litellm/utils.py (lines 3304-3324): structural rewrite of the dimensions check
  • tests/local_testing/test_get_optional_params_embeddings.py:
    • test_openai_non_text_embedding_3_with_per_call_drop_params — per-call drop_params=True succeeds and emits no dimensions
    • test_openai_non_text_embedding_3_with_global_drop_paramslitellm.drop_params=True succeeds likewise
    • Added try/finally state isolation to the existing raise test (another test in the file mutates litellm.drop_params without restoring, which would otherwise contaminate this test under randomized order)

Test plan

  • pytest tests/local_testing/test_get_optional_params_embeddings.py -v → 7/7 pass
  • Verified the new tests fail without the fix (they catch the real bug)
  • Default behavior preserved: drop_params=False (default) still raises
  • text-embedding-3 and allowed_openai_params paths unchanged

Notes

  • drop_params precedence (per-call vs global) follows the existing _check_valid_arg pattern: either being True wins. No new policy introduced.
  • PR fix(embeddings): drop dimensions for openai_compatible_providers when drop_params=True #23120 (referenced in the issue thread) addresses a different branch in the same function (provider_config.map_openai_params path); orthogonal to this fix.
  • Other providers (Cohere, Voyage, Bedrock) flow through BaseEmbeddingConfig.map_openai_params and already honor drop_params per provider config.

The OpenAI-provider branch in `get_optional_params_embeddings` hard-raised
`UnsupportedParamsError` whenever `dimensions` was passed to a non
`text-embedding-3` model, even though the error message itself instructed
users to set `litellm.drop_params=True`. The flag (per-call and global)
was never consulted on this path, breaking the documented escape hatch
for users proxying to vLLM/TEI/Ollama-compat embedding servers via the
`openai/...` model prefix.

Now mirror the `drop_params` handling already used by `_check_valid_arg`
in the same function: when either `drop_params=True` (per-call) or
`litellm.drop_params=True` (global) is set, silently strip `dimensions`
from `non_default_params` and continue; otherwise preserve the existing
error to keep current behavior for users who have not opted in.

Adds two regression tests (per-call and global flag) and pins the
existing raise-by-default behavior against accidental future drift.

Fixes BerriAI#26787

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes the drop_params flag being silently ignored when dimensions was passed to a non-text-embedding-3 OpenAI embedding model. The documented escape hatch (litellm.drop_params = True) had no effect because the early raise in utils.py bypassed the flag entirely.

  • litellm/utils.py: Wraps the UnsupportedParamsError raise inside an inner if/else that first checks litellm.drop_params is True or drop_params is True; if true, pops dimensions and continues. The optional_params = non_default_params assignment is moved outside the outer condition so it always runs (matching the original else: path).
  • tests/local_testing/test_get_optional_params_embeddings.py: Adds two focused regression tests (per-call and global drop_params) and hardens the existing raise test with try/finally state isolation to prevent cross-test contamination.

Confidence Score: 5/5

Safe to merge — the change is a small, well-scoped guard added before an existing raise, default behaviour is unchanged, and new regression tests confirm both the fix and the fallback.

The fix is minimal and surgical: a two-branch check wraps one existing raise, and the unconditional assignment of optional_params = non_default_params correctly replaces the old else: path. No other code paths in the function are touched. The new tests are properly isolated with try/finally. Two pre-existing tests still leak global state but this is not introduced by the PR and does not affect correctness here.

No files require special attention.

Important Files Changed

Filename Overview
litellm/utils.py Structural rewrite of the dimensions guard in the OpenAI branch of get_optional_params_embeddings: checks drop_params before raising, then unconditionally assigns optional_params = non_default_params.
tests/local_testing/test_get_optional_params_embeddings.py Adds two new regression tests for per-call and global drop_params, and adds try/finally state isolation to the existing raise test. Pre-existing tests (test_vertex_projects, test_bedrock_embed_v2_with_drop_params) still leak litellm.drop_params = True without restoring.

Reviews (2): Last reviewed commit: "fixup: simplify drop_params guard per gr..." | Re-trigger Greptile

Comment thread litellm/utils.py Outdated
@codecov

codecov Bot commented Apr 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed-hq

codspeed-hq Bot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing xr843:fix/embedding-dimensions-drop-params (d6af1ea) with main (934ecdc)

Open in CodSpeed

Drop the redundant `drop_params is not None and` prefix —
`drop_params is True` already implies non-None. Behavior unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xr843

xr843 commented May 3, 2026

Copy link
Copy Markdown
Contributor Author

Pushed d6af1eaa5b to address greptile P2: dropped the redundant drop_params is not None and prefix per the suggested edit. Behavior unchanged — drop_params is True already implies non-None. Existing tests still pass.

@xr843
xr843 changed the base branch from main to litellm_oss_staging May 5, 2026 06:05
@mateo-berri
mateo-berri deleted the branch BerriAI:litellm_oss_staging May 18, 2026 23:27
@xr843

xr843 commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

I noticed this was closed unmerged. Was this superseded by another change, or would you prefer a narrower follow-up PR against the current staging branch?

The original failure mode was the unsupported dimensions parameter when drop_params=True for embeddings. I can rework or reopen if this is still wanted.

@mateo-berri

Copy link
Copy Markdown
Contributor

Hi @xr843. Sorry to close your branch. This was done automatically because litellm_oss_staging was auto-deleted after being merged. We've since disabled branch auto-deletion. Reopening

@mateo-berri mateo-berri reopened this May 21, 2026
@mateo-berri

mateo-berri commented May 21, 2026

Copy link
Copy Markdown
Contributor

Can you add some screenshot or video proof of this working? I would like to see a before and after this PR please. Thank you for your contributions

@xr843

xr843 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Hi @mateo-berri — thanks for re-opening, and here's the before/after proof you asked for.

Since get_optional_params_embeddings() is pure param-mapping logic, this reproduces without any API key or network call — it directly shows whether dimensions is dropped or leaks into the outgoing OpenAI request. Same script run against the base commit vs. this PR's litellm/utils.py:

repro script (no API key needed)
import litellm
from litellm import get_llm_provider
from litellm.utils import get_optional_params_embeddings
from litellm.exceptions import UnsupportedParamsError

model, provider, _, _ = get_llm_provider(model="openai/Qwen/Qwen3-Embedding-0.6B")  # OpenAI-provider, NOT text-embedding-3

def run(label, *, drop_call=None, drop_global=False):
    litellm.drop_params = drop_global
    kw = dict(model=model, dimensions=1024, custom_llm_provider=provider)
    if drop_call is not None:
        kw["drop_params"] = drop_call
    try:
        out = get_optional_params_embeddings(**kw)
        print(f"{label} -> {out}   dimensions leaked? {'dimensions' in out}")
    except UnsupportedParamsError:
        print(f"{label} -> RAISED UnsupportedParamsError")
    finally:
        litellm.drop_params = False

run("A. default drop_params=False")
run("B. per-call drop_params=True", drop_call=True)
run("C. global litellm.drop_params=True", drop_global=True)

❌ BEFORE (base litellm/utils.py, no fix)

A. default drop_params=False        -> RAISED UnsupportedParamsError
B. per-call drop_params=True        -> RAISED UnsupportedParamsError   # bug: drop_params ignored
C. global litellm.drop_params=True  -> RAISED UnsupportedParamsError   # bug: documented escape hatch is a dead end

✅ AFTER (this PR)

A. default drop_params=False        -> RAISED UnsupportedParamsError   # default behavior preserved
B. per-call drop_params=True        -> {}   dimensions leaked? False   # stripped
C. global litellm.drop_params=True  -> {}   dimensions leaked? False   # stripped

Summary

scenario BEFORE AFTER
default (drop_params=False) raises raises (unchanged)
per-call drop_params=True raises (bug) dimensions stripped
global litellm.drop_params=True raises (bug) dimensions stripped

Case A confirms default behavior is untouched; B and C show the documented drop_params=True escape hatch now actually works instead of dead-ending in a raise. The two new regression tests in tests/local_testing/test_get_optional_params_embeddings.py cover exactly B and C (and fail without the fix). Happy to attach a terminal screenshot too if you'd prefer, but this is fully reproducible from the snippet above. Thanks!

@mateo-berri

Copy link
Copy Markdown
Contributor

LGTM; thanks!

@mateo-berri
mateo-berri merged commit 9119403 into BerriAI:litellm_oss_staging Jun 2, 2026
89 of 92 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants