fix(skillopt-sleep): surface Claude CLI spawn failures instead of silent zero scores (#126)
* fix(skillopt-sleep): surface Claude CLI spawn failures instead of silent zero scores In _call and attempt_with_tools, the bare 'except Exception: return ""' swallows FileNotFoundError (e.g. bare 'claude' on Windows with npm .cmd shim) and any other spawn failure, returning an empty string that the trainer treats as a legitimate model response that scores 0.0 everywhere. Now the exception is caught explicitly: last_call_error is set, a warning is logged, and the empty-string return is preserved for backward compatibility of the control flow. This mirrors the pattern from #92 (codex backend) which fixed the same class of 'dead CLI masquerades as nothing to learn' bug. Issue: #121 * test(skillopt-sleep): add tests verifying Claude CLI spawn failures are surfaced Add two tests to TestClaudeCliBackendBare: - test_spawn_failure_sets_last_call_error: _call sets last_call_error and returns '' when subprocess.run raises FileNotFoundError. - test_attempt_tools_spawn_failure_sets_last_call_error: same for attempt_with_tools. These prove the fix from the parent commit (surface spawn failures instead of silently scoring 0) and guard against regressions.
This commit is contained in:
@@ -634,7 +634,12 @@ class ClaudeCliBackend(CliBackend):
|
|||||||
cmd, capture_output=True, creationflags=_NO_WINDOW, text=True, timeout=self.timeout, cwd=clean_cwd,
|
cmd, capture_output=True, creationflags=_NO_WINDOW, text=True, timeout=self.timeout, cwd=clean_cwd,
|
||||||
input=prompt,
|
input=prompt,
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception as exc:
|
||||||
|
import logging
|
||||||
|
self.last_call_error = f"Claude CLI spawn failed: {exc}"
|
||||||
|
logging.getLogger("skillopt_sleep").warning(
|
||||||
|
"Claude CLI could not be executed: %s", exc,
|
||||||
|
)
|
||||||
return ""
|
return ""
|
||||||
finally:
|
finally:
|
||||||
try:
|
try:
|
||||||
@@ -698,7 +703,12 @@ class ClaudeCliBackend(CliBackend):
|
|||||||
)
|
)
|
||||||
resp = (proc.stdout or "").strip()
|
resp = (proc.stdout or "").strip()
|
||||||
self._detect_cli_error(resp, proc.stderr or "")
|
self._detect_cli_error(resp, proc.stderr or "")
|
||||||
except Exception:
|
except Exception as exc:
|
||||||
|
import logging
|
||||||
|
self.last_call_error = f"Claude CLI spawn failed: {exc}"
|
||||||
|
logging.getLogger("skillopt_sleep").warning(
|
||||||
|
"Claude CLI could not be executed: %s", exc,
|
||||||
|
)
|
||||||
resp = ""
|
resp = ""
|
||||||
self._tokens += len(prompt) // 4 + len(resp) // 4
|
self._tokens += len(prompt) // 4 + len(resp) // 4
|
||||||
called: List[str] = []
|
called: List[str] = []
|
||||||
|
|||||||
@@ -1118,6 +1118,36 @@ class TestClaudeCliBackendBare(unittest.TestCase):
|
|||||||
# But it's also recorded for detection
|
# But it's also recorded for detection
|
||||||
self.assertIn("Not logged in", getattr(be, "last_call_error", ""))
|
self.assertIn("Not logged in", getattr(be, "last_call_error", ""))
|
||||||
|
|
||||||
|
def test_spawn_failure_sets_last_call_error(self):
|
||||||
|
"""When subprocess.run raises FileNotFoundError, _call must set
|
||||||
|
last_call_error and log a warning instead of silently returning ''."""
|
||||||
|
from skillopt_sleep.backend import ClaudeCliBackend
|
||||||
|
be = ClaudeCliBackend(
|
||||||
|
claude_path="/nonexistent/claude-binary",
|
||||||
|
timeout=3,
|
||||||
|
)
|
||||||
|
result = be._call("test prompt")
|
||||||
|
self.assertEqual(result, "")
|
||||||
|
self.assertIn("Claude CLI spawn failed", be.last_call_error)
|
||||||
|
|
||||||
|
def test_attempt_tools_spawn_failure_sets_last_call_error(self):
|
||||||
|
"""When subprocess.run raises in attempt_with_tools, last_call_error
|
||||||
|
must be set and a warning logged."""
|
||||||
|
from skillopt_sleep.backend import ClaudeCliBackend
|
||||||
|
from skillopt_sleep.types import TaskRecord
|
||||||
|
be = ClaudeCliBackend(
|
||||||
|
claude_path="/nonexistent/claude-binary",
|
||||||
|
timeout=3,
|
||||||
|
)
|
||||||
|
task = TaskRecord(
|
||||||
|
id="t1", project="/p", intent="test",
|
||||||
|
reference="ref", reference_kind="exact",
|
||||||
|
tags=[], split="train",
|
||||||
|
)
|
||||||
|
resp, called = be.attempt_with_tools(task, "", "", tools=["search"])
|
||||||
|
self.assertEqual(resp, "")
|
||||||
|
self.assertIn("Claude CLI spawn failed", be.last_call_error)
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user