feat: add LiteLLM as chat model provider for 100+ LLM backends#456
feat: add LiteLLM as chat model provider for 100+ LLM backends#456RheagalFire wants to merge 7 commits into
Conversation
|
cc @yiboyasss |
There was a problem hiding this comment.
Code Review
This pull request introduces a new LiteLLM provider to the xagent core model, enabling access to numerous LLM providers through a unified interface. The implementation includes the LiteLLM class, updates to the model adapter factory, and a suite of unit tests. Feedback highlights several critical issues in the new implementation: the chat method fails to include the mandatory raw field in tool call responses, the stream_chat method uses incorrect chunk types and lacks support for tool calls, and there is a risk of an IndexError when accessing response choices without prior validation.
| }, | ||
| } | ||
| ) | ||
| return {"type": "tool_call", "tool_calls": tool_calls} |
There was a problem hiding this comment.
The tool call response dictionary is missing the raw field. According to the BaseLLM.chat interface documentation (see base.py line 163), the returned dictionary for tool calls must include the full response JSON in a raw field to allow for debugging and advanced processing.
return {
"type": "tool_call",
"tool_calls": tool_calls,
"raw": response.model_dump() if hasattr(response, "model_dump") else dict(response),
}| async for chunk in response: | ||
| delta = chunk.choices[0].delta if chunk.choices else None | ||
| if delta is None: | ||
| continue | ||
| content = getattr(delta, "content", None) | ||
| if content: | ||
| yield StreamChunk(type=ChunkType.TEXT, content=content) |
There was a problem hiding this comment.
The stream_chat implementation has several issues that break consistency and functionality:
- It uses
ChunkType.TEXTinstead of the standardChunkType.TOKENused in the rest of the codebase (e.g., inBaseLLM.stream_chat). - It omits the
deltafield inStreamChunk, which is expected by consumers. - It completely ignores tool calls in the stream, which will cause them to be dropped silently during streaming.
Updating the loop to handle these cases ensures compatibility with the expected streaming behavior and supports tool calling in streaming mode.
async for chunk in response:
if not chunk.choices:
continue
delta = chunk.choices[0].delta
# Handle text content
content = getattr(delta, "content", None)
if content:
yield StreamChunk(type=ChunkType.TOKEN, content=content, delta=content)
# Handle tool calls
if hasattr(delta, "tool_calls") and delta.tool_calls:
tool_calls = []
for tc in delta.tool_calls:
tool_calls.append({
"index": getattr(tc, "index", 0),
"id": getattr(tc, "id", None),
"type": "function",
"function": {
"name": getattr(tc.function, "name", None),
"arguments": getattr(tc.function, "arguments", ""),
},
})
yield StreamChunk(
type=ChunkType.TOOL_CALL,
tool_calls=tool_calls,
raw=chunk.model_dump() if hasattr(chunk, "model_dump") else dict(chunk)
)| ) as e: | ||
| raise LLMRetryableError(str(e)) from e | ||
|
|
||
| choice = response.choices[0] |
There was a problem hiding this comment.
The code accesses response.choices[0] without verifying that choices is not empty. While LiteLLM typically returns at least one choice on success, certain edge cases (like content filtering or provider-specific errors) could result in an empty list, leading to an IndexError.
| choice = response.choices[0] | |
| if not response.choices: | |
| raise LLMRetryableError("LiteLLM returned an empty response (no choices).") | |
| choice = response.choices[0] |
|
Could you fix the lint, and resolve the gemini comments? |
|
@qinxuye Fixed all three gemini review items:
Lint passes clean. |
|
Lint (pre-commit) failed again... Could you fix it? |
|
@qinxuye can u check again |
rogercloud
left a comment
There was a problem hiding this comment.
Code Review
Thanks for the work here — adding LiteLLM as a meta-provider is a reasonable direction, the factory wiring follows the existing per-provider pattern, and all three prior Gemini findings (missing raw field, ChunkType.TOKEN/delta/tool calls in the stream, empty-choices guard) are addressed. The design is sound; the main feedback is about how the provider is implemented rather than the direction.
1. (Blocking, design) Don't re-implement OpenAI-shaped parsing — reuse OpenAILLM
litellm.acompletion() returns OpenAI-shaped responses (choices[0].delta.content, tool_calls with index/id/function, usage.prompt_tokens, model_dump()). The current LiteLLM.chat()/stream_chat() hand-roll a second, thinner parser for this same shape, which drops capabilities that OpenAILLM already gets right:
- Streaming token usage is lost —
chat()callsadd_token_usage(), butstream_chat()records nothing and never emits aChunkType.USAGEchunk (OpenAILLMsetsstream_options={"include_usage": True}and emits usage). - Streaming tool-call argument fragments are not accumulated across chunks —
OpenAILLM._parse_stream_chunkaccumulates by index/id; the new code yields each partial fragment standalone, producing incomplete tool calls. output_config(json_schema structured output) is silently ignored —OpenAILLMconverts it into a concreteresponse_format.- First-token / inter-token timeout control and the empty-content-raises behavior are also absent.
DeepSeekLLM is the precedent: it subclasses OpenAILLM and reuses all of this instead of duplicating it.
Suggested path, sequenced as two PRs:
- A prior, standalone refactor PR that extracts a replaceable "completion call" seam in
OpenAILLM(e.g. an overridable_acreate()), since today the API call is inlined insidechat/stream_chat. - This PR rebased on top of it, with
LiteLLMbecoming a thinOpenAILLMsubclass that only overrides the call seam (swapAsyncOpenAIclient →litellm.acompletion, passdrop_params=True).
This single change subsumes the streaming-usage, tool-call-accumulation, output_config, empty-content, and code-duplication issues at once.
2. (Blocking, packaging) Declare litellm as a dependency
litellm.py imports litellm, but the package is absent from pyproject.toml (neither core deps nor an extra). The lazy in-method import defers the failure, but a standard install yields ModuleNotFoundError on first call — an implicit/phantom dependency. Add litellm to the core [project.dependencies], consistent with the other provider SDKs (openai, anthropic, google-genai, zai-sdk), which are all core.
3. (Required, completeness) Register litellm in the provider metadata
The adapter has a "litellm" branch, but litellm is not in _SUPPORTED_PROVIDER_METADATA in providers.py, so it never appears in get_supported_provider_metadata() and is not selectable in the UI provider list. Add a metadata entry so it surfaces in the UI.
Tests
The 20 unit tests cover init, non-stream chat, tool calling, error mapping, and the factory well. Gap: there are no stream_chat tests — which is exactly the path with the parity issues above. Once restructured around OpenAILLM, the inherited streaming path should get coverage.
Summary
Adds
LiteLLMas a new chat model provider alongside OpenAI, Claude, DeepSeek, Gemini, and Zhipu, enabling access to 100+ LLM backends through LiteLLM's unified interface.Changes
src/xagent/core/model/chat/basic/litellm.pyLiteLLM(BaseLLM)with asyncchat()andstream_chat(). Useslitellm.acompletion()withdrop_params=True. Supports tool calling, response format, token usage tracking. Maps litellm exceptions toLLMRetryableError/LLMTimeoutError.src/xagent/core/model/chat/basic/__init__.pyLiteLLM.src/xagent/core/model/chat/basic/adapter.py"litellm"provider branch increate_base_llm()factory.tests/core/model/chat/basic/test_litellm.pyTests
Unit tests -- 20 passed:
Live e2e via Anthropic:
Lint:
Example usage
Any LiteLLM model string works. See LiteLLM docs for full list.