Skip to content

feat: add MiniMax as embedding and LLM (genie) provider with M3 as default - #530

Open
octo-patch wants to merge 5 commits into
feyninc:mainfrom
octo-patch:feature/add-minimax-provider
Open

feat: add MiniMax as embedding and LLM (genie) provider with M3 as default#530
octo-patch wants to merge 5 commits into
feyninc:mainfrom
octo-patch:feature/add-minimax-provider

Conversation

@octo-patch

@octo-patch octo-patch commented Mar 22, 2026

Copy link
Copy Markdown

Summary

Add MiniMax as a first-class embedding and LLM provider in Chonkie, with the latest MiniMax-M3 as the default LLM model.

MiniMaxEmbeddings

  • Uses MiniMax's native embedding API (not OpenAI-compatible) via direct httpx REST calls
  • Supports the embo-01 model (1536 dimensions)
  • Distinguishes between "db" (storage) and "query" (search) embedding types for asymmetric retrieval
  • Full batch and async support with automatic retry on transient HTTP errors
  • Registered in EmbeddingsRegistry with minimax:// provider URI and embo-* pattern matching

MiniMaxGenie

  • LLM inference via MiniMax's OpenAI-compatible API (https://api.minimax.io/v1)
  • Supports MiniMax-M3 (default), MiniMax-M2.7, and MiniMax-M2.7-highspeed models
  • MiniMax-M3 features a 512K context window, up to 128K output, and image input support
  • Temperature clamping to MiniMax's accepted range
  • Automatic <think> tag stripping for reasoning models
  • JSON mode and structured output support

Files Changed

  • src/chonkie/embeddings/minimax.py — MiniMaxEmbeddings with native API client
  • src/chonkie/genie/minimax.py — MiniMaxGenie with OpenAI-compat SDK (M3 default, keeps M2.7)
  • src/chonkie/embeddings/registry.py — Registry entries for minimax provider
  • src/chonkie/embeddings/__init__.py, src/chonkie/genie/__init__.py, src/chonkie/__init__.py — Exports
  • pyproject.tomlminimax optional dependency group
  • tests/embeddings/test_minimax_embeddings.py — Unit + integration tests
  • tests/genie/test_minimax_genie.py — Unit + integration tests (updated for M3)

Test plan

  • Unit tests for MiniMaxEmbeddings — all passing
  • Unit tests for MiniMaxGenie — updated for M3 default, all passing
  • Integration tests (xfail due to rate limits, verified manually)
  • ruff check and format pass
  • CI pipeline validation

Summary by CodeRabbit

  • New Features

    • Added MiniMax embeddings provider (batching, sync/async, retries) and MiniMax LLM service (text and structured JSON generation).
  • Tests

    • Added comprehensive test suites covering embeddings, LLM behavior, error handling, and integration (gated).
  • Chores

    • Added an optional "minimax" install extra and included it in the aggregate "all" extras.

Add MiniMax support with two new provider classes:

- MiniMaxEmbeddings: native MiniMax embedding API (embo-01 model,
  1536 dimensions) with db/query type distinction, batching, and
  async support
- MiniMaxGenie: OpenAI-compatible chat completions for MiniMax
  M2.7/M2.5 models with temperature clamping, think-tag stripping,
  and JSON structured output

Both providers are registered in the embeddings registry and
exported from the main package. Includes 61 unit tests and 6
integration tests.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 adds MiniMax as a first-class provider for both embeddings and LLM inference within the Chonkie library. It includes the necessary classes, registration, and testing to fully integrate MiniMax's capabilities, providing users with a new option for their AI applications.

Highlights

  • MiniMax Integration: This PR introduces MiniMax as a new provider for both embeddings and LLM inference (Genie), offering an alternative to existing services.
  • Embedding Support: The MiniMaxEmbeddings class provides native MiniMax Embedding API support, utilizing the embo-01 model with 1536 dimensions and supporting db and query embedding types.
  • LLM Inference (Genie): The MiniMaxGenie class offers OpenAI-compatible chat completions via the openai SDK, supporting models like MiniMax-M2.7 and MiniMax-M2.5, with temperature clamping and automatic <think> tag stripping.
  • Comprehensive Testing: The changes include extensive unit and integration tests for both embeddings and Genie, ensuring the stability and reliability of the new integration.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Add MiniMaxEmbeddings using MiniMax's native embedding API (embo-01, 1536 dims)
