Open
Conversation
Add MiniMax (MiniMax-M2.7, M2.5, M2.5-highspeed) as a 5th LLM provider alongside OpenAI, Anthropic, Google AI, and VideoDB Proxy. MiniMax uses an OpenAI-compatible API at api.minimax.io/v1. - Add MiniMax provider with config, model enum, and chat completions - Support tool/function calling, JSON response format, think-tag stripping - Temperature clamping to [0, 1] range - Auto-detection via MINIMAX_API_KEY env var - Add to get_default_llm() factory and LLMType/EnvPrefix enums - 35 unit tests + 3 integration tests (simple chat, JSON, tool calling)
Contributor
📝 WalkthroughWalkthroughA new MiniMax LLM provider integration is added to enable MiniMax as an LLM service option alongside existing providers. The implementation includes environment configuration, enum constants, client initialization with OpenAI-compatible API, message and tool formatting helpers, comprehensive unit tests, and integration test coverage. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant LLM as MiniMax LLM
participant Client as OpenAI Client
participant API as MiniMax API
App->>LLM: chat_completions(messages, tools, response_format)
LLM->>LLM: _format_messages(messages)
LLM->>LLM: _format_tools(tools)
LLM->>Client: client.chat.completions.create()
Client->>API: POST /v1/chat/completions
API-->>Client: Response (content, tool_calls, usage)
Client-->>LLM: ChatCompletion object
LLM->>LLM: _strip_think_tags(content)
LLM->>LLM: Parse tool_calls JSON arguments
LLM-->>App: LLMResponse (status, content, tool_calls, usage)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/director/llm/__init__.py (1)
20-29:⚠️ Potential issue | 🟠 MajorHonor
DEFAULT_LLMbefore key-based auto-detection.With the current ordering,
DEFAULT_LLM=minimaxis still bypassed whenever an earlier provider key likeOPENAI_API_KEYis present, so everyget_default_llm()call site keeps using OpenAI. Check the explicit override first, then fall back to the existing auto-detect order, and add a regression test forDEFAULT_LLM=minimaxplusOPENAI_API_KEY.🧭 Suggested fix
- if openai or default_llm == LLMType.OPENAI: + if default_llm == LLMType.OPENAI: return OpenAI() - elif anthropic or default_llm == LLMType.ANTHROPIC: + elif default_llm == LLMType.ANTHROPIC: return AnthropicAI() - elif googleai or default_llm == LLMType.GOOGLEAI: + elif default_llm == LLMType.GOOGLEAI: return GoogleAI() - elif minimax or default_llm == LLMType.MINIMAX: + elif default_llm == LLMType.MINIMAX: return MiniMax() + elif openai: + return OpenAI() + elif anthropic: + return AnthropicAI() + elif googleai: + return GoogleAI() + elif minimax: + return MiniMax() else: return VideoDBProxy()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/director/llm/__init__.py` around lines 20 - 29, The get_default_llm logic currently checks provider keys before honoring the DEFAULT_LLM env override, so DEFAULT_LLM=minimax is ignored when e.g. OPENAI_API_KEY exists; change the branching in the function that sets default_llm (references: default_llm variable and the return branches returning OpenAI, AnthropicAI, GoogleAI, MiniMax) to first check if default_llm is set and return the corresponding class (e.g., if default_llm == LLMType.MINIMAX return MiniMax()) before running the key-based auto-detection, and add a regression test asserting that DEFAULT_LLM=minimax plus OPENAI_API_KEY results in MiniMax being selected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/director/llm/minimax.py`:
- Around line 147-169: The chat_completions method currently ignores the stop
parameter; update the method (chat_completions in minimax.py) to include the
caller-supplied stop sequences in the request params when provided (e.g., set
params["stop"] = stop or the appropriate MiniMax key) so MiniMax receives and
respects the stop sequences; add this check alongside the existing
response_format/tools handling to only add the stop entry when stop is not None.
- Around line 171-199: The tool-call JSON parsing must be moved into the API
error-guard so malformed tool payloads don't bubble up; wrap access to
response.choices[0].message.tool_calls and the
json.loads(tool_call.function.arguments) parsing in the same try/except that
surrounds self.client.chat.completions.create (or add an inner try that catches
JSON/ValueError and returns an LLMResponse error), and ensure the method (the
block building the LLMResponse with tool_calls, finish_reason, usage fields)
returns a graceful LLMResponse on parse errors instead of letting exceptions
escape from response or tool_call.function.arguments.
In `@backend/tests/test_minimax_integration.py`:
- Around line 12-15: The module-level pytest mark currently only skips when
MINIMAX_API_KEY is missing; update the pytestmark so the module is also marked
as integration (e.g., change pytestmark to include pytest.mark.integration in
addition to the existing pytest.mark.skipif), referencing the pytestmark symbol
and the MINIMAX_API_KEY check so these tests run only when explicitly selected
with -m integration and when the env var is set.
---
Outside diff comments:
In `@backend/director/llm/__init__.py`:
- Around line 20-29: The get_default_llm logic currently checks provider keys
before honoring the DEFAULT_LLM env override, so DEFAULT_LLM=minimax is ignored
when e.g. OPENAI_API_KEY exists; change the branching in the function that sets
default_llm (references: default_llm variable and the return branches returning
OpenAI, AnthropicAI, GoogleAI, MiniMax) to first check if default_llm is set and
return the corresponding class (e.g., if default_llm == LLMType.MINIMAX return
MiniMax()) before running the key-based auto-detection, and add a regression
test asserting that DEFAULT_LLM=minimax plus OPENAI_API_KEY results in MiniMax
being selected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5b034c2-b2fb-4a0e-bc9d-a1ee2f4cb983
📒 Files selected for processing (8)
README.mdbackend/.env.samplebackend/director/constants.pybackend/director/llm/__init__.pybackend/director/llm/minimax.pybackend/tests/__init__.pybackend/tests/test_minimax.pybackend/tests/test_minimax_integration.py
Comment on lines
+147
to
+169
| def chat_completions( | ||
| self, messages: list, tools: list = [], stop=None, response_format=None | ||
| ): | ||
| """Get chat completions using MiniMax. | ||
|
|
||
| MiniMax provides an OpenAI-compatible API at https://api.minimax.io/v1. | ||
| docs: https://platform.minimaxi.com/document/ChatCompletion%20v2 | ||
| """ | ||
| params = { | ||
| "model": self.chat_model, | ||
| "messages": self._format_messages(messages), | ||
| "temperature": self.temperature, | ||
| "max_tokens": self.max_tokens, | ||
| "top_p": self.top_p, | ||
| "timeout": self.timeout, | ||
| } | ||
|
|
||
| if tools: | ||
| params["tools"] = self._format_tools(tools) | ||
| params["tool_choice"] = "auto" | ||
|
|
||
| if response_format: | ||
| params["response_format"] = response_format |
Contributor
There was a problem hiding this comment.
Don't silently ignore stop.
chat_completions(..., stop=...) accepts the parameter but never uses it, so MiniMax will ignore caller-supplied stop sequences.
🛑 Suggested fix
if tools:
params["tools"] = self._format_tools(tools)
params["tool_choice"] = "auto"
+ if stop is not None:
+ params["stop"] = stop
+
if response_format:
params["response_format"] = response_format📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def chat_completions( | |
| self, messages: list, tools: list = [], stop=None, response_format=None | |
| ): | |
| """Get chat completions using MiniMax. | |
| MiniMax provides an OpenAI-compatible API at https://api.minimax.io/v1. | |
| docs: https://platform.minimaxi.com/document/ChatCompletion%20v2 | |
| """ | |
| params = { | |
| "model": self.chat_model, | |
| "messages": self._format_messages(messages), | |
| "temperature": self.temperature, | |
| "max_tokens": self.max_tokens, | |
| "top_p": self.top_p, | |
| "timeout": self.timeout, | |
| } | |
| if tools: | |
| params["tools"] = self._format_tools(tools) | |
| params["tool_choice"] = "auto" | |
| if response_format: | |
| params["response_format"] = response_format | |
| def chat_completions( | |
| self, messages: list, tools: list = [], stop=None, response_format=None | |
| ): | |
| """Get chat completions using MiniMax. | |
| MiniMax provides an OpenAI-compatible API at https://api.minimax.io/v1. | |
| docs: https://platform.minimaxi.com/document/ChatCompletion%20v2 | |
| """ | |
| params = { | |
| "model": self.chat_model, | |
| "messages": self._format_messages(messages), | |
| "temperature": self.temperature, | |
| "max_tokens": self.max_tokens, | |
| "top_p": self.top_p, | |
| "timeout": self.timeout, | |
| } | |
| if tools: | |
| params["tools"] = self._format_tools(tools) | |
| params["tool_choice"] = "auto" | |
| if stop is not None: | |
| params["stop"] = stop | |
| if response_format: | |
| params["response_format"] = response_format |
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 148-148: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/director/llm/minimax.py` around lines 147 - 169, The chat_completions
method currently ignores the stop parameter; update the method (chat_completions
in minimax.py) to include the caller-supplied stop sequences in the request
params when provided (e.g., set params["stop"] = stop or the appropriate MiniMax
key) so MiniMax receives and respects the stop sequences; add this check
alongside the existing response_format/tools handling to only add the stop entry
when stop is not None.
Comment on lines
+171
to
+199
| try: | ||
| response = self.client.chat.completions.create(**params) | ||
| except Exception as e: | ||
| print(f"Error: {e}") | ||
| return LLMResponse(content=f"Error: {e}") | ||
|
|
||
| content = response.choices[0].message.content or "" | ||
| content = self._strip_think_tags(content) | ||
|
|
||
| return LLMResponse( | ||
| content=content, | ||
| tool_calls=[ | ||
| { | ||
| "id": tool_call.id, | ||
| "tool": { | ||
| "name": tool_call.function.name, | ||
| "arguments": json.loads(tool_call.function.arguments), | ||
| }, | ||
| "type": tool_call.type, | ||
| } | ||
| for tool_call in response.choices[0].message.tool_calls | ||
| ] | ||
| if response.choices[0].message.tool_calls | ||
| else [], | ||
| finish_reason=response.choices[0].finish_reason, | ||
| send_tokens=response.usage.prompt_tokens, | ||
| recv_tokens=response.usage.completion_tokens, | ||
| total_tokens=response.usage.total_tokens, | ||
| status=LLMResponseStatus.SUCCESS, |
Contributor
There was a problem hiding this comment.
Keep tool-call parsing inside the guarded error path.
json.loads(tool_call.function.arguments) runs after the API-call try/except, so one malformed tool payload will raise out of chat_completions() instead of returning an LLMResponse. That turns a recoverable provider error into an agent crash.
🧰 Suggested fix
try:
response = self.client.chat.completions.create(**params)
+ content = self._strip_think_tags(response.choices[0].message.content or "")
+ tool_calls = [
+ {
+ "id": tool_call.id,
+ "tool": {
+ "name": tool_call.function.name,
+ "arguments": json.loads(tool_call.function.arguments),
+ },
+ "type": tool_call.type,
+ }
+ for tool_call in response.choices[0].message.tool_calls or []
+ ]
except Exception as e:
print(f"Error: {e}")
- return LLMResponse(content=f"Error: {e}")
-
- content = response.choices[0].message.content or ""
- content = self._strip_think_tags(content)
+ return LLMResponse(content=f"Error: {e}", status=LLMResponseStatus.ERROR)
return LLMResponse(
content=content,
- tool_calls=[
- {
- "id": tool_call.id,
- "tool": {
- "name": tool_call.function.name,
- "arguments": json.loads(tool_call.function.arguments),
- },
- "type": tool_call.type,
- }
- for tool_call in response.choices[0].message.tool_calls
- ]
- if response.choices[0].message.tool_calls
- else [],
+ tool_calls=tool_calls,
finish_reason=response.choices[0].finish_reason,
send_tokens=response.usage.prompt_tokens,
recv_tokens=response.usage.completion_tokens,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try: | |
| response = self.client.chat.completions.create(**params) | |
| except Exception as e: | |
| print(f"Error: {e}") | |
| return LLMResponse(content=f"Error: {e}") | |
| content = response.choices[0].message.content or "" | |
| content = self._strip_think_tags(content) | |
| return LLMResponse( | |
| content=content, | |
| tool_calls=[ | |
| { | |
| "id": tool_call.id, | |
| "tool": { | |
| "name": tool_call.function.name, | |
| "arguments": json.loads(tool_call.function.arguments), | |
| }, | |
| "type": tool_call.type, | |
| } | |
| for tool_call in response.choices[0].message.tool_calls | |
| ] | |
| if response.choices[0].message.tool_calls | |
| else [], | |
| finish_reason=response.choices[0].finish_reason, | |
| send_tokens=response.usage.prompt_tokens, | |
| recv_tokens=response.usage.completion_tokens, | |
| total_tokens=response.usage.total_tokens, | |
| status=LLMResponseStatus.SUCCESS, | |
| try: | |
| response = self.client.chat.completions.create(**params) | |
| content = self._strip_think_tags(response.choices[0].message.content or "") | |
| tool_calls = [ | |
| { | |
| "id": tool_call.id, | |
| "tool": { | |
| "name": tool_call.function.name, | |
| "arguments": json.loads(tool_call.function.arguments), | |
| }, | |
| "type": tool_call.type, | |
| } | |
| for tool_call in response.choices[0].message.tool_calls or [] | |
| ] | |
| except Exception as e: | |
| print(f"Error: {e}") | |
| return LLMResponse(content=f"Error: {e}", status=LLMResponseStatus.ERROR) | |
| return LLMResponse( | |
| content=content, | |
| tool_calls=tool_calls, | |
| finish_reason=response.choices[0].finish_reason, | |
| send_tokens=response.usage.prompt_tokens, | |
| recv_tokens=response.usage.completion_tokens, | |
| total_tokens=response.usage.total_tokens, | |
| status=LLMResponseStatus.SUCCESS, |
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 173-173: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/director/llm/minimax.py` around lines 171 - 199, The tool-call JSON
parsing must be moved into the API error-guard so malformed tool payloads don't
bubble up; wrap access to response.choices[0].message.tool_calls and the
json.loads(tool_call.function.arguments) parsing in the same try/except that
surrounds self.client.chat.completions.create (or add an inner try that catches
JSON/ValueError and returns an LLMResponse error), and ensure the method (the
block building the LLMResponse with tool_calls, finish_reason, usage fields)
returns a graceful LLMResponse on parse errors instead of letting exceptions
escape from response or tool_call.function.arguments.
Comment on lines
+12
to
+15
| pytestmark = pytest.mark.skipif( | ||
| not os.getenv("MINIMAX_API_KEY"), | ||
| reason="MINIMAX_API_KEY not set", | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
rg -n 'pytest\.mark\.integration|pytestmark\s*=' backend/tests/test_minimax_integration.py
fd '^(pyproject\.toml|pytest\.ini|setup\.cfg|tox\.ini)$' | xargs -r rg -n 'markers|integration'Repository: video-db/Director
Length of output: 98
🏁 Script executed:
head -n 30 backend/tests/test_minimax_integration.pyRepository: video-db/Director
Length of output: 755
🏁 Script executed:
fd 'conftest.py' -x cat {} \;Repository: video-db/Director
Length of output: 43
🏁 Script executed:
find . -name "pyproject.toml" -o -name "pytest.ini" -o -name "setup.cfg" -o -name "tox.ini" | head -5Repository: video-db/Director
Length of output: 85
🏁 Script executed:
cat backend/pyproject.toml | grep -A 20 "markers\|pytest"Repository: video-db/Director
Length of output: 43
🏁 Script executed:
rg "pytest\.mark\.integration" backend/tests/Repository: video-db/Director
Length of output: 43
🏁 Script executed:
cat backend/pyproject.tomlRepository: video-db/Director
Length of output: 530
🏁 Script executed:
find . -type f -name "conftest.py" | head -3 | xargs catRepository: video-db/Director
Length of output: 43
🏁 Script executed:
rg "addopts|markers" backend/ --type toml --type ini --type cfgRepository: video-db/Director
Length of output: 88
🏁 Script executed:
rg "class.*integration\|def.*integration" backend/tests/ -A 2 | head -20Repository: video-db/Director
Length of output: 43
🏁 Script executed:
find backend -type f \( -name "pytest.ini" -o -name "setup.cfg" \)Repository: video-db/Director
Length of output: 43
🏁 Script executed:
rg "\[tool\.pytest" backend/pyproject.toml -A 10Repository: video-db/Director
Length of output: 43
🏁 Script executed:
rg "pytest\s+-.*integration|-m" backend/ | head -20Repository: video-db/Director
Length of output: 1614
🏁 Script executed:
git log --oneline backend/tests/test_minimax_integration.py 2>/dev/null | head -5Repository: video-db/Director
Length of output: 96
Add the missing pytest.mark.integration marker to match documented behavior.
The module docstring says these tests can be skipped with pytest -m "not integration", but the module lacks the integration marker. Without it, anyone with MINIMAX_API_KEY set will execute live API calls during normal test runs, regardless of the -m flag.
🧪 Suggested fix
-pytestmark = pytest.mark.skipif(
- not os.getenv("MINIMAX_API_KEY"),
- reason="MINIMAX_API_KEY not set",
-)
+pytestmark = [
+ pytest.mark.integration,
+ pytest.mark.skipif(
+ not os.getenv("MINIMAX_API_KEY"),
+ reason="MINIMAX_API_KEY not set",
+ ),
+]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pytestmark = pytest.mark.skipif( | |
| not os.getenv("MINIMAX_API_KEY"), | |
| reason="MINIMAX_API_KEY not set", | |
| ) | |
| pytestmark = [ | |
| pytest.mark.integration, | |
| pytest.mark.skipif( | |
| not os.getenv("MINIMAX_API_KEY"), | |
| reason="MINIMAX_API_KEY not set", | |
| ), | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/tests/test_minimax_integration.py` around lines 12 - 15, The
module-level pytest mark currently only skips when MINIMAX_API_KEY is missing;
update the pytestmark so the module is also marked as integration (e.g., change
pytestmark to include pytest.mark.integration in addition to the existing
pytest.mark.skipif), referencing the pytestmark symbol and the MINIMAX_API_KEY
check so these tests run only when explicitly selected with -m integration and
when the env var is set.
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
1 participant
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
Adds MiniMax as a 5th LLM provider alongside OpenAI, Anthropic, Google AI, and VideoDB Proxy.
MiniMax provides an OpenAI-compatible API, making integration seamless with the existing provider architecture. This PR adds support for:
Changes
backend/director/llm/minimax.py— New MiniMax provider withMiniMaxConfig,MiniMaxChatModelenum, andMiniMaxclass extendingBaseLLM. Includes temperature clamping to [0, 1] and think-tag stripping for M2.7 reasoning output.backend/director/constants.py— AddMINIMAXtoLLMTypeenum andMINIMAX_toEnvPrefixenum.backend/director/llm/__init__.py— Add MiniMax toget_default_llm()factory withMINIMAX_API_KEYauto-detection.backend/.env.sample— AddMINIMAX_API_KEYconfiguration.README.md— Mention MiniMax in supported LLM providers list.backend/tests/test_minimax.py— 35 unit tests covering config, model enum, message/tool formatting, chat completions, think-tag stripping, factory detection, and error handling.backend/tests/test_minimax_integration.py— 3 integration tests (simple chat, JSON response format, tool calling) that run against the real MiniMax API.Configuration
Test plan
pytest tests/test_minimax.py)DEFAULT_LLM=minimaxin a local Director instanceNo new dependencies required — MiniMax uses the existing
openaiSDK via its OpenAI-compatible API endpoint.Summary by CodeRabbit
Release Notes
New Features
Documentation