feat: add MiniMax as first-class LLM provider (default: M2.7)#211
Open
octo-patch wants to merge 4 commits intoxming521:masterfrom
Open
feat: add MiniMax as first-class LLM provider (default: M2.7)#211octo-patch wants to merge 4 commits intoxming521:masterfrom
octo-patch wants to merge 4 commits intoxming521:masterfrom
Conversation
- Add LLMProvider enum and preset system (OpenAI, DeepSeek, MiniMax, Custom) that auto-fills base_url and model_name in MakeDatasetArgs - Auto-detect MiniMax endpoints and disable response_format (unsupported) - Clamp temperature to 0.01 for MiniMax (rejects zero) - Support both api.minimax.io and api.minimaxi.com (China) endpoints - Update config templates with llm_provider documentation - Add 25 unit tests + 4 integration tests Co-Authored-By: Octopus <liyuan851277048@icloud.com>
Reviewer's Guide通过在配置中添加提供者预设、在在线推理客户端中加入 MiniMax 特定行为、更新文档与模板，以及新增专门的 MiniMax 测试套件，将 MiniMax 作为数据清洗流水线的一等在线 LLM 提供者集成进来。 基于 MiniMax 的在线数据清洗调用时序图sequenceDiagram
actor User
participant WeCloneCLI
participant MakeDatasetArgs
participant OnlineLLM
participant MiniMaxAPI
User->>WeCloneCLI: Run make-dataset
WeCloneCLI->>MakeDatasetArgs: Load settings.jsonc
MakeDatasetArgs->>MakeDatasetArgs: apply_provider_presets()
Note over MakeDatasetArgs: llm_provider = minimax
MakeDatasetArgs-->MakeDatasetArgs: base_url = https://api.minimax.io/v1
MakeDatasetArgs-->MakeDatasetArgs: model_name = MiniMax-M2.5
WeCloneCLI->>OnlineLLM: __init__(api_key, base_url, model_name, response_format)
OnlineLLM->>OnlineLLM: _is_no_response_format_provider(base_url)
OnlineLLM-->OnlineLLM: _supports_response_format = False
OnlineLLM-->OnlineLLM: response_format = ""
WeCloneCLI->>OnlineLLM: chat(prompt_text, system_prompt, temperature=0)
OnlineLLM->>OnlineLLM: clamp_temperature(temperature, base_url)
OnlineLLM-->OnlineLLM: temperature = 0.01
OnlineLLM->>MiniMaxAPI: POST /chat with model, messages, temperature=0.01
MiniMaxAPI-->>OnlineLLM: JSON response
OnlineLLM-->>WeCloneCLI: cleaned text
WeCloneCLI-->>User: Dataset with cleaned data
LLM 提供者预设与 OnlineLLM 行为的类图classDiagram
class LLMProvider {
<<enumeration>>
OPENAI
DEEPSEEK
MINIMAX
CUSTOM
}
class MakeDatasetArgs {
+llm_provider LLMProvider
+base_url str
+llm_api_key str
+model_name str
+apply_provider_presets() MakeDatasetArgs
}
class LLMProviderPresets {
+OPENAI_base_url str
+OPENAI_model_name str
+DEEPSEEK_base_url str
+DEEPSEEK_model_name str
+MINIMAX_base_url str
+MINIMAX_model_name str
}
class OnlineLLM {
-_NO_RESPONSE_FORMAT_PROVIDERS set
+api_key str
+base_url str
+model_name str
+response_format str
+prompt_with_system bool
+__init__(api_key str, base_url str, model_name str, max_workers int, prompt_with_system bool, response_format str)
+chat(prompt_text str, system_prompt str, temperature float, max_tokens int) dict
+_is_no_response_format_provider(base_url str) bool
+clamp_temperature(temperature float, base_url str) float
}
LLMProviderPresets ..> LLMProvider : keys
MakeDatasetArgs --> LLMProvider : uses
MakeDatasetArgs ..> LLMProviderPresets : reads
OnlineLLM ..> LLMProviderPresets : behavior aligned_with
文件级改动
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideAdds MiniMax as a first-class online LLM provider for the data cleaning pipeline via provider presets in configuration, MiniMax-specific behavior in the online inference client, updated docs/templates, and a dedicated MiniMax test suite. Sequence diagram for MiniMax-based online data cleaning callsequenceDiagram
actor User
participant WeCloneCLI
participant MakeDatasetArgs
participant OnlineLLM
participant MiniMaxAPI
User->>WeCloneCLI: Run make-dataset
WeCloneCLI->>MakeDatasetArgs: Load settings.jsonc
MakeDatasetArgs->>MakeDatasetArgs: apply_provider_presets()
Note over MakeDatasetArgs: llm_provider = minimax
MakeDatasetArgs-->MakeDatasetArgs: base_url = https://api.minimax.io/v1
MakeDatasetArgs-->MakeDatasetArgs: model_name = MiniMax-M2.5
WeCloneCLI->>OnlineLLM: __init__(api_key, base_url, model_name, response_format)
OnlineLLM->>OnlineLLM: _is_no_response_format_provider(base_url)
OnlineLLM-->OnlineLLM: _supports_response_format = False
OnlineLLM-->OnlineLLM: response_format = ""
WeCloneCLI->>OnlineLLM: chat(prompt_text, system_prompt, temperature=0)
OnlineLLM->>OnlineLLM: clamp_temperature(temperature, base_url)
OnlineLLM-->OnlineLLM: temperature = 0.01
OnlineLLM->>MiniMaxAPI: POST /chat with model, messages, temperature=0.01
MiniMaxAPI-->>OnlineLLM: JSON response
OnlineLLM-->>WeCloneCLI: cleaned text
WeCloneCLI-->>User: Dataset with cleaned data
Class diagram for LLM provider presets and OnlineLLM behaviorclassDiagram
class LLMProvider {
<<enumeration>>
OPENAI
DEEPSEEK
MINIMAX
CUSTOM
}
class MakeDatasetArgs {
+llm_provider LLMProvider
+base_url str
+llm_api_key str
+model_name str
+apply_provider_presets() MakeDatasetArgs
}
class LLMProviderPresets {
+OPENAI_base_url str
+OPENAI_model_name str
+DEEPSEEK_base_url str
+DEEPSEEK_model_name str
+MINIMAX_base_url str
+MINIMAX_model_name str
}
class OnlineLLM {
-_NO_RESPONSE_FORMAT_PROVIDERS set
+api_key str
+base_url str
+model_name str
+response_format str
+prompt_with_system bool
+__init__(api_key str, base_url str, model_name str, max_workers int, prompt_with_system bool, response_format str)
+chat(prompt_text str, system_prompt str, temperature float, max_tokens int) dict
+_is_no_response_format_provider(base_url str) bool
+clamp_temperature(temperature float, base_url str) float
}
LLMProviderPresets ..> LLMProvider : keys
MakeDatasetArgs --> LLMProvider : uses
MakeDatasetArgs ..> LLMProviderPresets : reads
OnlineLLM ..> LLMProviderPresets : behavior aligned_with
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 5 个问题，并留下了一些整体性的反馈：
OnlineLLM中的提供商检测逻辑依赖对base_url的子串匹配来触发 MiniMax 特定行为；建议把「提供商能力」集中管理（例如通过LLMProvider/ 预设，或者一个小的能力映射表），这样response_format支持与 temperature 限制就可以由显式的 provider 标志驱动，而不是通过解析 URL。_NO_RESPONSE_FORMAT_PROVIDERS常量现在也被用于控制 temperature 限制，这比名字所暗示的职责更宽；可以考虑重命名它，或者单独引入一个用于 temperature 约束的常量，以让这些检查的意图更清晰，并避免把彼此无关的行为耦合在一起。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The provider detection logic in `OnlineLLM` relies on substring matching against `base_url` for MiniMax-specific behavior; consider centralizing provider capabilities (e.g., via `LLMProvider`/presets or a small capability map) so response_format support and temperature clamping are driven by an explicit provider flag instead of URL parsing.
- The `_NO_RESPONSE_FORMAT_PROVIDERS` constant is now also used to control temperature clamping, which is broader than its name suggests; renaming it or introducing a separate constant for temperature constraints would make the intent of these checks clearer and avoid coupling unrelated behaviors.
## Individual Comments
### Comment 1
<location path="weclone/utils/config_models.py" line_range="198-207" />
<code_context>
clean_batch_size: int = Field(10, description="Batch size for data cleaning")
vision_api: VisionApiConfig = Field(VisionApiConfig())
+ @model_validator(mode="after")
+ def apply_provider_presets(self):
+ """Apply provider presets when llm_provider is set."""
+ if self.llm_provider and self.llm_provider in LLM_PROVIDER_PRESETS:
+ preset = LLM_PROVIDER_PRESETS[self.llm_provider]
+ if not self.base_url:
+ self.base_url = preset["base_url"]
+ if not self.model_name:
+ self.model_name = preset["model_name"]
+ return self
+
</code_context>
<issue_to_address>
**suggestion:** Defaulting based on truthiness may unintentionally override explicitly provided but falsy values.
Using `if not self.base_url` / `if not self.model_name` will treat empty strings as missing and replace them with preset values. If callers may intentionally pass an empty string (e.g. to disable a default), that intent is lost. To only apply presets when the fields are unset, check explicitly for `None` (e.g. `if self.base_url is None:`).
Suggested implementation:
```python
@model_validator(mode="after")
def apply_provider_presets(self):
"""Apply provider presets when llm_provider is set."""
if self.llm_provider and self.llm_provider in LLM_PROVIDER_PRESETS:
preset = LLM_PROVIDER_PRESETS[self.llm_provider]
# Only apply defaults when the fields are truly unset (None),
# so that falsy-but-intentional values like "" are respected.
if self.base_url is None:
self.base_url = preset["base_url"]
if self.model_name is None:
self.model_name = preset["model_name"]
return self
```
None required, assuming `model_validator` and `LLM_PROVIDER_PRESETS` are already correctly imported/defined elsewhere in this file.
</issue_to_address>
### Comment 2
<location path="weclone/core/inference/online_infer.py" line_range="50-58" />
<code_context>
+ return any(host in base_url_lower for host in OnlineLLM._NO_RESPONSE_FORMAT_PROVIDERS)
+
+ @staticmethod
+ def clamp_temperature(temperature: float, base_url: str) -> float:
+ """Clamp temperature for providers that require it to be in (0.0, 1.0].
+
+ MiniMax API rejects temperature=0; use a small positive value instead.
+ """
+ if OnlineLLM._is_no_response_format_provider(base_url) and temperature <= 0:
+ return 0.01
+ return temperature
@retry_openai_api(max_retries=200, base_delay=30.0, max_delay=180.0)
</code_context>
<issue_to_address>
**suggestion:** Clamp temperature on the upper bound as well to fully satisfy MiniMax range requirements.
The docstring says this helper enforces `(0.0, 1.0]` for MiniMax, but it only adjusts `temperature <= 0` and leaves values above `1.0` unchanged. If a caller passes `temperature=1.5`, MiniMax could still reject the request. To fully enforce the documented contract, clamp both ends for MiniMax-like providers, e.g. `return min(max(temperature, 0.01), 1.0)`, while leaving other providers unchanged.
```suggestion
@staticmethod
def clamp_temperature(temperature: float, base_url: str) -> float:
"""Clamp temperature for providers that require it to be in (0.0, 1.0].
For MiniMax-like providers:
- MiniMax API rejects temperature=0; use a small positive value instead.
- Values above 1.0 are also rejected; clamp to 1.0.
Other providers are left unchanged.
"""
if OnlineLLM._is_no_response_format_provider(base_url):
return min(max(temperature, 0.01), 1.0)
return temperature
```
</issue_to_address>
### Comment 3
<location path="tests/test_minimax_provider.py" line_range="145-146" />
<code_context>
+# Unit Tests – Temperature clamping
+# ---------------------------------------------------------------------------
+
+class TestTemperatureClamping:
+ """Test OnlineLLM.clamp_temperature for MiniMax constraints."""
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a temperature clamping test for `base_url=None`/empty to document passthrough behavior
Current clamping tests cover MiniMax, OpenAI, the China endpoint, and `_is_no_response_format_provider` for empty strings. Please also add a test asserting that `OnlineLLM.clamp_temperature(0.0, None)` (and/or `""`) returns `0.0`. This will document and lock in the intended passthrough behavior when the provider cannot be detected from the URL and protect against future changes to `_is_no_response_format_provider` altering this behavior inadvertently.
```suggestion
class TestTemperatureClamping:
"""Test OnlineLLM.clamp_temperature for MiniMax constraints."""
def test_clamp_temperature_passthrough_when_base_url_none_or_empty(self):
"""If provider cannot be detected from base_url, temperature is passed through."""
# base_url is None – should be a no-op passthrough
assert OnlineLLM.clamp_temperature(0.0, None) == 0.0
# base_url is empty string – should also be a no-op passthrough
assert OnlineLLM.clamp_temperature(0.0, "") == 0.0
```
</issue_to_address>
### Comment 4
<location path="tests/test_minimax_provider.py" line_range="180" />
<code_context>
+class TestResponseFormatHandling:
+ """Test that response_format is disabled for MiniMax."""
+
+ @patch("weclone.core.inference.online_infer.OpenAI")
+ def test_minimax_disables_response_format(self, mock_openai_cls):
+ llm = OnlineLLM(
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test that MiniMax overrides an explicitly provided `response_format`
`TestResponseFormatHandling` only covers the default `response_format` case for MiniMax. Please also add a test for the case where a caller explicitly passes a non-empty `response_format` while targeting MiniMax. For example, instantiate `OnlineLLM(..., base_url="https://api.minimax.io/v1", response_format="json_object")` and assert that `llm._supports_response_format is False` and `llm.response_format == ""` to verify `OnlineLLM.__init__` correctly ignores the explicit value when response formats aren’t supported.
</issue_to_address>
### Comment 5
<location path="tests/test_minimax_provider.py" line_range="223-227" />
<code_context>
+ )
+ llm.chat("Hello")
+
+ call_kwargs = mock_client.chat.completions.create.call_args
+ # response_format should NOT be in the call params
+ assert "response_format" not in call_kwargs.kwargs and (
</code_context>
<issue_to_address>
**suggestion (testing):** Simplify and harden the assertion that `response_format` is omitted for MiniMax
The current assertion inspects both `call_kwargs.kwargs` and the string representation of `call_kwargs`, which is brittle and may yield false positives if that representation changes. Since `call_args` is an `(args, kwargs)` tuple, you can simplify to:
```python
_, kwargs = mock_client.chat.completions.create.call_args
assert "response_format" not in kwargs
```
This keeps the test focused on the actual keyword arguments passed to the client and avoids reliance on `unittest.mock` internals.
```suggestion
_, kwargs = mock_client.chat.completions.create.call_args
# response_format should NOT be in the call params
assert "response_format" not in kwargs
```
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 5 issues, and left some high level feedback:
- The provider detection logic in
OnlineLLMrelies on substring matching againstbase_urlfor MiniMax-specific behavior; consider centralizing provider capabilities (e.g., viaLLMProvider/presets or a small capability map) so response_format support and temperature clamping are driven by an explicit provider flag instead of URL parsing. - The
_NO_RESPONSE_FORMAT_PROVIDERSconstant is now also used to control temperature clamping, which is broader than its name suggests; renaming it or introducing a separate constant for temperature constraints would make the intent of these checks clearer and avoid coupling unrelated behaviors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The provider detection logic in `OnlineLLM` relies on substring matching against `base_url` for MiniMax-specific behavior; consider centralizing provider capabilities (e.g., via `LLMProvider`/presets or a small capability map) so response_format support and temperature clamping are driven by an explicit provider flag instead of URL parsing.
- The `_NO_RESPONSE_FORMAT_PROVIDERS` constant is now also used to control temperature clamping, which is broader than its name suggests; renaming it or introducing a separate constant for temperature constraints would make the intent of these checks clearer and avoid coupling unrelated behaviors.
## Individual Comments
### Comment 1
<location path="weclone/utils/config_models.py" line_range="198-207" />
<code_context>
clean_batch_size: int = Field(10, description="Batch size for data cleaning")
vision_api: VisionApiConfig = Field(VisionApiConfig())
+ @model_validator(mode="after")
+ def apply_provider_presets(self):
+ """Apply provider presets when llm_provider is set."""
+ if self.llm_provider and self.llm_provider in LLM_PROVIDER_PRESETS:
+ preset = LLM_PROVIDER_PRESETS[self.llm_provider]
+ if not self.base_url:
+ self.base_url = preset["base_url"]
+ if not self.model_name:
+ self.model_name = preset["model_name"]
+ return self
+
</code_context>
<issue_to_address>
**suggestion:** Defaulting based on truthiness may unintentionally override explicitly provided but falsy values.
Using `if not self.base_url` / `if not self.model_name` will treat empty strings as missing and replace them with preset values. If callers may intentionally pass an empty string (e.g. to disable a default), that intent is lost. To only apply presets when the fields are unset, check explicitly for `None` (e.g. `if self.base_url is None:`).
Suggested implementation:
```python
@model_validator(mode="after")
def apply_provider_presets(self):
"""Apply provider presets when llm_provider is set."""
if self.llm_provider and self.llm_provider in LLM_PROVIDER_PRESETS:
preset = LLM_PROVIDER_PRESETS[self.llm_provider]
# Only apply defaults when the fields are truly unset (None),
# so that falsy-but-intentional values like "" are respected.
if self.base_url is None:
self.base_url = preset["base_url"]
if self.model_name is None:
self.model_name = preset["model_name"]
return self
```
None required, assuming `model_validator` and `LLM_PROVIDER_PRESETS` are already correctly imported/defined elsewhere in this file.
</issue_to_address>
### Comment 2
<location path="weclone/core/inference/online_infer.py" line_range="50-58" />
<code_context>
+ return any(host in base_url_lower for host in OnlineLLM._NO_RESPONSE_FORMAT_PROVIDERS)
+
+ @staticmethod
+ def clamp_temperature(temperature: float, base_url: str) -> float:
+ """Clamp temperature for providers that require it to be in (0.0, 1.0].
+
+ MiniMax API rejects temperature=0; use a small positive value instead.
+ """
+ if OnlineLLM._is_no_response_format_provider(base_url) and temperature <= 0:
+ return 0.01
+ return temperature
@retry_openai_api(max_retries=200, base_delay=30.0, max_delay=180.0)
</code_context>
<issue_to_address>
**suggestion:** Clamp temperature on the upper bound as well to fully satisfy MiniMax range requirements.
The docstring says this helper enforces `(0.0, 1.0]` for MiniMax, but it only adjusts `temperature <= 0` and leaves values above `1.0` unchanged. If a caller passes `temperature=1.5`, MiniMax could still reject the request. To fully enforce the documented contract, clamp both ends for MiniMax-like providers, e.g. `return min(max(temperature, 0.01), 1.0)`, while leaving other providers unchanged.
```suggestion
@staticmethod
def clamp_temperature(temperature: float, base_url: str) -> float:
"""Clamp temperature for providers that require it to be in (0.0, 1.0].
For MiniMax-like providers:
- MiniMax API rejects temperature=0; use a small positive value instead.
- Values above 1.0 are also rejected; clamp to 1.0.
Other providers are left unchanged.
"""
if OnlineLLM._is_no_response_format_provider(base_url):
return min(max(temperature, 0.01), 1.0)
return temperature
```
</issue_to_address>
### Comment 3
<location path="tests/test_minimax_provider.py" line_range="145-146" />
<code_context>
+# Unit Tests – Temperature clamping
+# ---------------------------------------------------------------------------
+
+class TestTemperatureClamping:
+ """Test OnlineLLM.clamp_temperature for MiniMax constraints."""
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a temperature clamping test for `base_url=None`/empty to document passthrough behavior
Current clamping tests cover MiniMax, OpenAI, the China endpoint, and `_is_no_response_format_provider` for empty strings. Please also add a test asserting that `OnlineLLM.clamp_temperature(0.0, None)` (and/or `""`) returns `0.0`. This will document and lock in the intended passthrough behavior when the provider cannot be detected from the URL and protect against future changes to `_is_no_response_format_provider` altering this behavior inadvertently.
```suggestion
class TestTemperatureClamping:
"""Test OnlineLLM.clamp_temperature for MiniMax constraints."""
def test_clamp_temperature_passthrough_when_base_url_none_or_empty(self):
"""If provider cannot be detected from base_url, temperature is passed through."""
# base_url is None – should be a no-op passthrough
assert OnlineLLM.clamp_temperature(0.0, None) == 0.0
# base_url is empty string – should also be a no-op passthrough
assert OnlineLLM.clamp_temperature(0.0, "") == 0.0
```
</issue_to_address>
### Comment 4
<location path="tests/test_minimax_provider.py" line_range="180" />
<code_context>
+class TestResponseFormatHandling:
+ """Test that response_format is disabled for MiniMax."""
+
+ @patch("weclone.core.inference.online_infer.OpenAI")
+ def test_minimax_disables_response_format(self, mock_openai_cls):
+ llm = OnlineLLM(
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test that MiniMax overrides an explicitly provided `response_format`
`TestResponseFormatHandling` only covers the default `response_format` case for MiniMax. Please also add a test for the case where a caller explicitly passes a non-empty `response_format` while targeting MiniMax. For example, instantiate `OnlineLLM(..., base_url="https://api.minimax.io/v1", response_format="json_object")` and assert that `llm._supports_response_format is False` and `llm.response_format == ""` to verify `OnlineLLM.__init__` correctly ignores the explicit value when response formats aren’t supported.
</issue_to_address>
### Comment 5
<location path="tests/test_minimax_provider.py" line_range="223-227" />
<code_context>
+ )
+ llm.chat("Hello")
+
+ call_kwargs = mock_client.chat.completions.create.call_args
+ # response_format should NOT be in the call params
+ assert "response_format" not in call_kwargs.kwargs and (
</code_context>
<issue_to_address>
**suggestion (testing):** Simplify and harden the assertion that `response_format` is omitted for MiniMax
The current assertion inspects both `call_kwargs.kwargs` and the string representation of `call_kwargs`, which is brittle and may yield false positives if that representation changes. Since `call_args` is an `(args, kwargs)` tuple, you can simplify to:
```python
_, kwargs = mock_client.chat.completions.create.call_args
assert "response_format" not in kwargs
```
This keeps the test focused on the actual keyword arguments passed to the client and avoids reliance on `unittest.mock` internals.
```suggestion
_, kwargs = mock_client.chat.completions.create.call_args
# response_format should NOT be in the call params
assert "response_format" not in kwargs
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
added 2 commits
March 17, 2026 14:11
- Use is None checks instead of truthiness for preset fields - Clamp MiniMax temperature upper bound to 1.0 (was only lower) - Add tests for None/empty base_url passthrough behavior - Add test for explicit response_format override on MiniMax - Simplify response_format assertion using call_args unpacking
- Update default model from MiniMax-M2.5 to MiniMax-M2.7 - Update all config templates, README docs, and tests - Keep all previous models as alternatives
octo-patch
changed the title
JiwaniZakir
reviewed
Apr 3, 2026
JiwaniZakir
left a comment
JiwaniZakir
left a comment
There was a problem hiding this comment.
The template files (settings.template.jsonc, mllm.template.jsonc, tg.template.jsonc) leave "base_url": "https://xxx/v1" as an active, non-commented field directly below the new llm_provider comment block. Since the docs say setting llm_provider auto-fills base_url, a user who enables "llm_provider": "minimax" but forgets to remove the explicit base_url placeholder will likely get a broken config — it's unclear from the template whether an explicit base_url takes precedence or whether the placeholder value causes an error. The placeholder should either be commented out or replaced with a note like // "base_url": "" // optional: overrides provider default.
Additionally, _extract_json_from_text in tests/test_minimax_provider.py is a local re-implementation rather than an import of the actual production function. This means the tests won't catch regressions if the real extract_json_from_text in offline_infer.py is changed — the unit tests will keep passing against stale logic. The mock setup already goes to lengths to isolate heavy dependencies; it would be cleaner to either import the real function directly (mocking only the transitive heavy imports that trigger on module load) or at minimum add a comment explaining why the divergence is intentional.
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
2 participants
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
Changes
Available Models
Testing