with db/query type distinction for asymmetric retrieval. Also add MiniMaxGenie
for LLM inference via OpenAI-compatible API (M2.5, M2.5-highspeed) with
temperature clamping and think-tag stripping.

- MiniMaxEmbeddings: direct httpx REST client, batch support, async support
- MiniMaxGenie: OpenAI-compat SDK, JSON mode, structured output
- Registry: minimax:// provider URI, embo-* pattern matching
- 58 unit tests (31 embedding + 27 genie), 6 integration tests
- Documentation and overview page updates

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for MiniMax as a provider for both embeddings (MiniMaxEmbeddings) and LLM inference (MiniMaxGenie). The changes include the implementation of the two new classes, their registration, and comprehensive unit and integration tests. The code is well-structured and follows existing patterns in the repository. I have one suggestion for MiniMaxEmbeddings to improve the performance of asynchronous operations by reusing the httpx.AsyncClient instance. Otherwise, the implementation looks great.

Comment on lines +162 to +187
List of embedding vectors (each a list of floats).

Raises:
ValueError: If the API returns an error.

"""
async with httpx.AsyncClient(timeout=self._timeout) as client:
response = await client.post(
_MINIMAX_EMBEDDINGS_URL,
headers={
"Authorization": f"Bearer {self.api_key}",
"Content-Type": "application/json",
},
json={
"model": self.model,
"texts": texts,
"type": self.embedding_type,
},
)
response.raise_for_status()
data = response.json()

base_resp = data.get("base_resp", {})
if base_resp.get("status_code", 0) != 0:
raise ValueError(
f"MiniMax API error: {base_resp.get('status_msg', 'unknown error')}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

For better performance, the httpx.AsyncClient should be instantiated once in the __init__ method and reused across all asynchronous calls, rather than being created for each call within _acall_api. This avoids the overhead of setting up a new client and connection pool for every API request, which is especially important for batch operations.

This pattern is already used for the synchronous httpx.Client and is also consistent with how the MiniMaxGenie class handles its AsyncOpenAI client.

You should add self._async_client = httpx.AsyncClient(timeout=self._timeout) to __init__ and then use self._async_client here.

        response = await self._async_client.post(
            _MINIMAX_EMBEDDINGS_URL,
            headers={
                "Authorization": f"Bearer {self.api_key}",
                "Content-Type": "application/json",
            },
            json={
                "model": self.model,
                "texts": texts,
                "type": self.embedding_type,
            },
        )
        response.raise_for_status()
        data = response.json()

        base_resp = data.get("base_resp", {})
        if base_resp.get("status_code", 0) != 0:
            raise ValueError(
                f"MiniMax API error: {base_resp.get('status_msg', 'unknown error')}"
            )

        vectors = data.get("vectors")
        if vectors is None:
            raise ValueError("MiniMax API response missing 'vectors' field")
        return vectors

@octo-patch octo-patch changed the title feat: add MiniMax as embedding and LLM (Genie) provider feat: add MiniMax as embedding and LLM (genie) provider Mar 22, 2026
PR Bot and others added 2 commits March 26, 2026 21:43
MiniMax does not have a public embedding API. Remove the minimax-embeddings
doc page and the MiniMax entry from the embeddings overview.
- Add MiniMax-M3 to model list and set as default
- Keep MiniMax-M2.7 and MiniMax-M2.7-highspeed
- Remove older models (M2.5/M2.5-highspeed)
- Update related tests
@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 780ab57f-284f-4121-8883-76ee5187961d

📥 Commits

Reviewing files that changed from the base of the PR and between 68c4f56 and 56c1a9c.

📒 Files selected for processing (2)
  • src/chonkie/embeddings/minimax.py
  • tests/embeddings/test_minimax_embeddings.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/embeddings/test_minimax_embeddings.py
  • src/chonkie/embeddings/minimax.py

📝 Walkthrough

Walkthrough

This PR introduces MiniMax as a new embeddings and LLM provider: adds MiniMaxEmbeddings (sync/async, batching, retries, tokenizer/dimension, resource management) and MiniMaxGenie (sync/async OpenAI-compatible generation, JSON schema injection, think-tag stripping, retries), wires them into exports/registry, and adds comprehensive tests and a pyproject optional extra.

Changes

MiniMax Provider Integration

Layer / File(s) Summary
MiniMax Embeddings Implementation
src/chonkie/embeddings/minimax.py
MiniMaxEmbeddings subclasses BaseEmbeddings to call MiniMax's /v1/embeddings endpoint with retry/backoff logic, supports both sync (embed, embed_batch) and async (aembed, aembed_batch) embedding, validates embedding_type and API key, batches input by configurable size, returns numpy.float32 arrays, exposes dimension and get_tokenizer(), and includes availability/repr helpers.
MiniMax Genie Implementation
src/chonkie/genie/minimax.py
MiniMaxGenie subclasses BaseGenie to call MiniMax's OpenAI-compatible chat-completions API with sync (generate, generate_json) and async (agenerate, agenerate_json) methods, validates pydantic and openai availability, requires API key, clamps temperature, injects JSON schemas into prompts, removes <think>...</think> blocks from outputs, applies tenacity retry/backoff on rate-limit/timeout/API errors, and includes availability/repr helpers.
Package Integration and Registry Wiring
src/chonkie/embeddings/__init__.py, src/chonkie/embeddings/registry.py, src/chonkie/genie/__init__.py, src/chonkie/__init__.py, pyproject.toml
Module __all__ exports expose MiniMaxEmbeddings and MiniMaxGenie; embeddings registry registers the minimax provider, ^embo- pattern, and embo-01 model; top-level __init__.py re-exports both classes; pyproject.toml defines the minimax optional extra and includes it in the all aggregate.
Embeddings Test Suite
tests/embeddings/test_minimax_embeddings.py
Fixtures mock HTTP responses and client; unit tests validate import, construction, error handling (missing key/dependency, invalid type), single/batch embedding, batching/chunking logic, dimension, tokenizer, similarity, __repr__, callability, and API error cases; registry tests verify provider/pattern resolution; integration tests call the real API when MINIMAX_API_KEY is set, marked xfail for rate-limit sensitivity.
Genie Test Suite
tests/genie/test_minimax_genie.py
Fixtures and mocks for OpenAI clients; unit tests validate import, BaseGenie inheritance, error handling (missing key/dependencies), text/JSON generation with mocked responses, <think> tag stripping (text and JSON), batch generation, temperature defaults/clamping/propagation, think-tag utility edge cases, availability detection, __repr__, and base URL wiring; integration tests call the real API when MINIMAX_API_KEY is set.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Soft paws on code tonight,
I nibble bugs and stitch the light,
MiniMax hops into the stack,
embeddings hum and genie's back,
I thump for tests — all checks pass bright.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding MiniMax as both an embedding provider and LLM (genie) provider with M3 as the default model, which aligns with the substantial code additions across embeddings and genie modules.
Docstring Coverage ✅ Passed Docstring coverage is 97.85% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch feature/add-minimax-provider

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@octo-patch octo-patch changed the title feat: add MiniMax as embedding and LLM (genie) provider feat: add MiniMax as embedding and LLM (genie) provider with M3 as default Jun 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)
tests/embeddings/test_minimax_embeddings.py (1)

1-371: ⚠️ Potential issue | 🟠 Major

Add test coverage for MiniMaxEmbeddings async methods and tenacity retry behavior
MiniMaxEmbeddings implements aembed/aembed_batch and applies tenacity retry to both _call_api and _acall_api (retrying on httpx.HTTPStatusError, httpx.ConnectError, httpx.TimeoutException), but tests/embeddings/test_minimax_embeddings.py only covers synchronous embed/embed_batch paths and doesn’t assert any retry behavior. Add pytest.mark.asyncio unit tests for aembed/aembed_batch, plus tests that force retryable exceptions and verify the number of attempts / eventual success.

🤖 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/embeddings/test_minimax_embeddings.py` around lines 1 - 371, Add async
unit tests that cover MiniMaxEmbeddings.aembed and aembed_batch and verify
tenacity retry behavior: create pytest.mark.asyncio tests that use a mocked
async httpx client (patch MiniMaxEmbeddings to use a mock _client) and call
aembed/aembed_batch; assert they return proper numpy arrays like
embed/embed_batch; also add tests that make the mock client's async post raise
httpx.HTTPStatusError, httpx.ConnectError, or httpx.TimeoutException for the
first N-1 calls and succeed on the Nth call to exercise retries on
_acall_api/_call_api, asserting the number of attempts (mock post call_count or
side_effect sequence) and final success. Ensure tests reference
MiniMaxEmbeddings.aembed, MiniMaxEmbeddings.aembed_batch, and the retry-wrapped
internal methods (_acall_api/_call_api) so reviewers can locate the code.
🧹 Nitpick comments (6)
tests/embeddings/test_minimax_embeddings.py (1)

38-46: 💤 Low value

Optional: Simplify the fixture by removing redundant _client assignment.

Line 45 replaces _client after construction, but the patch context on line 42 already ensures mock_client is returned when MiniMaxEmbeddings.__init__ calls httpx.Client(timeout=...). The manual replacement is redundant.

♻️ Simplify by removing line 45
 `@pytest.fixture`
 def embedding_model(mock_api_response):
     """Create a MiniMaxEmbeddings instance with a mocked httpx client."""
     mock_client = MagicMock()
     mock_client.post.return_value = mock_api_response
     with patch("httpx.Client", return_value=mock_client):
         model = MiniMaxEmbeddings(api_key="test-minimax-key")
-    # Replace the real client with our mock
-    model._client = mock_client
     return model
🤖 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/embeddings/test_minimax_embeddings.py` around lines 38 - 46, The
fixture embedding_model currently patches httpx.Client to return mock_client, so
when MiniMaxEmbeddings(api_key="test-minimax-key") is constructed it already
receives the mocked client; remove the redundant manual assignment model._client
= mock_client (referencing the embedding_model function, mock_client variable,
and MiniMaxEmbeddings class) to simplify the fixture and rely solely on the
patch context.
tests/genie/test_minimax_genie.py (1)

19-26: ⚡ Quick win

Consider adding functional tests for async methods.

The test suite verifies that agenerate, agenerate_json, and generate_json_batch methods exist but does not functionally test them. Adding tests for these methods would improve coverage and catch potential issues in async code paths.

🤖 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/genie/test_minimax_genie.py` around lines 19 - 26, Add functional async
tests that exercise MiniMaxGenie's asynchronous code paths: create async test
functions (using pytest.mark.asyncio or asyncio.run) that instantiate
MiniMaxGenie and call agenerate with a simple prompt and assert the returned
result shape/type, call agenerate_json and assert the JSON-parsed structure and
expected keys, and call generate_json_batch with a small batch input and assert
each item in the returned batch has the expected structure; reference the
methods agenerate, agenerate_json, and generate_json_batch on the MiniMaxGenie
class and include minimal assertions (types/keys/status) to verify correct async
behavior.
src/chonkie/embeddings/minimax.py (1)

168-168: ⚖️ Poor tradeoff

Async method creates new client per call, unlike sync pattern.

The async _acall_api creates a new AsyncClient for each call (line 168), while the sync version reuses self._client. This is safer for resource management but incurs connection overhead per call. For batch operations via aembed_batch, this means N/batch_size new connections.

Consider storing self._async_client and implementing async cleanup, similar to the sync pattern, for better performance on repeated async calls.

🤖 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/chonkie/embeddings/minimax.py` at line 168, The async method _acall_api
currently creates a new httpx.AsyncClient per call which causes connection
overhead (especially in aembed_batch); change the implementation to reuse a
single AsyncClient by adding self._async_client (parallel to the sync
self._client), initialize it lazily, use it inside _acall_api instead of
creating a new client each call, and add an async cleanup method (e.g., async
close_async_client or implement __aenter__/__aexit__ on the class) to close
self._async_client when the service is shut down or disposed to match the sync
resource-management pattern.
src/chonkie/genie/minimax.py (3)

100-102: 💤 Low value

Both sync and async clients created but async client may not be cleaned up.

Lines 101-102 create both self.client and self.async_client. Unlike the embeddings class, these clients are never explicitly closed. OpenAI clients typically handle cleanup internally, but for consistency and to avoid potential resource leaks with long-lived instances, consider adding a close method.

🤖 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/chonkie/genie/minimax.py` around lines 100 - 102, The code creates both
OpenAI and AsyncOpenAI clients (self.client and self.async_client) but never
closes them; add cleanup by implementing a close method on the class in
minimax.py that calls self.client.close() if present and asynchronously closes
the async client (await self.async_client.aclose() or await
self.async_client.close() depending on the library) inside a try/except to
handle missing attributes, and also provide an async_close/async def aclose
method (or __aenter__/__aexit__ handlers) so callers can properly await cleanup
of AsyncOpenAI; reference the OpenAI, AsyncOpenAI, self.client, and
self.async_client symbols when adding these methods.

95-96: 💤 Low value

Temperature silently clamped without user warning.

The temperature is clamped to [0.01, 1.0] (line 96), but if a user passes temperature=0, they silently get 0.01. This could surprise users expecting deterministic output. Consider logging a warning when clamping occurs.

Also, the docstring says (0, 1] but the code enforces [0.01, 1.0] — a minor documentation inconsistency.

🤖 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/chonkie/genie/minimax.py` around lines 95 - 96, The constructor currently
silently clamps the temperature via self.temperature = max(0.01,
min(temperature, 1.0)); update this to detect when the incoming temperature is
outside the allowed range and emit a warning (e.g., using the module logger or
warnings.warn) that reports the original value and the clamped value, then
assign the clamped value to self.temperature; also make the docstring for the
class or __init__ match the enforced range (change "(0, 1]" to "[0.01, 1.0]" or
adjust the clamp to match the documented "(0, 1]" behavior) so code and docs are
consistent.

35-36: 💤 Low value

Regex re.DOTALL flag is redundant.

The pattern [\s\S]*? already matches any character including newlines, making the re.DOTALL flag redundant. This is harmless but could be simplified.

🤖 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/chonkie/genie/minimax.py` around lines 35 - 36, The compiled regex
_THINK_TAG_RE uses [\s\S]*? which already matches newlines, so remove the
redundant re.DOTALL flag from the re.compile call; update the call that defines
_THINK_TAG_RE to just re.compile(r"<think>[\s\S]*?</think>\s*") (leaving the
pattern as-is) so behavior is unchanged but the flag is not used.
🤖 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/chonkie/embeddings/minimax.py`:
- Around line 99-107: The retry decorator currently retries all
httpx.HTTPStatusError instances; change it to retry only on transient HTTP
errors by replacing retry_if_exception_type((httpx.HTTPStatusError,
httpx.ConnectError, httpx.TimeoutException)) with a custom predicate (use
tenacity.retry_if_exception with a lambda or function) that: for
httpx.HTTPStatusError inspects exc.response.status_code and returns True only
for 429 or 5xx (>=500), and for httpx.ConnectError or httpx.TimeoutException
returns True; keep stop_after_attempt and wait_exponential as-is so the
backoff/retry targets only transient (429/5xx) HTTP failures while still
retrying connection/timeout errors.
- Line 95: The MiniMaxEmbeddings class creates an httpx.Client instance stored
as self._client in the __init__ method but never closes it, causing resource
leaks. Add a __del__ method to the MiniMaxEmbeddings class that closes the
self._client when the instance is garbage collected, or alternatively implement
context manager methods (__enter__ and __exit__) to properly manage the client's
lifecycle. Ensure the client is explicitly closed by calling
self._client.close() to release any underlying network connections and
resources.

In `@tests/embeddings/test_minimax_embeddings.py`:
- Around line 269-280: The tests currently use pytest.raises(Exception) which is
too broad; update the assertions to expect the specific ValueError raised by
MiniMaxEmbeddings._call_api. Replace pytest.raises(Exception) with
pytest.raises(ValueError) in test_api_error_response (the block calling
embedding_model.embed("test") with the mocked API error response) and also in
the analogous test covering the missing "vectors" field (the test that calls
embedding_model.embed with a response lacking "vectors"). This ensures the tests
assert the correct exception type from MiniMaxEmbeddings._call_api.

---

Outside diff comments:
In `@tests/embeddings/test_minimax_embeddings.py`:
- Around line 1-371: Add async unit tests that cover MiniMaxEmbeddings.aembed
and aembed_batch and verify tenacity retry behavior: create pytest.mark.asyncio
tests that use a mocked async httpx client (patch MiniMaxEmbeddings to use a
mock _client) and call aembed/aembed_batch; assert they return proper numpy
arrays like embed/embed_batch; also add tests that make the mock client's async
post raise httpx.HTTPStatusError, httpx.ConnectError, or httpx.TimeoutException
for the first N-1 calls and succeed on the Nth call to exercise retries on
_acall_api/_call_api, asserting the number of attempts (mock post call_count or
side_effect sequence) and final success. Ensure tests reference
MiniMaxEmbeddings.aembed, MiniMaxEmbeddings.aembed_batch, and the retry-wrapped
internal methods (_acall_api/_call_api) so reviewers can locate the code.

---

Nitpick comments:
In `@src/chonkie/embeddings/minimax.py`:
- Line 168: The async method _acall_api currently creates a new
httpx.AsyncClient per call which causes connection overhead (especially in
aembed_batch); change the implementation to reuse a single AsyncClient by adding
self._async_client (parallel to the sync self._client), initialize it lazily,
use it inside _acall_api instead of creating a new client each call, and add an
async cleanup method (e.g., async close_async_client or implement
__aenter__/__aexit__ on the class) to close self._async_client when the service
is shut down or disposed to match the sync resource-management pattern.

In `@src/chonkie/genie/minimax.py`:
- Around line 100-102: The code creates both OpenAI and AsyncOpenAI clients
(self.client and self.async_client) but never closes them; add cleanup by
implementing a close method on the class in minimax.py that calls
self.client.close() if present and asynchronously closes the async client (await
self.async_client.aclose() or await self.async_client.close() depending on the
library) inside a try/except to handle missing attributes, and also provide an
async_close/async def aclose method (or __aenter__/__aexit__ handlers) so
callers can properly await cleanup of AsyncOpenAI; reference the OpenAI,
AsyncOpenAI, self.client, and self.async_client symbols when adding these
methods.
- Around line 95-96: The constructor currently silently clamps the temperature
via self.temperature = max(0.01, min(temperature, 1.0)); update this to detect
when the incoming temperature is outside the allowed range and emit a warning
(e.g., using the module logger or warnings.warn) that reports the original value
and the clamped value, then assign the clamped value to self.temperature; also
make the docstring for the class or __init__ match the enforced range (change
"(0, 1]" to "[0.01, 1.0]" or adjust the clamp to match the documented "(0, 1]"
behavior) so code and docs are consistent.
- Around line 35-36: The compiled regex _THINK_TAG_RE uses [\s\S]*? which
already matches newlines, so remove the redundant re.DOTALL flag from the
re.compile call; update the call that defines _THINK_TAG_RE to just
re.compile(r"<think>[\s\S]*?</think>\s*") (leaving the pattern as-is) so
behavior is unchanged but the flag is not used.

In `@tests/embeddings/test_minimax_embeddings.py`:
- Around line 38-46: The fixture embedding_model currently patches httpx.Client
to return mock_client, so when MiniMaxEmbeddings(api_key="test-minimax-key") is
constructed it already receives the mocked client; remove the redundant manual
assignment model._client = mock_client (referencing the embedding_model
function, mock_client variable, and MiniMaxEmbeddings class) to simplify the
fixture and rely solely on the patch context.

In `@tests/genie/test_minimax_genie.py`:
- Around line 19-26: Add functional async tests that exercise MiniMaxGenie's
asynchronous code paths: create async test functions (using pytest.mark.asyncio
or asyncio.run) that instantiate MiniMaxGenie and call agenerate with a simple
prompt and assert the returned result shape/type, call agenerate_json and assert
the JSON-parsed structure and expected keys, and call generate_json_batch with a
small batch input and assert each item in the returned batch has the expected
structure; reference the methods agenerate, agenerate_json, and
generate_json_batch on the MiniMaxGenie class and include minimal assertions
(types/keys/status) to verify correct async behavior.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5539d8fe-9e25-4673-aa26-1480e224c023

📥 Commits

Reviewing files that changed from the base of the PR and between 30d75f4 and 68c4f56.

📒 Files selected for processing (9)
  • pyproject.toml
  • src/chonkie/__init__.py
  • src/chonkie/embeddings/__init__.py
  • src/chonkie/embeddings/minimax.py
  • src/chonkie/embeddings/registry.py
  • src/chonkie/genie/__init__.py
  • src/chonkie/genie/minimax.py
  • tests/embeddings/test_minimax_embeddings.py
  • tests/genie/test_minimax_genie.py

Comment thread src/chonkie/embeddings/minimax.py
Comment thread src/chonkie/embeddings/minimax.py
Comment thread tests/embeddings/test_minimax_embeddings.py
- Add close()/__enter__/__exit__/__del__ to release httpx.Client pool
- Restrict retry to transient errors only (ConnectError, TimeoutException,
  5xx HTTPStatusError) — no more retrying 4xx like 401/422
- Tighten test_api_error_response and test_missing_vectors_field to
  pytest.raises(ValueError, match=...) instead of generic Exception

Co-Authored-By: Octopus <liyuan851277048@icloud.com>
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.

1 participant