fix(sleep): reset ClaudeCliBackend.last_call_error at each call start

Follow-up to #126. ClaudeCliBackend recorded last_call_error on failure but
never reset it, so a failure followed by a successful call still reported the
stale error in persisted diagnostics — making a healthy run look failed. Both
_call() and attempt_with_tools() now clear it at the start, matching
CodexCliBackend.

Note: the "TimeoutExpired leaks the prompt into logs" concern raised in review
does NOT apply on current main — the prompt is passed via stdin (input=), not
argv (fixed by #98), so TimeoutExpired's str() only contains the argv (no
prompt). Verified empirically.

Adds a regression test asserting a prior failure is cleared on the next
success.

Co-Authored-By: Claude <noreply@anthropic.com>

skillopt/model/__init__.py

@@ -215,7 +215,7 @@ def chat_optimizer_messages(
             timeout=timeout,
         )
     if get_optimizer_backend() == "minimax_chat":
-        return _minimax.chat_optimizer_messages(
+        return _minimax.chat_target_messages(
             messages=messages,
             max_completion_tokens=max_completion_tokens,
             retries=retries,
@@ -540,4 +540,3 @@ 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)

skillopt/model/minimax_backend.py

@@ -36,9 +36,6 @@ 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()
@@ -225,7 +222,7 @@ def chat_target(
     stage: str = "target",
     reasoning_effort: str | None = None,
     timeout: float | None = None,
-) -> tuple[str, dict[str, int]]:
+) -> tuple[str, dict[int]]:
     del reasoning_effort
     messages = [{"role": "system", "content": system}, {"role": "user", "content": user}]
     return _chat_messages_impl(
@@ -245,7 +242,7 @@ def chat_optimizer(
     stage: str = "optimizer",
     reasoning_effort: str | None = None,
     timeout: float | None = None,
-) -> tuple[str, dict[str, int]]:
+) -> tuple[str, dict[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
@@ -259,7 +256,6 @@ def chat_optimizer(
         max_completion_tokens,
         retries,
         stage,
-        deployment=OPTIMIZER_DEPLOYMENT or None,
         timeout=timeout,
     )
 
@@ -289,35 +285,6 @@ 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()
 
@@ -333,10 +300,4 @@ 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
-
-
-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
+    os.environ["TARGET_DEPLOYMENT"] = TARGET_DEPLOYMENT

skillopt_sleep/backend.py

@@ -615,6 +615,9 @@ 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")
@@ -657,6 +660,7 @@ 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:

tests/test_minimax_backend.py

@@ -1,62 +0,0 @@
-"""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()

tests/test_sleep_engine.py

@@ -1148,6 +1148,26 @@ 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, "")
+