Add vLLM OpenAI-Compatible API Support #252
Conversation
Summary of ChangesHello @reopio, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces comprehensive support for vLLM's OpenAI-compatible API, integrating it as a new backend option for Large Language Models. The changes enable users to configure custom vLLM API endpoints and specify distinct models for both shallow and deep thinking agents, as well as for embedding generation, all through interactive command-line prompts. This significantly expands the system's flexibility, allowing for the utilization of self-hosted vLLM deployments and their performance benefits while adhering to the familiar OpenAI API standard. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully adds support for vLLM as an LLM provider. The changes are well-structured, but there are a few areas for improvement. I've identified a critical bug where user input for the vLLM embedding model is discarded, which would prevent the feature from working as intended. Additionally, there are several instances of code duplication and unused configuration that should be addressed to improve the maintainability and clarity of the codebase. My detailed comments provide specific suggestions to resolve these issues.
tradingagents/graph/trading_graph.py
Outdated
| if self.config["llm_provider"] == "vllm": | ||
| questionary.text( | ||
| "Please input the vllm embedding model name (default: None):", | ||
| default="None", | ||
| validate=lambda x: len(x.strip()) > 0 or "Please enter a valid embedding model name.", | ||
| style=questionary.Style( | ||
| [ | ||
| ("text", "fg:green"), | ||
| ("highlighted", "noinherit"), | ||
| ] | ||
| ), | ||
| ).ask() | ||
| self.bull_memory = FinancialSituationMemory("bull_memory", self.config) | ||
| self.bear_memory = FinancialSituationMemory("bear_memory", self.config) | ||
| self.trader_memory = FinancialSituationMemory("trader_memory", self.config) | ||
| self.invest_judge_memory = FinancialSituationMemory("invest_judge_memory", self.config) | ||
| self.risk_manager_memory = FinancialSituationMemory("risk_manager_memory", self.config) | ||
| else: | ||
| self.bull_memory = FinancialSituationMemory("bull_memory", self.config) | ||
| self.bear_memory = FinancialSituationMemory("bear_memory", self.config) | ||
| self.trader_memory = FinancialSituationMemory("trader_memory", self.config) | ||
| self.invest_judge_memory = FinancialSituationMemory("invest_judge_memory", self.config) | ||
| self.risk_manager_memory = FinancialSituationMemory("risk_manager_memory", self.config) |
There was a problem hiding this comment.
There are a couple of issues in this block:
- Critical Bug: The user's input for the vLLM embedding model name is prompted but the result is discarded. The
default='None'is also problematic as it would likely cause an error if used as a model name. - Code Duplication: The initialization of
FinancialSituationMemoryfor all agents is duplicated in both theifandelsebranches.
To fix this, you should store the user's input and move the duplicated memory initialization out of the conditional block. A valid default model name should also be used.
if self.config["llm_provider"] == "vllm":
self.config["embeddings"] = questionary.text(
"Please input the vllm embedding model name (e.g., nomic-embed-text):",
default="nomic-embed-text",
validate=lambda x: len(x.strip()) > 0 or "Please enter a valid embedding model name.",
style=questionary.Style(
[
("text", "fg:green"),
("highlighted", "noinherit"),
]
),
).ask()
self.bull_memory = FinancialSituationMemory("bull_memory", self.config)
self.bear_memory = FinancialSituationMemory("bear_memory", self.config)
self.trader_memory = FinancialSituationMemory("trader_memory", self.config)
self.invest_judge_memory = FinancialSituationMemory("invest_judge_memory", self.config)
self.risk_manager_memory = FinancialSituationMemory("risk_manager_memory", self.config)
cli/utils.py
Outdated
| "vllm": [ | ||
| ("llama3.1 local", "llama3.1"), | ||
| ("qwen3", "qwen3"), | ||
| ] |
There was a problem hiding this comment.
These model options for vllm are defined but are never used because the code for the vllm provider prompts the user for a model name via text input. This unused configuration can be confusing for future maintainers. Please remove it.
| if provider == "vllm": | ||
| choice = questionary.text( | ||
| "Please input the vllm model name for shallow thinking (default: llama3.1):", | ||
| default="llama3.1", | ||
| validate=lambda x: len(x.strip()) > 0 or "Please enter a valid model name.", | ||
| style=questionary.Style( | ||
| [ | ||
| ("text", "fg:green"), | ||
| ("highlighted", "noinherit"), | ||
| ] | ||
| ), | ||
| ).ask() | ||
|
|
||
| else: | ||
| # Use questionary to create an interactive selection menu for shallow thinking LLM engines | ||
| choice = questionary.select( | ||
| "Select Your [Quick-Thinking LLM Engine]:", | ||
| choices=[ | ||
| questionary.Choice(display, value=value) | ||
| for display, value in SHALLOW_AGENT_OPTIONS[provider.lower()] | ||
| ], | ||
| instruction="\n- Use arrow keys to navigate\n- Press Enter to select", | ||
| style=questionary.Style( | ||
| [ | ||
| ("selected", "fg:magenta noinherit"), | ||
| ("highlighted", "fg:magenta noinherit"), | ||
| ("pointer", "fg:magenta noinherit"), | ||
| ] | ||
| ), | ||
| ).ask() |
There was a problem hiding this comment.
This if/else block for selecting a model is nearly identical to the one in select_deep_thinking_agent (lines 243-272). This duplication makes the code harder to maintain. Consider extracting this logic into a helper function that can be called by both select_shallow_thinking_agent and select_deep_thinking_agent to reduce redundancy.
cli/utils.py
Outdated
| "vllm": [ | ||
| ("llama3.1 local", "llama3.1"), | ||
| ("qwen3", "qwen3"), | ||
| ] |
There was a problem hiding this comment.
Similar to the shallow thinking agent options, these vllm options are defined but not used. The logic for the vllm provider prompts the user for text input directly. Please remove this unused configuration to avoid confusion.
tradingagents/agents/utils/memory.py
Outdated
| import chromadb | ||
| from chromadb.config import Settings | ||
| from openai import OpenAI | ||
| import questionary |
There was a problem hiding this comment.
The questionary library is imported but not used in this file. It should be removed to keep the code clean and avoid unnecessary dependencies in this module.
1 participant
Summary
This PR adds support for vLLM's OpenAI-compatible API, allowing users to leverage vLLM as an alternative LLM backend.
Changes
Usage
When selecting vLLM as the LLM provider, users will be prompted to manually enter:
http://localhost:8000/v1)Benefits
Testing