feat(llm): add MiniMax provider#2510
Conversation
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMiniMax is integrated as an LLM provider with OpenAI-compatible and Anthropic-compatible APIs. The change adds configuration, registration, model metadata, message translation, metadata propagation through ChangesMiniMax provider integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LLMInput
participant ReActAgent
participant MiniMaxProvider
participant OpenAIChatCompletions
participant AnthropicMessages
LLMInput->>ReActAgent: run(input)
ReActAgent->>MiniMaxProvider: generate(input with metadata)
alt OpenAI-compatible base URL
MiniMaxProvider->>OpenAIChatCompletions: create(chat completion parameters)
OpenAIChatCompletions-->>MiniMaxProvider: completion response
else Anthropic-compatible base URL
MiniMaxProvider->>AnthropicMessages: create(messages parameters)
AnthropicMessages-->>MiniMaxProvider: messages response
end
MiniMaxProvider-->>ReActAgent: LLMOutput with tool calls and metadata
ReActAgent-->>LLMInput: next iteration preserves metadata
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
| Filename | Overview |
|---|---|
| src/llm/providers/minimax.py | Adds the MiniMax provider with OpenAI and Anthropic request handling. |
| src/llm/tools/executor.py | Carries request and response metadata through tool-loop turns. |
| src/llm/core/types.py | Adds MiniMax to provider types and supports structured message content with metadata. |
| src/llm/providers/resolver.py | Registers MiniMax in provider resolution. |
| src/llm/prompt/builder.py | Rejects structured content in system-message paths that require text. |
Reviews (5): Last reviewed commit: "fix(llm): tighten tool history validatio..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/llm/providers/minimax.py`:
- Around line 108-121: Update generate in the exception handler to log the
original exception and relevant model/provider context via
logging.getLogger(__name__).error(...) before raising any translated
AuthenticationError, RateLimitError, or ContextLengthError, while preserving the
existing classification and re-raise behavior.
- Around line 175-249: Reduce _generate_anthropic below 50 lines by extracting
the response.content text/tool_use processing into a focused helper and moving
response.usage-to-dictionary construction into a separate helper. Keep the
existing text aggregation, ToolCall creation, optional usage behavior, and
returned LLMOutput semantics unchanged.
- Around line 175-249: The _generate_anthropic method must enable adaptive
thinking by default for MiniMax-M3 requests. When the selected model is
MiniMax-M3 and llm_input.metadata does not provide a thinking override, set
params["thinking"] to {"type": "adaptive"}; preserve any caller-supplied
thinking value and existing behavior for other models.
- Line 63: Update the Minimax provider initialization around self.api_key to
retrieve the environment-backed secret with the repository’s established
required-key pattern, raising KeyError when neither the explicit api_key nor
MINIMAX_API_KEY is available instead of defaulting to an empty string. Apply the
same missing-secret behavior consistently in the existing AtlasProvider,
ClaudeProvider, and OpenAIProvider implementations.
- Around line 123-173: Update _generate_openai so reasoning-enabled requests
identified by thinking or reasoning_split in extra_body apply the same
max_tokens floor used by AtlasProvider.generate when llm_input.max_tokens is
unset or below that floor. Preserve explicitly larger max_tokens values, while
keeping the existing behavior for non-reasoning requests.
In `@tests/test_minimax_provider.py`:
- Around line 161-174: Extend tests in test_minimax_provider.py with an
OpenAI-provider case covering an empty choices list or missing first message and
asserting the existing “empty or filtered response” ValueError. Add generate()
exception-translation tests for HTTP 401, HTTP 429, and context-length failures,
asserting AuthenticationError, RateLimitError, and ContextLengthError
respectively while reusing the provider/client setup patterns already present.
- Around line 106-159: Extend both MiniMax provider tests to return and assert
inbound tool calls: update the mocked _OpenAICompletions.create response with a
choice.message.tool_calls entry and verify generate produces the expected
ToolCall, then update _AnthropicMessages.create with a tool_use content block
and assert the corresponding parsed ToolCall. Keep the existing outbound schema
assertions while covering both backend parsing paths in
test_minimax_provider_uses_openai_chat_completions and
test_minimax_provider_uses_anthropic_messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ed4b8e6e-ee95-48e8-9c0f-f7baa7297cf6
📒 Files selected for processing (7)
.env.examplesrc/llm/core/types.pysrc/llm/providers/__init__.pysrc/llm/providers/minimax.pysrc/llm/providers/resolver.pytests/test_minimax_provider.pytests/test_resolver.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
src/llm/providers/__init__.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/providers/resolver.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/{server,backend,api,src}/**/*.{ts,tsx,js,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
Log detailed error context on the server side
Files:
src/llm/providers/__init__.pysrc/llm/core/types.pysrc/llm/providers/resolver.pysrc/llm/providers/minimax.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
src/llm/providers/__init__.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/providers/resolver.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
src/llm/providers/__init__.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/providers/resolver.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
src/llm/providers/__init__.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/providers/resolver.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
src/llm/providers/__init__.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/providers/resolver.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.{py,pyi}: Follow PEP 8 conventions in Python code
Use type annotations on all function signatures in Python
Prefer immutable data structures such as frozen dataclasses and NamedTuple in Python
**/*.{py,pyi}: Auto-format Python files using black/ruff after edit
Run type checking using mypy/pyright after editing Python files
**/*.{py,pyi}: Use Protocol from typing module for duck typing and defining object shapes in Python
Use dataclasses with@dataclassdecorator for DTOs (Data Transfer Objects) in Python
Use context managers (with statement) for resource management in Python
Use generators for lazy evaluation and memory-efficient iteration in Python
Files:
src/llm/providers/__init__.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/providers/resolver.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.py: Use black for code formatting in Python
Use isort for import sorting in Python
Use ruff for linting Python codeAvoid using
print()statements in Python code; use theloggingmodule instead
**/*.py: Retrieve secrets and API keys from environment variables using os.environ with error handling (raise KeyError if missing) rather than hardcoding credentials
Use bandit for static security analysis in Python projects
Files:
src/llm/providers/__init__.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/providers/resolver.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*test*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-testing.md)
**/*test*.{py,pyi}: Use pytest as the testing framework for Python projects
Use pytest.mark for test categorization with markers like@pytest.mark.unitand@pytest.mark.integration
Files:
tests/test_resolver.pytests/test_minimax_provider.py
🧠 Learnings (1)
📚 Learning: 2026-06-28T09:52:09.015Z
Learnt from: gaurav0107
Repo: affaan-m/ECC PR: 2369
File: skills/continuous-learning-v2/scripts/test_parse_instinct.py:1099-1111
Timestamp: 2026-06-28T09:52:09.015Z
Learning: For pytest tests, ensure any marker you use (e.g., `pytest.mark.unit`) is registered in a config file (e.g., `pytest.ini`/`pyproject.toml` under `tool.pytest.ini_options`) or in a `conftest.py` that is reachable from the test module’s directory. If the marker registration `conftest.py` is not discovered for that test path, pytest will emit `PytestUnknownMarkWarning`; in such cases, register the marker globally or add a `conftest.py` within/above the test directory so the marker is known.
Applied to files:
tests/test_resolver.pytests/test_minimax_provider.py
🔇 Additional comments (10)
.env.example (1)
40-48: LGTM!src/llm/core/types.py (1)
24-24: LGTM!src/llm/providers/__init__.py (1)
6-6: LGTM!Also applies to: 16-16
src/llm/providers/resolver.py (1)
13-13: LGTM!Also applies to: 23-23
tests/test_resolver.py (1)
2-13: LGTM!Also applies to: 47-51
src/llm/providers/minimax.py (1)
73-80: 🩺 Stability & Availability
_enforce_credentials=Falseis already used in the other OpenAI-compatible providers, so this isn’t a Minimax-specific issue.> Likely an incorrect or invalid review comment.tests/test_minimax_provider.py (4)
1-72: LGTM!
74-159: LGTM!
176-288: LGTM!Good regression coverage using real SDK clients +
httpx.MockTransportto catch base-URL path-duplication bugs.
19-19: 📐 Maintainability & Code QualityNo action needed:
pytest.mark.unitis registered intests/conftest.py.> Likely an incorrect or invalid review comment.
| base_url: str | None = None, | ||
| default_model: str | None = None, | ||
| ) -> None: | ||
| self.api_key = api_key or os.environ.get(self.api_key_env) or "" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
Missing secret raises KeyError — falls back to empty string instead.
self.api_key silently becomes "" when MINIMAX_API_KEY is unset, deferring failure to a later API call rather than failing fast.
As per coding guidelines, "Retrieve secrets and API keys from environment variables using os.environ with error handling (raise KeyError if missing) rather than hardcoding credentials." Note this mirrors the existing pattern in AtlasProvider/ClaudeProvider/OpenAIProvider, so a fix here alone would be inconsistent unless applied repo-wide; flagging on the new file for visibility.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/llm/providers/minimax.py` at line 63, Update the Minimax provider
initialization around self.api_key to retrieve the environment-backed secret
with the repository’s established required-key pattern, raising KeyError when
neither the explicit api_key nor MINIMAX_API_KEY is available instead of
defaulting to an empty string. Apply the same missing-secret behavior
consistently in the existing AtlasProvider, ClaudeProvider, and OpenAIProvider
implementations.
Source: Coding guidelines
| def generate(self, llm_input: LLMInput) -> LLMOutput: | ||
| try: | ||
| if self._uses_anthropic: | ||
| return self._generate_anthropic(llm_input) | ||
| return self._generate_openai(llm_input) | ||
| except Exception as e: | ||
| msg = str(e) | ||
| if "401" in msg or "authentication" in msg.lower(): | ||
| raise AuthenticationError(msg, provider=self.provider_type) from e | ||
| if "429" in msg or "rate_limit" in msg.lower(): | ||
| raise RateLimitError(msg, provider=self.provider_type) from e | ||
| if "context" in msg.lower() and "length" in msg.lower(): | ||
| raise ContextLengthError(msg, provider=self.provider_type) from e | ||
| raise |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
No server-side logging of translated error context.
generate() catches and re-classifies exceptions but never logs the original error/context before raising the translated exception, making production debugging harder.
As per coding guidelines for **/{server,backend,api,src}/**/*.py: "Log detailed error context on the server side." Add a logging.getLogger(__name__).error(...) call (with model/provider context) before re-raising translated errors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/llm/providers/minimax.py` around lines 108 - 121, Update generate in the
exception handler to log the original exception and relevant model/provider
context via logging.getLogger(__name__).error(...) before raising any translated
AuthenticationError, RateLimitError, or ContextLengthError, while preserving the
existing classification and re-raise behavior.
Source: Coding guidelines
| def _generate_openai(self, llm_input: LLMInput) -> LLMOutput: | ||
| params: dict[str, Any] = { | ||
| "model": llm_input.model or self.default_model, | ||
| "messages": [msg.to_dict() for msg in llm_input.messages], | ||
| } | ||
| if llm_input.temperature != 1.0: | ||
| params["temperature"] = llm_input.temperature | ||
| if llm_input.max_tokens is not None: | ||
| params["max_tokens"] = llm_input.max_tokens | ||
| if llm_input.tools: | ||
| params["tools"] = [tool.to_openai_tool() for tool in llm_input.tools] | ||
|
|
||
| extra_body = { | ||
| key: llm_input.metadata[key] | ||
| for key in ("thinking", "reasoning_split", "service_tier") | ||
| if key in llm_input.metadata | ||
| } | ||
| if extra_body: | ||
| params["extra_body"] = extra_body | ||
|
|
||
| response = self.client.chat.completions.create(**params) | ||
| if not response.choices or response.choices[0].message is None: | ||
| raise ValueError(EMPTY_FILTERED_RESPONSE_ERROR) | ||
| choice = response.choices[0] | ||
|
|
||
| tool_calls = None | ||
| if choice.message.tool_calls: | ||
| tool_calls = [ | ||
| ToolCall( | ||
| id=tc.id or "", | ||
| name=tc.function.name, | ||
| arguments=_parse_tool_arguments(tc.function.arguments), | ||
| ) | ||
| for tc in choice.message.tool_calls | ||
| ] | ||
|
|
||
| usage = None | ||
| if response.usage: | ||
| usage = { | ||
| "prompt_tokens": response.usage.prompt_tokens, | ||
| "completion_tokens": response.usage.completion_tokens, | ||
| "total_tokens": response.usage.total_tokens, | ||
| } | ||
|
|
||
| return LLMOutput( | ||
| content=choice.message.content or "", | ||
| tool_calls=tool_calls, | ||
| model=response.model, | ||
| usage=usage, | ||
| stop_reason=choice.finish_reason, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
No max_tokens floor on the OpenAI-compatible path for reasoning models.
Unlike AtlasProvider.generate, which floors max_tokens because reasoning models "spend tokens on a thinking budget before the answer," _generate_openai only sets max_tokens when the caller explicitly supplies it (line 130-131). If a caller enables thinking/reasoning_split via extra_body without also setting max_tokens, the provider's own default could truncate the thinking budget before any visible answer is produced, especially for the 524,288 max-output MiniMax-M3 model.
🎯 Suggested floor for reasoning completions
- if llm_input.max_tokens is not None:
- params["max_tokens"] = llm_input.max_tokens
+ max_tokens = llm_input.max_tokens
+ if max_tokens is None or max_tokens < DEFAULT_MINIMAX_MAX_TOKENS:
+ max_tokens = DEFAULT_MINIMAX_MAX_TOKENS
+ params["max_tokens"] = max_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.
| def _generate_openai(self, llm_input: LLMInput) -> LLMOutput: | |
| params: dict[str, Any] = { | |
| "model": llm_input.model or self.default_model, | |
| "messages": [msg.to_dict() for msg in llm_input.messages], | |
| } | |
| if llm_input.temperature != 1.0: | |
| params["temperature"] = llm_input.temperature | |
| if llm_input.max_tokens is not None: | |
| params["max_tokens"] = llm_input.max_tokens | |
| if llm_input.tools: | |
| params["tools"] = [tool.to_openai_tool() for tool in llm_input.tools] | |
| extra_body = { | |
| key: llm_input.metadata[key] | |
| for key in ("thinking", "reasoning_split", "service_tier") | |
| if key in llm_input.metadata | |
| } | |
| if extra_body: | |
| params["extra_body"] = extra_body | |
| response = self.client.chat.completions.create(**params) | |
| if not response.choices or response.choices[0].message is None: | |
| raise ValueError(EMPTY_FILTERED_RESPONSE_ERROR) | |
| choice = response.choices[0] | |
| tool_calls = None | |
| if choice.message.tool_calls: | |
| tool_calls = [ | |
| ToolCall( | |
| id=tc.id or "", | |
| name=tc.function.name, | |
| arguments=_parse_tool_arguments(tc.function.arguments), | |
| ) | |
| for tc in choice.message.tool_calls | |
| ] | |
| usage = None | |
| if response.usage: | |
| usage = { | |
| "prompt_tokens": response.usage.prompt_tokens, | |
| "completion_tokens": response.usage.completion_tokens, | |
| "total_tokens": response.usage.total_tokens, | |
| } | |
| return LLMOutput( | |
| content=choice.message.content or "", | |
| tool_calls=tool_calls, | |
| model=response.model, | |
| usage=usage, | |
| stop_reason=choice.finish_reason, | |
| ) | |
| def _generate_openai(self, llm_input: LLMInput) -> LLMOutput: | |
| params: dict[str, Any] = { | |
| "model": llm_input.model or self.default_model, | |
| "messages": [msg.to_dict() for msg in llm_input.messages], | |
| } | |
| max_tokens = llm_input.max_tokens | |
| if max_tokens is None or max_tokens < DEFAULT_MINIMAX_MAX_TOKENS: | |
| max_tokens = DEFAULT_MINIMAX_MAX_TOKENS | |
| params["max_tokens"] = max_tokens | |
| if llm_input.tools: | |
| params["tools"] = [tool.to_openai_tool() for tool in llm_input.tools] | |
| extra_body = { | |
| key: llm_input.metadata[key] | |
| for key in ("thinking", "reasoning_split", "service_tier") | |
| if key in llm_input.metadata | |
| } | |
| if extra_body: | |
| params["extra_body"] = extra_body | |
| response = self.client.chat.completions.create(**params) | |
| if not response.choices or response.choices[0].message is None: | |
| raise ValueError(EMPTY_FILTERED_RESPONSE_ERROR) | |
| choice = response.choices[0] | |
| tool_calls = None | |
| if choice.message.tool_calls: | |
| tool_calls = [ | |
| ToolCall( | |
| id=tc.id or "", | |
| name=tc.function.name, | |
| arguments=_parse_tool_arguments(tc.function.arguments), | |
| ) | |
| for tc in choice.message.tool_calls | |
| ] | |
| usage = None | |
| if response.usage: | |
| usage = { | |
| "prompt_tokens": response.usage.prompt_tokens, | |
| "completion_tokens": response.usage.completion_tokens, | |
| "total_tokens": response.usage.total_tokens, | |
| } | |
| return LLMOutput( | |
| content=choice.message.content or "", | |
| tool_calls=tool_calls, | |
| model=response.model, | |
| usage=usage, | |
| stop_reason=choice.finish_reason, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/llm/providers/minimax.py` around lines 123 - 173, Update _generate_openai
so reasoning-enabled requests identified by thinking or reasoning_split in
extra_body apply the same max_tokens floor used by AtlasProvider.generate when
llm_input.max_tokens is unset or below that floor. Preserve explicitly larger
max_tokens values, while keeping the existing behavior for non-reasoning
requests.
| def test_minimax_anthropic_provider_rejects_empty_responses() -> None: | ||
| provider = MiniMaxProvider(api_key="test", base_url=MINIMAX_ANTHROPIC_BASE_URL) | ||
| provider.client = _AnthropicClient( | ||
| SimpleNamespace( | ||
| content=[], | ||
| model=DEFAULT_MINIMAX_MODEL, | ||
| usage=SimpleNamespace(input_tokens=1, output_tokens=0), | ||
| stop_reason="end_turn", | ||
| ) | ||
| ) | ||
|
|
||
| with pytest.raises(ValueError, match="empty or filtered response"): | ||
| provider.generate(LLMInput(messages=[Message(role=Role.USER, content="hi")])) | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Add symmetric OpenAI empty-response test and error-translation coverage.
Only the Anthropic empty/filtered-response guard is tested. The OpenAI path has the same guard (if not response.choices or response.choices[0].message is None) per the upstream contract, but it's untested here. Additionally, generate()'s exception translation (401→AuthenticationError, 429→RateLimitError, context-length→ContextLengthError) has no test coverage in this file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_minimax_provider.py` around lines 161 - 174, Extend tests in
test_minimax_provider.py with an OpenAI-provider case covering an empty choices
list or missing first message and asserting the existing “empty or filtered
response” ValueError. Add generate() exception-translation tests for HTTP 401,
HTTP 429, and context-length failures, asserting AuthenticationError,
RateLimitError, and ContextLengthError respectively while reusing the
provider/client setup patterns already present.
daltino
left a comment
daltino
left a comment
There was a problem hiding this comment.
This PR adds support for the MiniMax provider, following the established patterns in the repo. The .env.example updates, constant declarations, provider implementation, and resolver integration look well-structured, complete, and consistent with the codebase conventions. The addition aligns with the contributing guidelines, expanding provider options without affecting existing functionality.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
src/llm/providers/minimax.py (5)
248-249: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnforce
max_tokensfloor for reasoning models.
_generate_openailacks amax_tokensfloor for reasoning models. If a caller enables a thinking budget without also settingmax_tokens, the provider's default could truncate the budget before a visible answer is produced.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/llm/providers/minimax.py` around lines 248 - 249, Update _generate_openai so reasoning models enforce an appropriate max_tokens floor when a thinking budget is enabled, including when llm_input.max_tokens is unset. Preserve an explicitly larger caller-provided max_tokens value while raising lower or absent values to the required floor.
231-239: 📐 Maintainability & Code Quality | 🟡 Minor | 💤 Low valueLog detailed error context.
generate()catches and re-classifies exceptions but never logs the original error context. As per coding guidelines, log detailed error context on the server side before raising the translated exception to aid in production debugging.🔧 Proposed fix
except Exception as e: + import logging + logging.getLogger(__name__).error("MiniMax provider generation error: %s", e, exc_info=True) msg = str(e) if "401" in msg or "authentication" in msg.lower():🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/llm/providers/minimax.py` around lines 231 - 239, Update generate()’s exception handler to log the caught exception with detailed server-side context before any re-classification or re-raise. Preserve the existing AuthenticationError, RateLimitError, ContextLengthError, and generic raise behavior after logging.Source: Coding guidelines
324-327: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnable adaptive thinking by default for MiniMax-M3.
MiniMax-M3’s Anthropic-compatible endpoint disables thinking by default, which can cause plain requests to return empty content. Enable adaptive thinking explicitly when the caller selects MiniMax-M3 and does not provide an override.
🔧 Proposed fix
- for key in ("thinking", "service_tier"): - if key in llm_input.metadata: - params[key] = llm_input.metadata[key] + if "thinking" in llm_input.metadata: + params["thinking"] = llm_input.metadata["thinking"] + elif params["model"] == "MiniMax-M3": + params["thinking"] = {"type": "adaptive"} + + if "service_tier" in llm_input.metadata: + params["service_tier"] = llm_input.metadata["service_tier"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/llm/providers/minimax.py` around lines 324 - 327, Update the parameter construction near the metadata loop to enable adaptive thinking by default when the selected model is MiniMax-M3 and the caller has not supplied a thinking override. Preserve the existing metadata value when “thinking” is present, and leave other models and parameters unchanged.
301-380: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFunction exceeds 50 lines.
_generate_anthropicsignificantly exceeds the 50-line limit. As per coding guidelines, extract helper functions (e.g., for parsing content blocks or extracting token usage) to improve maintainability and testability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/llm/providers/minimax.py` around lines 301 - 380, The _generate_anthropic method exceeds the 50-line limit; extract its response-processing responsibilities into focused helpers, such as helpers for parsing content blocks and building usage data. Keep _generate_anthropic responsible for request construction and orchestration, preserving the existing text, tool-call, metadata, and usage behavior.Source: Coding guidelines
181-181: 🔒 Security & Privacy | 🟡 Minor | 💤 Low valueRaise
KeyErrorwhen secret is missing.
self.api_keysilently defaults to""whenMINIMAX_API_KEYis unset. As per coding guidelines, retrieve secrets usingos.environand raiseKeyErrorif missing rather than deferring failure to a later API call.🔧 Proposed fix
- self.api_key = api_key or os.environ.get(self.api_key_env) or "" + self.api_key = api_key or os.environ[self.api_key_env]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/llm/providers/minimax.py` at line 181, Update the API-key initialization in the Minimax provider constructor to use os.environ directly when no explicit api_key is supplied, removing the empty-string fallback so a missing MINIMAX_API_KEY raises KeyError immediately.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/llm/tools/executor.py`:
- Line 5: Update the import statement in executor.py to import Callable from
collections.abc instead of typing, while preserving the existing Any and
Protocol imports from typing.
In `@tests/test_minimax_provider.py`:
- Around line 276-337: Update tests/test_minimax_provider.py:276-337 around
preserved_content to deep-copy the fixture before generation, assert the
original remains unchanged, and verify the serialized assistant content is a
distinct object; at tests/test_minimax_provider.py:344-374, snapshot both
multimodal input lists before generation, then assert each remains unchanged and
is not aliased by the serialized output, covering the no-mutation/new-object
contract.
- Around line 257-264: Extend the assertions for the assistant message in the
completions serialization test to validate the serialized tool call’s ID and
function name, alongside its existing type and arguments checks. Use the
expected values from the fixture so the call remains correctly associated with
the subsequent tool result.
---
Duplicate comments:
In `@src/llm/providers/minimax.py`:
- Around line 248-249: Update _generate_openai so reasoning models enforce an
appropriate max_tokens floor when a thinking budget is enabled, including when
llm_input.max_tokens is unset. Preserve an explicitly larger caller-provided
max_tokens value while raising lower or absent values to the required floor.
- Around line 231-239: Update generate()’s exception handler to log the caught
exception with detailed server-side context before any re-classification or
re-raise. Preserve the existing AuthenticationError, RateLimitError,
ContextLengthError, and generic raise behavior after logging.
- Around line 324-327: Update the parameter construction near the metadata loop
to enable adaptive thinking by default when the selected model is MiniMax-M3 and
the caller has not supplied a thinking override. Preserve the existing metadata
value when “thinking” is present, and leave other models and parameters
unchanged.
- Around line 301-380: The _generate_anthropic method exceeds the 50-line limit;
extract its response-processing responsibilities into focused helpers, such as
helpers for parsing content blocks and building usage data. Keep
_generate_anthropic responsible for request construction and orchestration,
preserving the existing text, tool-call, metadata, and usage behavior.
- Line 181: Update the API-key initialization in the Minimax provider
constructor to use os.environ directly when no explicit api_key is supplied,
removing the empty-string fallback so a missing MINIMAX_API_KEY raises KeyError
immediately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3338481d-0720-4a93-aabb-1fe13bfc4be4
📒 Files selected for processing (9)
.env.examplesrc/llm/cli/selector.pysrc/llm/core/types.pysrc/llm/providers/minimax.pysrc/llm/tools/executor.pytests/test_executor.pytests/test_minimax_provider.pytests/test_resolver.pytests/test_types.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
tests/test_types.pysrc/llm/cli/selector.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/tools/executor.pytests/test_executor.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
tests/test_types.pysrc/llm/cli/selector.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/tools/executor.pytests/test_executor.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
tests/test_types.pysrc/llm/cli/selector.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/tools/executor.pytests/test_executor.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
tests/test_types.pysrc/llm/cli/selector.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/tools/executor.pytests/test_executor.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
tests/test_types.pysrc/llm/cli/selector.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/tools/executor.pytests/test_executor.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.{py,pyi}: Follow PEP 8 conventions in Python code
Use type annotations on all function signatures in Python
Prefer immutable data structures such as frozen dataclasses and NamedTuple in Python
**/*.{py,pyi}: Auto-format Python files using black/ruff after edit
Run type checking using mypy/pyright after editing Python files
**/*.{py,pyi}: Use Protocol from typing module for duck typing and defining object shapes in Python
Use dataclasses with@dataclassdecorator for DTOs (Data Transfer Objects) in Python
Use context managers (with statement) for resource management in Python
Use generators for lazy evaluation and memory-efficient iteration in Python
Files:
tests/test_types.pysrc/llm/cli/selector.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/tools/executor.pytests/test_executor.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.py: Use black for code formatting in Python
Use isort for import sorting in Python
Use ruff for linting Python codeAvoid using
print()statements in Python code; use theloggingmodule instead
**/*.py: Retrieve secrets and API keys from environment variables using os.environ with error handling (raise KeyError if missing) rather than hardcoding credentials
Use bandit for static security analysis in Python projects
Files:
tests/test_types.pysrc/llm/cli/selector.pysrc/llm/core/types.pytests/test_resolver.pysrc/llm/tools/executor.pytests/test_executor.pysrc/llm/providers/minimax.pytests/test_minimax_provider.py
**/*test*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-testing.md)
**/*test*.{py,pyi}: Use pytest as the testing framework for Python projects
Use pytest.mark for test categorization with markers like@pytest.mark.unitand@pytest.mark.integration
Files:
tests/test_types.pytests/test_resolver.pytests/test_executor.pytests/test_minimax_provider.py
**/{server,backend,api,src}/**/*.{ts,tsx,js,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
Log detailed error context on the server side
Files:
src/llm/cli/selector.pysrc/llm/core/types.pysrc/llm/tools/executor.pysrc/llm/providers/minimax.py
🧠 Learnings (1)
📚 Learning: 2026-06-28T09:52:09.015Z
Learnt from: gaurav0107
Repo: affaan-m/ECC PR: 2369
File: skills/continuous-learning-v2/scripts/test_parse_instinct.py:1099-1111
Timestamp: 2026-06-28T09:52:09.015Z
Learning: For pytest tests, ensure any marker you use (e.g., `pytest.mark.unit`) is registered in a config file (e.g., `pytest.ini`/`pyproject.toml` under `tool.pytest.ini_options`) or in a `conftest.py` that is reachable from the test module’s directory. If the marker registration `conftest.py` is not discovered for that test path, pytest will emit `PytestUnknownMarkWarning`; in such cases, register the marker globally or add a `conftest.py` within/above the test directory so the marker is known.
Applied to files:
tests/test_types.pytests/test_resolver.pytests/test_executor.pytests/test_minimax_provider.py
🪛 ast-grep (0.44.1)
src/llm/providers/minimax.py
[info] 93-93: use jsonify instead of json.dumps for JSON output
Context: json.dumps(tool_call.arguments)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.15.21)
src/llm/tools/executor.py
[warning] 5-5: Import from collections.abc instead: Callable
Import from collections.abc
(UP035)
src/llm/providers/minimax.py
[warning] 75-75: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 111-111: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 117-117: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 301-301: Too many branches (14 > 12)
(PLR0912)
[warning] 307-307: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (11)
src/llm/tools/executor.py (1)
78-78: LGTM!Also applies to: 97-115
src/llm/core/types.py (2)
24-24: LGTM!
28-34: LGTM!.env.example (1)
45-45: LGTM!src/llm/cli/selector.py (2)
98-101: LGTM!
113-116: LGTM!tests/test_types.py (2)
29-29: LGTM!
39-39: LGTM!tests/test_resolver.py (2)
3-3: LGTM!
110-119: LGTM!tests/test_minimax_provider.py (1)
1-1: LGTM!Also applies to: 11-18, 31-59, 64-65, 97-117, 120-128, 131-220
| preserved_content = [ | ||
| {"type": "thinking", "thinking": "plan", "signature": "sig"}, | ||
| { | ||
| "type": "tool_use", | ||
| "id": "tool_1", | ||
| "name": "search", | ||
| "input": {"query": "docs"}, | ||
| }, | ||
| { | ||
| "type": "tool_use", | ||
| "id": "tool_2", | ||
| "name": "search", | ||
| "input": {"query": "examples"}, | ||
| }, | ||
| ] | ||
|
|
||
| provider.generate( | ||
| LLMInput( | ||
| messages=[ | ||
| Message(role=Role.USER, content="Find references."), | ||
| Message( | ||
| role=Role.ASSISTANT, | ||
| content="", | ||
| tool_calls=[ | ||
| ToolCall(id="tool_1", name="search", arguments={}), | ||
| ToolCall(id="tool_2", name="search", arguments={}), | ||
| ], | ||
| metadata={"anthropic_content": preserved_content}, | ||
| ), | ||
| Message( | ||
| role=Role.TOOL, | ||
| content="Found docs.", | ||
| tool_call_id="tool_1", | ||
| ), | ||
| Message( | ||
| role=Role.TOOL, | ||
| content="Found examples.", | ||
| tool_call_id="tool_2", | ||
| ), | ||
| ] | ||
| ) | ||
| ) | ||
|
|
||
| assert client.messages.params["messages"] == [ | ||
| {"role": "user", "content": "Find references."}, | ||
| {"role": "assistant", "content": preserved_content}, | ||
| { | ||
| "role": "user", | ||
| "content": [ | ||
| { | ||
| "type": "tool_result", | ||
| "tool_use_id": "tool_1", | ||
| "content": "Found docs.", | ||
| }, | ||
| { | ||
| "type": "tool_result", | ||
| "tool_use_id": "tool_2", | ||
| "content": "Found examples.", | ||
| }, | ||
| ], | ||
| }, | ||
| ] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use independent snapshots for mutable serialization fixtures.
The current assertions can pass after in-place mutation because the input objects also serve as expected values.
tests/test_minimax_provider.py#L276-L337: deep-copypreserved_content, assert the original remains unchanged, and verify the serialized content is a distinct object.tests/test_minimax_provider.py#L344-L374: snapshot both multimodal lists before generation and assert each input remains unchanged and unaliased.
As per coding guidelines, “Always create new objects, never mutate existing ones.”
📍 Affects 1 file
tests/test_minimax_provider.py#L276-L337(this comment)tests/test_minimax_provider.py#L344-L374
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_minimax_provider.py` around lines 276 - 337, Update
tests/test_minimax_provider.py:276-337 around preserved_content to deep-copy the
fixture before generation, assert the original remains unchanged, and verify the
serialized assistant content is a distinct object; at
tests/test_minimax_provider.py:344-374, snapshot both multimodal input lists
before generation, then assert each remains unchanged and is not aliased by the
serialized output, covering the no-mutation/new-object contract.
Source: Coding guidelines
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
tests/test_executor.py (2)
88-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCategorize the new pytest test.
Add
@pytest.mark.unitto the new test and register the marker in pytest configuration if it is not already registered; otherwise collection can emitPytestUnknownMarkWarning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_executor.py` around lines 88 - 127, Add the pytest.mark.unit decorator to test_react_agent_preserves_request_and_response_metadata and register the unit marker in the existing pytest configuration if it is not already declared, avoiding unknown-marker warnings during collection.Sources: Coding guidelines, Learnings
92-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an immutable capture in the test double.
self.inputs.append(...)mutates shared test state. Prefer tuple reassignment or a newly allocated list so this test follows the repository’s immutable-object requirement and does not introduce avoidable aliasing.Proposed fix
def __init__(self) -> None: - self.inputs: list[LLMInput] = [] + self.inputs: tuple[LLMInput, ...] = () def generate(self, llm_input: LLMInput) -> LLMOutput: - self.inputs.append(llm_input) + self.inputs = (*self.inputs, llm_input)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_executor.py` around lines 92 - 97, Update MetadataProvider.generate to avoid mutating self.inputs with append; capture each LLMInput through immutable tuple reassignment or a newly allocated collection while preserving the existing input-capture behavior.Source: Coding guidelines
src/llm/tools/executor.py (1)
91-98: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCreate an immutable request snapshot per iteration.
LLMInputis frozen, butmessages=messagesreuses the list that is later extended for subsequent tool rounds. A provider retaining the first input can therefore observe its history mutate aftergenerate()returns. Pass fresh containers formessagesand metadata.Proposed fix
input_copy = LLMInput( - messages=messages, + messages=list(messages), model=input.model, temperature=input.temperature, max_tokens=input.max_tokens, tools=tools, - metadata=input.metadata, + metadata=dict(input.metadata), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/llm/tools/executor.py` around lines 91 - 98, Update the LLMInput construction in the tool-round execution flow to create independent containers for each request snapshot: copy messages and metadata instead of reusing the mutable values later updated between iterations. Preserve the existing request fields and ensure retained inputs cannot observe subsequent tool-round mutations.Source: Coding guidelines
tests/test_builder.py (1)
28-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the required return annotation to the new test.
Declare
test_build_rejects_structured_system_content(self) -> Noneto comply with the repository’s Python typing guideline.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_builder.py` around lines 28 - 36, Add the return annotation -> None to the test_build_rejects_structured_system_content method signature, leaving the test behavior unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/llm/tools/executor.py`:
- Around line 91-98: Update the LLMInput construction in the tool-round
execution flow to create independent containers for each request snapshot: copy
messages and metadata instead of reusing the mutable values later updated
between iterations. Preserve the existing request fields and ensure retained
inputs cannot observe subsequent tool-round mutations.
In `@tests/test_builder.py`:
- Around line 28-36: Add the return annotation -> None to the
test_build_rejects_structured_system_content method signature, leaving the test
behavior unchanged.
In `@tests/test_executor.py`:
- Around line 88-127: Add the pytest.mark.unit decorator to
test_react_agent_preserves_request_and_response_metadata and register the unit
marker in the existing pytest configuration if it is not already declared,
avoiding unknown-marker warnings during collection.
- Around line 92-97: Update MetadataProvider.generate to avoid mutating
self.inputs with append; capture each LLMInput through immutable tuple
reassignment or a newly allocated collection while preserving the existing
input-capture behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6a99b06b-7e88-4cd9-8441-1bb61e80f20b
📒 Files selected for processing (4)
src/llm/tools/executor.pytests/test_builder.pytests/test_executor.pytests/test_minimax_provider.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
src/llm/tools/executor.pytests/test_builder.pytests/test_executor.pytests/test_minimax_provider.py
**/{server,backend,api,src}/**/*.{ts,tsx,js,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
Log detailed error context on the server side
Files:
src/llm/tools/executor.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
src/llm/tools/executor.pytests/test_builder.pytests/test_executor.pytests/test_minimax_provider.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
src/llm/tools/executor.pytests/test_builder.pytests/test_executor.pytests/test_minimax_provider.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
src/llm/tools/executor.pytests/test_builder.pytests/test_executor.pytests/test_minimax_provider.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
src/llm/tools/executor.pytests/test_builder.pytests/test_executor.pytests/test_minimax_provider.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.{py,pyi}: Follow PEP 8 conventions in Python code
Use type annotations on all function signatures in Python
Prefer immutable data structures such as frozen dataclasses and NamedTuple in Python
**/*.{py,pyi}: Auto-format Python files using black/ruff after edit
Run type checking using mypy/pyright after editing Python files
**/*.{py,pyi}: Use Protocol from typing module for duck typing and defining object shapes in Python
Use dataclasses with@dataclassdecorator for DTOs (Data Transfer Objects) in Python
Use context managers (with statement) for resource management in Python
Use generators for lazy evaluation and memory-efficient iteration in Python
Files:
src/llm/tools/executor.pytests/test_builder.pytests/test_executor.pytests/test_minimax_provider.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.py: Use black for code formatting in Python
Use isort for import sorting in Python
Use ruff for linting Python codeAvoid using
print()statements in Python code; use theloggingmodule instead
**/*.py: Retrieve secrets and API keys from environment variables using os.environ with error handling (raise KeyError if missing) rather than hardcoding credentials
Use bandit for static security analysis in Python projects
Files:
src/llm/tools/executor.pytests/test_builder.pytests/test_executor.pytests/test_minimax_provider.py
**/*test*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-testing.md)
**/*test*.{py,pyi}: Use pytest as the testing framework for Python projects
Use pytest.mark for test categorization with markers like@pytest.mark.unitand@pytest.mark.integration
Files:
tests/test_builder.pytests/test_executor.pytests/test_minimax_provider.py
🧠 Learnings (1)
📚 Learning: 2026-06-28T09:52:09.015Z
Learnt from: gaurav0107
Repo: affaan-m/ECC PR: 2369
File: skills/continuous-learning-v2/scripts/test_parse_instinct.py:1099-1111
Timestamp: 2026-06-28T09:52:09.015Z
Learning: For pytest tests, ensure any marker you use (e.g., `pytest.mark.unit`) is registered in a config file (e.g., `pytest.ini`/`pyproject.toml` under `tool.pytest.ini_options`) or in a `conftest.py` that is reachable from the test module’s directory. If the marker registration `conftest.py` is not discovered for that test path, pytest will emit `PytestUnknownMarkWarning`; in such cases, register the marker globally or add a `conftest.py` within/above the test directory so the marker is known.
Applied to files:
tests/test_builder.pytests/test_executor.pytests/test_minimax_provider.py
🔇 Additional comments (4)
src/llm/tools/executor.py (2)
5-6: LGTM!Also applies to: 22-22
106-112: 🗄️ Data Integrity & IntegrationNo issue: MiniMax reads
message.metadatadirectly.Message.to_dict()still omits metadata, butMiniMaxProvider._anthropic_message()consumesmessage.metadata["anthropic_content"]on the request path, and the MiniMax tests assert the serialized payload keeps the preserved blocks.> Likely an incorrect or invalid review comment.tests/test_builder.py (1)
2-2: LGTM!tests/test_minimax_provider.py (1)
262-263: LGTM!
2 participants
What Changed
MINIMAX_BASE_URL.Why This Change
This makes the MiniMax models selectable through the existing provider registry while preserving the repository's current configuration and adapter patterns.
Testing Done
.venv/bin/python -m pytest -q tests/test_*.py -m 'not integration'(96 passed).venv/bin/ruff check --isolatedon the changed Python files.venv/bin/mypy src/llmgit diff --checkType of Change
feat:New featureSecurity & Quality Checklist