feat(execute_code): recovery hints for known sandbox failure classes
The top execute_code failure shapes in production (state.db mining) are sandbox-contract confusions, not logic bugs: importing tools that aren't in the sandbox from hermes_tools (23x in one window, incl. importing the built-in helpers json_parse/shell_quote/retry), importing third-party packages absent from the sandbox interpreter (matplotlib 6x), and indexing tool-result dicts as strings. The stderr traceback alone sends models into re-diagnosis loops. Failed scripts (exit != 0) now carry one actionable 'hint' field: - unavailable hermes_tools import -> lists the tools that ARE importable in this session + points to normal tool calls otherwise; - built-in helper import -> 'no import needed, call it directly'; - ModuleNotFoundError -> 'sandbox has stdlib only; use terminal() with the project venv for third-party packages'; - string-indexing errors -> 'tool functions return dicts, do not json.loads them'. Bounded 4KB stderr scan, first match wins, never raises; successful scripts and unknown failures are untouched.
This commit is contained in:
parent
1cefabc8af
commit
eb62143006
|
|
@ -0,0 +1,66 @@
|
|||
"""Tests for execute_code sandbox failure hints."""
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.code_execution_tool import _sandbox_failure_hint, execute_code
|
||||
|
||||
|
||||
class TestSandboxFailureHint:
|
||||
def test_unavailable_tool_import_lists_available(self):
|
||||
err = ("Traceback (most recent call last):\n File \"script.py\", line 1\n"
|
||||
"ImportError: cannot import name 'browser_navigate' from 'hermes_tools'")
|
||||
h = _sandbox_failure_hint(err, enabled_tools={"terminal", "read_file"})
|
||||
assert "browser_navigate" in h
|
||||
assert "read_file" in h and "terminal" in h
|
||||
assert "normal tool call" in h
|
||||
|
||||
def test_builtin_helper_import_redirects(self):
|
||||
err = "ImportError: cannot import name 'json_parse' from 'hermes_tools'"
|
||||
h = _sandbox_failure_hint(err)
|
||||
assert "BUILT-IN" in h
|
||||
assert "no import" in h.lower()
|
||||
|
||||
def test_missing_third_party_module(self):
|
||||
err = "ModuleNotFoundError: No module named 'matplotlib'"
|
||||
h = _sandbox_failure_hint(err)
|
||||
assert "matplotlib" in h
|
||||
assert "stdlib" in h
|
||||
|
||||
def test_dict_vs_string_confusion(self):
|
||||
err = "TypeError: string indices must be integers"
|
||||
h = _sandbox_failure_hint(err)
|
||||
assert "DICTS" in h
|
||||
|
||||
def test_unknown_failure_no_hint(self):
|
||||
assert _sandbox_failure_hint("ZeroDivisionError: division by zero") is None
|
||||
|
||||
def test_empty_stderr_no_hint(self):
|
||||
assert _sandbox_failure_hint("") is None
|
||||
|
||||
|
||||
class TestLiveSandboxHint:
|
||||
def test_bad_import_produces_hint_field(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||
r = json.loads(execute_code(
|
||||
"from hermes_tools import totally_fake_tool\nprint('unreachable')",
|
||||
task_id="t-sbhint",
|
||||
))
|
||||
assert r["status"] == "error"
|
||||
assert "hint" in r
|
||||
assert "totally_fake_tool" in r["hint"]
|
||||
|
||||
def test_missing_module_produces_hint(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||
r = json.loads(execute_code(
|
||||
"import nonexistent_pkg_zzz\n", task_id="t-sbhint",
|
||||
))
|
||||
assert r["status"] == "error"
|
||||
assert "not installed in the sandbox" in r.get("hint", "")
|
||||
|
||||
def test_successful_script_has_no_hint(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||
r = json.loads(execute_code("print('fine')", task_id="t-sbhint"))
|
||||
assert r["status"] == "success"
|
||||
assert "hint" not in r
|
||||
|
|
@ -34,6 +34,7 @@ import json
|
|||
import logging
|
||||
import os
|
||||
import platform
|
||||
import re
|
||||
import secrets
|
||||
import shlex
|
||||
import socket
|
||||
|
|
@ -372,6 +373,61 @@ _TOOL_STUBS = {
|
|||
}
|
||||
|
||||
|
||||
def _sandbox_failure_hint(stderr_text: str, enabled_tools=None) -> Optional[str]:
|
||||
"""Map well-known sandbox script failures to one actionable recovery hint.
|
||||
|
||||
Production mining (state.db): the top execute_code failure classes are
|
||||
hermes_tools import misuse (importing tools that aren't in the sandbox,
|
||||
23x in one window), calling the built-in helpers via import, treating
|
||||
tool results as strings instead of dicts, and importing third-party
|
||||
packages that don't exist in the sandbox interpreter. Bounded scan,
|
||||
first match wins, never raises.
|
||||
"""
|
||||
if not stderr_text:
|
||||
return None
|
||||
window = stderr_text[:4000]
|
||||
try:
|
||||
m = re.search(
|
||||
r"cannot import name '(\w+)' from 'hermes_tools'", window
|
||||
)
|
||||
if m:
|
||||
missing = m.group(1)
|
||||
available = sorted(SANDBOX_ALLOWED_TOOLS & set(enabled_tools or SANDBOX_ALLOWED_TOOLS))
|
||||
builtin = {"json_parse", "shell_quote", "retry"}
|
||||
if missing in builtin:
|
||||
return (
|
||||
f"{missing} is a BUILT-IN helper in the sandbox — no import "
|
||||
f"needed. Remove it from the import line and call {missing}(...) directly."
|
||||
)
|
||||
return (
|
||||
f"'{missing}' is not available inside the execute_code sandbox. "
|
||||
f"Importable tools here: {', '.join(available)}. For anything "
|
||||
"else, use the normal tool call instead of execute_code."
|
||||
)
|
||||
m = re.search(r"NameError: name '(json_parse|shell_quote|retry)' is not defined", window)
|
||||
if m:
|
||||
return (
|
||||
f"{m.group(1)} is built into the generated sandbox module — "
|
||||
"call it directly at module scope without importing it."
|
||||
)
|
||||
m = re.search(r"ModuleNotFoundError: No module named '([\w.]+)'", window)
|
||||
if m:
|
||||
return (
|
||||
f"'{m.group(1)}' is not installed in the sandbox interpreter. "
|
||||
"Use Python stdlib inside execute_code, or run the code via "
|
||||
"terminal() with the project venv's python instead."
|
||||
)
|
||||
if re.search(r"TypeError: string indices must be integers|AttributeError: 'str' object has no attribute 'get'", window):
|
||||
return (
|
||||
"Tool functions in the sandbox return DICTS (already parsed) — "
|
||||
"do not json.loads() them or index them like strings. "
|
||||
"Example: read_file(path)['content']."
|
||||
)
|
||||
except Exception:
|
||||
return None
|
||||
return None
|
||||
|
||||
|
||||
def generate_hermes_tools_module(enabled_tools: List[str],
|
||||
transport: str = "uds") -> str:
|
||||
"""
|
||||
|
|
@ -1626,6 +1682,12 @@ def execute_code(
|
|||
# Include stderr in output so the LLM sees the traceback
|
||||
if stderr_text:
|
||||
result["output"] = stdout_text + "\n--- stderr ---\n" + stderr_text
|
||||
# Known-failure-class recovery hint (import misuse, missing
|
||||
# module, dict-vs-string result handling) so the model fixes
|
||||
# the script on the next attempt instead of re-diagnosing.
|
||||
hint = _sandbox_failure_hint(stderr_text, enabled_tools=sandbox_tools)
|
||||
if hint:
|
||||
result["hint"] = hint
|
||||
|
||||
return json.dumps(result, ensure_ascii=False)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue