Skip to content

Revert "Get optional params behavior" - #23234

Merged
Sameerlite merged 1 commit into
mainfrom
revert-23224-cursor/get-optional-params-behavior-ed4a
Mar 10, 2026
Merged

Revert "Get optional params behavior"#23234
Sameerlite merged 1 commit into
mainfrom
revert-23224-cursor/get-optional-params-behavior-ed4a

Conversation

@Sameerlite

Copy link
Copy Markdown
Contributor

Reverts #23224

@vercel

vercel Bot commented Mar 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Building Building Preview, Comment Mar 10, 2026 4:52am

Request Review

@Sameerlite
Sameerlite merged commit 09919a8 into main Mar 10, 2026
33 of 36 checks passed
@greptile-apps

greptile-apps Bot commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR reverts #23224, which had fixed a known bug where optional parameters like store, metadata, context_management, prompt_cache_key, and prompt_cache_retention were silently dropped when passed as kwargs to completion(). The revert removes the safety-net loop that forwarded these parameters, re-introducing the exact bug reported in #23087. Additionally, the PR deletes test_store_param.py, removing all regression tests for this forwarding behavior.

Key Concerns:

  • The store parameter and others listed in DEFAULT_CHAT_COMPLETION_PARAM_VALUES will again be silently dropped for callers passing them as kwargs, because get_non_default_completion_params() excludes them and they are not named parameters
  • Deleting regression tests removes detection mechanisms for this bug
  • No explanation or alternative fix is provided in the PR description

Confidence Score: 1/5

  • Unsafe to merge — re-introduces a known silent parameter-dropping bug with no alternative fix provided.
  • This PR reverts a targeted fix for a confirmed bug (issue [Bug]: store=true not forwarded for OpenAI gpt models 5.1/5.2 #23087) without providing a replacement solution. The removed code was small and surgical; reverting it restores broken behavior where callers passing store=True, metadata, etc. will have these values silently ignored. Compounding the risk, regression tests are deleted, eliminating detection mechanisms. The PR description provides no justification or follow-up plan.
  • litellm/main.py (safety-net loop removed) and deleted test file (test_store_param.py).

Last reviewed commit: 504e66c

Comment thread litellm/main.py
Comment on lines 1490 to 1492
optional_params = get_optional_params(
**optional_param_args, **non_default_params
)

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.

This revert re-introduces the silent parameter-dropping bug originally reported in #23087.

The removed safety-net loop forwarded kwargs present in DEFAULT_CHAT_COMPLETION_PARAM_VALUES (such as store, metadata, context_management, prompt_cache_key, prompt_cache_retention) into optional_param_args before calling get_optional_params().

Without it, these parameters are silently dropped because:

  1. They are not named parameters of completion(), so they arrive only via **kwargs
  2. get_non_default_completion_params() explicitly excludes any key in DEFAULT_CHAT_COMPLETION_PARAM_VALUES from non_default_params
  3. optional_param_args only includes explicitly named parameters
  4. optional_params (which is passed to the provider) is built from only optional_param_args and non_default_params

Result: callers passing store=True, metadata={...}, etc. will have their values silently ignored, regardless of the provider.

@ishaan-berri
ishaan-berri deleted the revert-23224-cursor/get-optional-params-behavior-ed4a branch March 26, 2026 22:30
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
…-optional-params-behavior-ed4a

Revert "Get optional params behavior"
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.

1 participant