fix(litellm): use underlying provider for metadata, drop eager serialization#594
Merged
Abhijeet Prasad (AbhiPrasad) merged 3 commits intoJul 17, 2026
Conversation
…ization Align the LiteLLM integration with .agents/skills/sdk-integrations/SKILL.md. Audit against the SKILL surfaced four issues: - **`metadata.provider` was the framework name.** SKILL §Completion-style explicitly says LiteLLM must set `metadata.provider` to the underlying provider (openai / cohere / openrouter / ...), not "litellm". Fixed by routing every span through `_resolve_provider(model)` which calls `litellm.get_llm_provider(model)`; falls back to `"litellm"` only if the lookup fails. `@lru_cache(maxsize=256)` on the resolver so the hot path doesn't re-dispatch through litellm's routing table per call. - **Excess serialization.** Every non-streaming wrapper called `_try_to_dict(response)` on the full pydantic response then indexed into it — but `bt_json` already serializes at log time. Replaced with direct attribute reads via a shared `_get_field(obj, key, default)` helper that works on both dicts and SDK objects (openai / anthropic / cohere had all been open-coding this). Streaming paths left intact. - **Dead code.** `_is_litellm_patched` was only used by crewai until #587 moved crewai to the "wrapper never owns tokens" pattern; no remaining callers anywhere. Removed the helper and its two subprocess tests. - **Excessive comments.** Trimmed the huge `wrap_litellm` / `_is_litellm_patched` docstrings and the "wrapt wrapper for litellm.X" one-liner on every wrapper. Also lifted the rerank output cap (`results[:100]`) into `integrations/utils.RERANK_OUTPUT_MAX_RESULTS` and documented the "rerank exception to no-silent-caps" rule in the SKILL — cohere carries the same magic number and can adopt the shared constant in a follow-up. ## Test plan - [x] Updated existing VCR-backed tests to assert the underlying provider (`openai`, `cohere`, `openrouter`) instead of `"litellm"`. No cassette re-recording (HTTP requests unchanged; span-shape assertions only). - [x] Added positive `metadata["provider"] == "openrouter"` assertion to the existing OpenRouter cassette test as coverage for the new resolver. - [x] Dropped the two `_is_litellm_patched` subprocess tests alongside the helper. - [x] **No new mocks/fakes, no cassette re-recording.** - [x] `ruff check` + `ruff format --check` on touched files — clean. - [ ] Reviewer to run: `cd py && nox -s "test_litellm(latest)"` and `cd py && nox -s "test_litellm(1.74.0)"` (author's box has a broken Python 3.14 install). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
starfolkai
Bot
force-pushed
the
fix/litellm-provider-metadata-drop-eager-serialization
branch
from
July 17, 2026 20:40
1bace7d to
ba72da4
Compare
Follow-up to the provider-metadata change. CI surfaced two categories: - Three ``auto_test_scripts/`` subprocess scripts (test_auto_litellm, test_patch_litellm_responses, test_patch_litellm_aresponses) still hardcoded ``provider == "litellm"``. All three call ``gpt-4o-mini`` → expect ``"openai"``. - ``test_litellm_text_completion_metrics`` / ``test_litellm_atext_completion_metrics`` use ``gpt-3.5-turbo-instruct``, for which ``litellm.get_llm_provider`` returns ``"text-completion-openai"`` (its legacy text-completion endpoint category, distinct from chat completions). Assertion updated to that string — it IS the provider identifier litellm routes to. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
``litellm.get_llm_provider`` reports endpoint-category identifiers for legacy ``/v1/completions`` routes (``text-completion-openai``, ``azure_text``, ``text-completion-codestral``, ``text-completion-inception``). The SKILL says ``metadata.provider`` names whose pricing applies — same billing entity as the chat variant — so filtering by ``openai`` in the UI should include both chat and text-completion calls. Added a small ``_PROVIDER_ALIASES`` table that folds those routing keys back to the pricing provider before returning from ``_resolve_provider``. List sourced from litellm ``constants.py``. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Abhijeet Prasad (AbhiPrasad)
approved these changes
Jul 17, 2026
Abhijeet Prasad (AbhiPrasad)
enabled auto-merge (squash)
July 17, 2026 21:01
Abhijeet Prasad (AbhiPrasad)
deleted the
fix/litellm-provider-metadata-drop-eager-serialization
branch
July 17, 2026 21:05
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Align the LiteLLM integration with
.agents/skills/sdk-integrations/SKILL.md. Audit turned up four issues:metadata.provider. SKILL §Completion-style explicitly says LiteLLM must setmetadata.providerto the underlying provider (openai/cohere/openrouter/ …), not the framework name. Fixed by routing every span through_resolve_provider(model)which callslitellm.get_llm_provider(model); falls back to"litellm"only when the lookup fails.@lru_cache(maxsize=256)on the resolver so the hot path doesn't re-dispatch through litellm's routing table per call._try_to_dict(response)on the full pydantic response then indexed into it — butbt_jsonalready serializes at log time. Replaced with direct attribute reads via a new shared_get_field(obj, key, default)helper inintegrations/utils.pythat works on both dicts and SDK objects (openai / anthropic / cohere had all been open-coding this pattern). Streaming paths left intact._is_litellm_patchedwas only used by crewai until fix(crewai): add provider metadata, route tools to metadata, drop eager serialization #587 moved crewai to the "wrapper never owns tokens" pattern; no remaining callers anywhere. Removed the helper and its two subprocess tests.wrap_litellm/_is_litellm_patcheddocstrings and the"""wrapt wrapper for litellm.X."""one-liner on every wrapper.Also lifted the rerank output cap (
results[:100]) intointegrations/utils.RERANK_OUTPUT_MAX_RESULTSand documented the "rerank exception to no-silent-caps" rule in the SKILL. Cohere carries the same magic number and can adopt the shared constant in a follow-up.Test plan
openai,cohere,openrouter) instead of"litellm". No cassette re-recording — HTTP requests unchanged, span-shape assertions only.metadata["provider"] == "openrouter"assertion to the existing OpenRouter cassette test as coverage for the new resolver._is_litellm_patchedsubprocess tests alongside the helper.ruff check+ruff format --checkon touched files — clean.cd py && nox -s "test_litellm(latest)"andcd py && nox -s "test_litellm(1.74.0)"(author's box has a broken Python 3.14 install).🤖 Generated with Claude Code
Created by abhijeet
Slack thread