Add MiniMax provider routing - #2621
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for routing MiniMax models by registering 'minimax' in the provider registry and adding corresponding unit tests. The reviewer pointed out a critical issue where using the generic OpenAIProvider directly for MiniMax will incorrectly default to OpenAI's base URL and API key. They suggested creating a dedicated MiniMaxProvider subclass to properly handle MiniMax-specific configurations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| _PROVIDER_REGISTRY: dict[str, tuple[str, str]] = { | ||
| "openai": ("giskard.llm.providers.openai", "OpenAIProvider"), | ||
| "minimax": ("giskard.llm.providers.openai", "OpenAIProvider"), |
There was a problem hiding this comment.
Using OpenAIProvider directly for minimax without explicit configuration will cause it to default to OpenAI's base URL (https://api.openai.com/v1) and look for OPENAI_API_KEY in the environment. This means unconfigured routing for minimax/* models will fail or incorrectly target OpenAI endpoints.
To fix this, we should define a MiniMaxProvider subclass in libs/giskard-llm/src/giskard/llm/providers/openai.py that defaults to the MiniMax base URL (https://api.minimax.chat/v1) and looks up MINIMAX_API_KEY from the environment if not explicitly provided.
For example, in libs/giskard-llm/src/giskard/llm/providers/openai.py:
class MiniMaxProvider(OpenAIProvider):
_PROVIDER = "minimax"
def __init__(
self,
api_key: str | None = None,
base_url: str | None = None,
**kwargs: Any
) -> None:
import os
api_key = api_key or os.environ.get("MINIMAX_API_KEY")
base_url = base_url or "https://api.minimax.chat/v1"
super().__init__(api_key=api_key, base_url=base_url, **kwargs)| "minimax": ("giskard.llm.providers.openai", "OpenAIProvider"), | |
| "minimax": ("giskard.llm.providers.openai", "MiniMaxProvider"), |
davidberenstein1957
left a comment
There was a problem hiding this comment.
MiniMax provider routing looks good.
Reason: Add MiniMax provider support so the LLM provider registry can route MiniMax-M3 and MiniMax-M2.7 models.
Added a MiniMax registry alias backed by the OpenAI-compatible provider implementation.
Checks: pytest libs/giskard-llm/tests/test_routing.py -q