Compare commits

..

1 Commits

Author SHA1 Message Date
Yif Yang 340a4c870f fix(minimax): honour OPTIMIZER_DEPLOYMENT for optimizer-role calls
Follow-up to #116. MiniMax exposed only TARGET_DEPLOYMENT, and
set_optimizer_deployment() never touched MiniMax, so a configured
optimizer_model was ignored: chat_optimizer and the optimizer message path
both sent TARGET_DEPLOYMENT. In mixed optimizer/target setups this could send
the wrong model name to the optimizer endpoint.

Adds OPTIMIZER_DEPLOYMENT and set_optimizer_deployment() to minimax_backend,
wired into the model facade. chat_optimizer and a new chat_optimizer_messages
use it, falling back to TARGET_DEPLOYMENT when unset. Also fixes dict[int]
return annotations to dict[str, int]. Adds routing tests.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-07-13 06:35:38 +00:00
5 changed files with 106 additions and 28 deletions
+2 -1
View File
@@ -215,7 +215,7 @@ def chat_optimizer_messages(
timeout=timeout,
)
if get_optimizer_backend() == "minimax_chat":
return _minimax.chat_target_messages(
return _minimax.chat_optimizer_messages(
messages=messages,
max_completion_tokens=max_completion_tokens,
retries=retries,
@@ -540,3 +540,4 @@ def set_optimizer_deployment(deployment: str) -> None:
_openai.set_optimizer_deployment(deployment)
_claude.set_optimizer_deployment(deployment)
_qwen.set_optimizer_deployment(deployment)
_minimax.set_optimizer_deployment(deployment)
+42 -3
View File
@@ -36,6 +36,9 @@ TARGET_DEPLOYMENT = os.environ.get(
"TARGET_DEPLOYMENT",
default_model_for_backend("minimax_chat"),
)
# Optimizer role can point at a different MiniMax model than the target; falls
# back to TARGET_DEPLOYMENT when unset so single-model setups are unaffected.
OPTIMIZER_DEPLOYMENT = os.environ.get("OPTIMIZER_DEPLOYMENT", "")
_config_lock = threading.Lock()
tracker = TokenTracker()
@@ -222,7 +225,7 @@ def chat_target(
stage: str = "target",
reasoning_effort: str | None = None,
timeout: float | None = None,
) -> tuple[str, dict[int]]:
) -> tuple[str, dict[str, int]]:
del reasoning_effort
messages = [{"role": "system", "content": system}, {"role": "user", "content": user}]
return _chat_messages_impl(
@@ -242,7 +245,7 @@ def chat_optimizer(
stage: str = "optimizer",
reasoning_effort: str | None = None,
timeout: float | None = None,
) -> tuple[str, dict[int]]:
) -> tuple[str, dict[str, int]]:
"""Optimizer chat call. Backend stores the trained skill; uses the same
MiniMax-proxied OpenAI-compat endpoint as `chat_target`. Added in the
parallel-training fix; previously missing in skillopt 0.2.0's
@@ -256,6 +259,7 @@ def chat_optimizer(
max_completion_tokens,
retries,
stage,
deployment=OPTIMIZER_DEPLOYMENT or None,
timeout=timeout,
)
@@ -285,6 +289,35 @@ def chat_target_messages(
)
def chat_optimizer_messages(
messages: list[dict[str, Any]],
max_completion_tokens: int = 16384,
retries: int = 5,
stage: str = "optimizer",
reasoning_effort: str | None = None,
*,
tools: list[dict[str, Any]] | None = None,
tool_choice: str | dict[str, Any] | None = None,
return_message: bool = False,
timeout: float | None = None,
) -> tuple[Any, dict[str, int]]:
"""Optimizer-role message API. Same endpoint as ``chat_target_messages`` but
honours ``OPTIMIZER_DEPLOYMENT`` so optimizer and target can use distinct
MiniMax models; falls back to the target deployment when unset."""
del reasoning_effort
return _chat_messages_impl(
messages,
max_completion_tokens,
retries,
stage,
tools=tools,
tool_choice=tool_choice,
return_message=return_message,
deployment=OPTIMIZER_DEPLOYMENT or None,
timeout=timeout,
)
def get_token_summary() -> dict[str, dict[str, int]]:
return tracker.summary()
@@ -300,4 +333,10 @@ def set_reasoning_effort(effort: str | None) -> None:
def set_target_deployment(deployment: str) -> None:
global TARGET_DEPLOYMENT
TARGET_DEPLOYMENT = deployment or default_model_for_backend("minimax_chat")
os.environ["TARGET_DEPLOYMENT"] = TARGET_DEPLOYMENT
os.environ["TARGET_DEPLOYMENT"] = TARGET_DEPLOYMENT
def set_optimizer_deployment(deployment: str) -> None:
global OPTIMIZER_DEPLOYMENT
OPTIMIZER_DEPLOYMENT = deployment or default_model_for_backend("minimax_chat")
os.environ["OPTIMIZER_DEPLOYMENT"] = OPTIMIZER_DEPLOYMENT
-4
View File
@@ -615,9 +615,6 @@ class ClaudeCliBackend(CliBackend):
# --exclude-dynamic-... drop per-machine cwd/env/memory/git sections
# cwd=<clean temp> no project CLAUDE.md
import tempfile
# Reset per call so a prior failure does not make a later successful
# call look failed in persisted diagnostics (matches CodexCliBackend).
self.last_call_error = ""
cmd = [self.claude_path, "-p", "--output-format", "text"]
if os.environ.get("ANTHROPIC_API_KEY"):
cmd.append("--bare")
@@ -660,7 +657,6 @@ class ClaudeCliBackend(CliBackend):
# validated honestly: we detect the call from the shim's log, not from
# a self-reported marker. Other tools are stubbed the same way.
import tempfile, shutil, stat
self.last_call_error = ""
work = tempfile.mkdtemp(prefix="skillopt_sleep_tools_")
calllog = os.path.join(work, "_tool_calls.log")
try:
+62
View File
@@ -0,0 +1,62 @@
"""Tests for the MiniMax backend, focusing on optimizer/target deployment
routing (regression for the #116 follow-up: optimizer calls must honour
OPTIMIZER_DEPLOYMENT, not silently reuse TARGET_DEPLOYMENT)."""
from __future__ import annotations
import unittest
from unittest import mock
from skillopt.model import minimax_backend as mm
def _fake_response(_payload, _timeout):
return {
"choices": [{"message": {"content": "ok"}}],
"usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2},
}
class TestMiniMaxDeploymentRouting(unittest.TestCase):
def setUp(self):
self._saved = (mm.TARGET_DEPLOYMENT, mm.OPTIMIZER_DEPLOYMENT)
mm.set_target_deployment("target-model")
mm.set_optimizer_deployment("optimizer-model")
def tearDown(self):
mm.TARGET_DEPLOYMENT, mm.OPTIMIZER_DEPLOYMENT = self._saved
def _captured_model(self, fn, *args, **kwargs):
seen = {}
def spy(payload, timeout):
seen["model"] = payload["model"]
return _fake_response(payload, timeout)
with mock.patch.object(mm, "_post_chat_completion", side_effect=spy):
fn(*args, **kwargs)
return seen["model"]
def test_target_text_uses_target_deployment(self):
self.assertEqual(self._captured_model(mm.chat_target, "sys", "usr"), "target-model")
def test_optimizer_text_uses_optimizer_deployment(self):
# The core bug: before the fix this sent "target-model".
self.assertEqual(self._captured_model(mm.chat_optimizer, "sys", "usr"), "optimizer-model")
def test_optimizer_messages_uses_optimizer_deployment(self):
msgs = [{"role": "user", "content": "hi"}]
self.assertEqual(self._captured_model(mm.chat_optimizer_messages, msgs), "optimizer-model")
def test_target_messages_uses_target_deployment(self):
msgs = [{"role": "user", "content": "hi"}]
self.assertEqual(self._captured_model(mm.chat_target_messages, msgs), "target-model")
def test_optimizer_falls_back_to_target_when_unset(self):
# Empty optimizer deployment -> setter fills default; explicitly clear it
# to confirm the `or None` fallback path uses TARGET_DEPLOYMENT.
mm.OPTIMIZER_DEPLOYMENT = ""
self.assertEqual(self._captured_model(mm.chat_optimizer, "sys", "usr"), "target-model")
if __name__ == "__main__":
unittest.main()
-20
View File
@@ -1148,26 +1148,6 @@ class TestClaudeCliBackendBare(unittest.TestCase):
self.assertEqual(resp, "")
self.assertIn("Claude CLI spawn failed", be.last_call_error)
def test_last_call_error_cleared_on_next_success(self):
"""A prior failure must not make a later successful call look failed:
_call resets last_call_error at the start of each call."""
from unittest import mock
from skillopt_sleep.backend import ClaudeCliBackend
be = ClaudeCliBackend(claude_path="claude", timeout=3)
# First call fails to spawn -> error recorded.
with mock.patch("skillopt_sleep.backend.subprocess.run",
side_effect=FileNotFoundError("no claude")):
self.assertEqual(be._call("p1"), "")
self.assertIn("Claude CLI spawn failed", be.last_call_error)
# Next call succeeds -> stale error must be cleared.
class _OK:
stdout = "answer"
stderr = ""
with mock.patch("skillopt_sleep.backend.subprocess.run", return_value=_OK()):
self.assertEqual(be._call("p2"), "answer")
self.assertEqual(be.last_call_error, "")