Skip to content

fix: restore commits from revert of PR #21601 - #23457

Merged
Chesars merged 2 commits into
BerriAI:litellm_oss_staging_03_11_2026from
Chesars:revert-revert-21601-model-cost-aliases
Mar 12, 2026
Merged

fix: restore commits from revert of PR #21601#23457
Chesars merged 2 commits into
BerriAI:litellm_oss_staging_03_11_2026from
Chesars:revert-revert-21601-model-cost-aliases

Conversation

@Chesars

@Chesars Chesars commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Summary

PR #23313 reverted all ~50 commits from PR #21601 to clean up the staging diff. However, only the model_cost aliases feature was re-applied (#23314). The remaining fixes from other contributors were lost when staging was merged to main (#23276).

This PR reverts the revert (#23313) to restore all lost commits, including:

Conflicts with #23314 (aliases) and #23151 (gpt_5_transformation) were resolved keeping the current (newer) versions.

Test plan

@vercel

vercel Bot commented Mar 12, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Mar 12, 2026 4:44pm

Request Review

Keep both sets of tests: upstream's OAuth2 token injection test and
our case-insensitive tool matching tests. Use upstream's version of
the bedrock output_config test (more comprehensive).
@Chesars
Chesars merged commit 33457ab into BerriAI:litellm_oss_staging_03_11_2026 Mar 12, 2026
4 of 5 checks passed
@Chesars
Chesars deleted the revert-revert-21601-model-cost-aliases branch March 12, 2026 16:43
@greptile-apps

greptile-apps Bot commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR restores ~50 commits that were lost when PR #23313 reverted PR #21601 to clean a staging diff — the aliases feature (PR #23314) and GPT-5 transformation (PR #23151) that had already been re-applied are intentionally excluded here. The changes land a broad set of independent bug fixes and small features across the caching, routing, provider transformation, proxy, and UI layers.

Key changes:

  • DualCache async TTLdefault_in_memory_ttl was not propagated to the async set_cache / set_cache_pipeline methods; now fixed.
  • Router retry loop — Non-retryable errors (e.g. 400 ContextWindowExceeded) surfaced during a retry attempt are now raised immediately instead of being swallowed and replaced by the original error; original_exception is also updated on every iteration so the last error is always re-raised when retries are exhausted.
  • Lowest-latency strategy — Average latency was always divided by len(item_latency) even in streaming mode, where item_ttft_latency should be used; this could produce a ZeroDivisionError or wrong averages.
  • Duration parser — Month overflow (+2m from November, etc.) was only handled for December; now uses modular arithmetic for full correctness.
  • Bedrock converse — Adds completion_tokens_details (with estimated reasoning_tokens) to usage output.
  • Fireworks AI — Removes duplicate /v1 prefix when api_base already contains it.
  • Sagemaker embeddings — Uses _load_credentials (supporting aws_role_name / aws_session_name) instead of manual boto3 client construction.
  • _redact_standard_logging_object — New helper to redact the standard_logging_object field in model_call_details is added but never called from perform_redaction, leaving the redaction incomplete and causing the updated e2e test assertions to fail.
  • Credential endpoints — Splits a previously-broken single handler (mismatched Path parameter) into two clean routes: /credentials/by_name/{credential_name:path} and /credentials/by_model/{model_id}.
  • MCP tool filtering — Case-insensitive comparison for OpenAPI operationId camelCase names.
  • VirtualKeysTable — Adds a manual "Fetch / Fetching" refresh button; stale data remains visible during background refetches.

Confidence Score: 3/5

  • Mostly safe to merge, but the redact-messages helper is dead code and will cause the updated e2e assertions to fail.
  • The vast majority of fixes are correct and well-tested with mocked unit tests. The one concrete logic gap is that _redact_standard_logging_object is defined but never wired into perform_redaction, meaning standard_logging_object content is not redacted when turn_off_message_logging is enabled, and the matching e2e test assertions were updated to expect the new behaviour that is not yet implemented. This is a functional regression for users relying on message-logging redaction.
  • litellm/litellm_core_utils/redact_messages.py_redact_standard_logging_object must be called from perform_redaction to complete the redaction fix.

Important Files Changed

Filename Overview
litellm/litellm_core_utils/redact_messages.py Adds _redact_standard_logging_object helper to redact the standard_logging_object field, but the function is never called from perform_redaction — the new redaction logic is dead code and the updated e2e test assertions will fail.
litellm/caching/dual_cache.py Correctly propagates default_in_memory_ttl to the async set_cache and set_cache_pipeline methods, which previously only applied it to the sync counterparts.
litellm/litellm_core_utils/duration_parser.py Correctly fixes month-overflow for multi-month duration strings (e.g. +2m in November), replacing the December-only special case with proper modular arithmetic.
litellm/router.py Adds non-retryable-error early-exit check inside the async retry loop and tracks the latest exception as original_exception. Logic is sound but uses a bare except Exception that could mask unexpected internal errors.
litellm/router_strategy/lowest_latency.py Fixes a latency calculation bug: the average is now computed from item_ttft_latency when streaming (rather than always dividing by len(item_latency)), preventing a potential ZeroDivisionError and incorrect averages.
litellm/llms/bedrock/chat/converse_transformation.py Adds completion_tokens_details (with reasoning_tokens estimated via token_counter) to the Bedrock usage response, improving parity with the OpenAI response schema.
litellm/llms/fireworks_ai/chat/transformation.py Strips a trailing /v1 from api_base before appending /v1/accounts/{account_id}/models, preventing a duplicate /v1/v1/… URL when the base already contains the version prefix.
litellm/llms/sagemaker/completion/handler.py Replaces the manual boto3 client creation in the embedding path with _load_credentials + boto3.Session, enabling IAM role assumption (aws_role_name / aws_session_name) to match the completion handler's behaviour.
litellm/proxy/credential_endpoints/endpoints.py Splits the previously-broken single get_credential handler (which had a credential_name Path parameter mismatched against the /by_model/{model_id} route) into two clean, correctly-routed handlers.
litellm/responses/litellm_completion_transformation/transformation.py Adds convert_apply_patch_tool_call_to_chat_completion_tool_call for ResponseApplyPatchToolCall serialisation, and merges consecutive assistant messages to satisfy Anthropic's "tool_use must precede tool_result" requirement.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[async_function_with_retries] --> B[Initial attempt: make_call]
    B -->|success| C[Return response]
    B -->|exception e| D[original_exception = e]
    D --> E[log_retry / get healthy deployments]
    E --> F{_retry_policy_applies?}
    F -->|Yes| I[Sleep, increment attempt]
    F -->|No| G[should_retry_this_error]
    G -->|returns normally - retryable| I
    G -->|raises - non-retryable| H[raise e immediately]
    I --> J{attempts < num_retries?}
    J -->|Yes| B
    J -->|No| K[raise original_exception\nlast error seen]
Loading

Last reviewed commit: 4e6e1d8

Comment on lines +78 to 126
def _redact_standard_logging_object(model_call_details: dict):
"""Redact messages and response inside standard_logging_object if present."""
standard_logging_object = model_call_details.get("standard_logging_object")
if standard_logging_object is None:
return

redacted_str = "redacted-by-litellm"

if standard_logging_object.get("messages") is not None:
standard_logging_object["messages"] = [
{"role": "user", "content": redacted_str}
]

response = standard_logging_object.get("response")
if response is not None:
if isinstance(response, dict) and "output" in response:
# ResponsesAPIResponse format - redact content in output items
if isinstance(response.get("output"), list):
for output_item in response["output"]:
if isinstance(output_item, dict) and "content" in output_item:
if isinstance(output_item["content"], list):
for content_item in output_item["content"]:
if (
isinstance(content_item, dict)
and "text" in content_item
):
content_item["text"] = redacted_str
elif isinstance(response, dict) and "choices" in response:
# ModelResponse dict format - redact content in choices
if isinstance(response.get("choices"), list):
for choice in response["choices"]:
if isinstance(choice, dict):
if "message" in choice and isinstance(choice["message"], dict):
choice["message"]["content"] = redacted_str
if "audio" in choice["message"]:
choice["message"]["audio"] = None
elif "delta" in choice and isinstance(choice["delta"], dict):
choice["delta"]["content"] = redacted_str
if "audio" in choice["delta"]:
choice["delta"]["audio"] = None
elif isinstance(response, str):
standard_logging_object["response"] = redacted_str
else:
# For other formats (empty dict, None, etc.), use simple text format
standard_logging_object["response"] = {"text": redacted_str}


def perform_redaction(model_call_details: dict, result):
"""

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.

_redact_standard_logging_object is defined but never called

The helper function _redact_standard_logging_object is added to handle redacting the standard_logging_object inside model_call_details, but it is never invoked anywhere in the codebase (confirmed via global grep).

As a result, model_call_details["standard_logging_object"]["response"] and model_call_details["standard_logging_object"]["messages"] are not redacted when turn_off_message_logging is enabled, even though the e2e tests in tests/logging_callback_tests/test_logging_redaction_e2e_test.py were updated to assert the new finer-grained redaction behavior (e.g. response["choices"][0]["message"]["content"] == "redacted-by-litellm").

The function needs to be called from perform_redaction:

def perform_redaction(model_call_details: dict, result):
    # Redact model_call_details
    model_call_details["messages"] = [
        {"role": "user", "content": "redacted-by-litellm"}
    ]
    model_call_details["prompt"] = ""
    model_call_details["input"] = ""

    # Redact standard_logging_object if present
    _redact_standard_logging_object(model_call_details)
    ...

Without this call, the updated e2e test assertions will fail and users who rely on turn_off_message_logging will continue to have unredacted content leaked via the standard_logging_object.

Comment thread litellm/router.py
Comment on lines +5585 to 5601
# continuing the retry loop. Respect retry policy
# precedence - only check when no retry policy applies.
if not _retry_policy_applies:
try:
self.should_retry_this_error(
error=e,
healthy_deployments=_healthy_deployments,
all_deployments=_all_deployments,
context_window_fallbacks=context_window_fallbacks,
regular_fallbacks=fallbacks,
content_policy_fallbacks=content_policy_fallbacks,
)
except Exception:
raise e

_timeout = self._time_to_sleep_before_retry(
e=e,

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.

Bare except Exception silently masks unexpected errors from should_retry_this_error

The pattern used to detect non-retryable errors is:

try:
    self.should_retry_this_error(error=e, ...)
except Exception:
    raise e

should_retry_this_error always raises error (i.e., e) itself when the error is non-retryable, so except Exception: raise e is equivalent to except Exception: raise. However, if should_retry_this_error ever raises an unexpected internal exception (e.g., a TypeError from a None comparison or a missing attribute), that bug will be silently swallowed and e will be raised instead, making debugging very difficult.

Consider catching only the specific LiteLLM exception types that should_retry_this_error intentionally raises:

try:
    self.should_retry_this_error(error=e, ...)
except litellm.LITELLM_EXCEPTION_TYPES:  # or tuple of specific types
    raise e

Or simpler, since the function always re-raises the same error object:

try:
    self.should_retry_this_error(error=e, ...)
except Exception:
    raise  # re-raise the actual exception from should_retry_this_error

@Chesars Chesars changed the title fix: restore commits lost by revert of PR #21601 fix: restore commits from revert of PR #21601 Mar 12, 2026
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
…el-cost-aliases

fix: restore commits lost by revert of PR BerriAI#21601
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