Skip to content

Conversation

@amanjaiswal73892
Copy link
Collaborator

@amanjaiswal73892 amanjaiswal73892 commented Aug 5, 2025

  • Implement LiteLLMModel and LiteLLMAPIMessageBuilder for LiteLLM API interaction.
  • Enhance APIPayload to include reasoning_effort parameter.
  • Introduce get_pricing_litellm function for pricing information retrieval.
  • Create tests for multi-action, single tool call scenarios and force tool call.
  • Added an option to discard extra tool calls by setting use_only_first_toolcall.

LiteLLM can be used with tool use agent as follows:

from agentlab.agents.tool_use_agent import DEFAULT_PROMPT_CONFIG, ToolUseAgentArgs
from agentlab.experiments.study import Study
from agentlab.llm.litellm_api import LiteLLMModelArgs

def get_agent(model_name: str) -> ToolUseAgentArgs:
    return ToolUseAgentArgs(
        model_args=LiteLLMModelArgs(
            model_name=model_name,
            max_new_tokens=2000,
            temperature=None,
            # use_only_first_toolcall=True,  # Set to True for single actions
        ),
        config=DEFAULT_PROMPT_CONFIG,
    )

models = [
    "openai/gpt-4.1",
    "openai/gpt-4.1-mini",
    "openai/gpt-4.1-nano",
    "openai/o3-2025-04-16",
    "anthropic/claude-3-7-sonnet-20250219",
    "anthropic/claude-sonnet-4-20250514",
    ## Add more models to test.
]
agent_args = [get_agent(model) for model in models]


study = Study(agent_args, "miniwob_tiny_test", logging_level_stdout=logging.WARNING)
study.run(
    n_jobs=5,
    parallel_backend="ray",
    strict_reproducibility=False,
    n_relaunch=3,
)

Description by Korbit AI

What change is being made?

Integrate LiteLLM API into the existing codebase, allowing the use of LiteLLM for chat completion and tool calls, and add tests for its functionalities.

Why are these changes being made?

The integration adds new API options for leveraging LiteLLM's capabilities in generating chat completion and executing tool actions, providing more flexibility and scalability when managing interactions. Tests are included to validate that the new functionality operates as expected, covering scenarios such as multiple tool calls, single tool calls, and forced tool calls. The change also includes tracking LiteLLM pricing, crucial for budgeting and analysis.

Is this description stale? Ask me to generate a new description by commenting /korbit-generate-pr-description

- Implement LiteLLMModel and LiteLLMAPIMessageBuilder for LiteLLM API interaction.
Enhance APIPayload to include reasoning_effort parameter.
- Introduce get_pricing_litellm function for pricing information retrieval.
- Create tests for multi-action, single tool call scenarios and force tool call.
- Added an option to discard extra tool calls by setting use_only_first_toolcall.
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Readability Unexplained type ignore ▹ view
Performance Inefficient Message List Building ▹ view
Functionality Hardcoded Configuration Parameter ▹ view
Design Unused Reasoning Effort Parameter ▹ view
Files scanned
File Path Reviewed
src/agentlab/llm/tracking.py
src/agentlab/llm/litellm_api.py
src/agentlab/llm/response_api.py

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

if tool_calls is None:
return None
tool_call_list = []
for tc in tool_calls: # type: ignore
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexplained type ignore category Readability

Tell me more
What is the issue?

Type ignore comment used without explanation of why type checking needs to be suppressed

Why this matters

Unexplained type ignores make it difficult to understand the code's type safety assumptions and maintainability

Suggested change ∙ Feature Preview

Add a comment explaining why the type ignore is necessary:

# type: ignore # tool_calls is dynamically typed from API response
for tc in tool_calls:
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines +69 to +70
for msg in payload.messages: # type: ignore
input.extend(msg.prepare_message())
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inefficient Message List Building category Performance

Tell me more
What is the issue?

Messages are being processed one at a time with extend() calls, which can be inefficient for large message histories.

Why this matters

Multiple extend operations on a list cause repeated memory allocations and copies. This becomes more significant with larger message histories.

Suggested change ∙ Feature Preview

Use a list comprehension to build the input list in one operation:

input = [msg for messages in payload.messages for msg in messages.prepare_message()]  # type: ignore
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

temperature=temperature,
max_tokens=max_tokens,
)
self.action_space_as_tools = True # this should be a config
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded Configuration Parameter category Functionality

Tell me more
What is the issue?

The action_space_as_tools parameter is hardcoded to True but commented as 'should be a config', without being included in the initialization parameters.

Why this matters

Users cannot configure this setting, forcing them to use tools-based action space even if they need text-based actions.

Suggested change ∙ Feature Preview

Add the parameter to the constructor:

def __init__(self, model_name: str, base_url: Optional[str] = None, api_key: Optional[str] = None, temperature: float | None = None, max_tokens: int | None = 100, use_only_first_toolcall: bool = False, action_space_as_tools: bool = True):
    ...
    self.action_space_as_tools = action_space_as_tools
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

cache_complete_prompt: bool = (
False # If True, will cache the complete prompt in the last message.
)
reasoning_effort: Literal["low", "medium", "high"] | None = None
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused Reasoning Effort Parameter category Design

Tell me more
What is the issue?

The 'reasoning_effort' parameter is introduced but not utilized in any of the response models (OpenAI, Claude, or OpenAIChatCompletion).

Why this matters

Adding unused parameters creates confusion and technical debt. The code suggests this parameter is only for LiteLLM API, but the implementation is missing.

Suggested change ∙ Feature Preview

Either remove the parameter until LiteLLM integration is implemented, or implement it in the _call_api methods. For example:

def _call_api(self, payload: APIPayload) -> Any:
    api_params = {...}
    if payload.reasoning_effort is not None:
        api_params['reasoning_effort'] = payload.reasoning_effort
    # rest of the code
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

@amanjaiswal73892 amanjaiswal73892 requested a review from ollmer August 5, 2025 18:14
@amanjaiswal73892 amanjaiswal73892 linked an issue Aug 6, 2025 that may be closed by this pull request
7 tasks
@recursix recursix merged commit 219e467 into main Aug 11, 2025
7 checks passed
@recursix recursix deleted the litellm branch August 11, 2025 17:15
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.

Bring LiteLLM support

3 participants