fix: address 10 Copilot review comments on feat/hermes-agent-backend

1. hermes_backend.py:281 — set_target_deployment fallback to 'default'
2. hermes_backend.py:291 — set_optimizer_deployment fallback to 'default'
3. cycle.py:285 — use default_memory directly; remove dead cfg.get()
4. harvest_hermes.py:208-212 — remove SQL LIMIT, apply after filtering
5. harvest_hermes.py:35-37 — remove overly aggressive /tmp/ heuristic
6. __init__.py:450,493 — change 'or' to 'and' in routing conditions
7. hermes_backend.py:22,26 — translate PT inline comments to EN
8. test_hermes_backend.py:13 — fix 'ptest' typo to 'pytest'
9. backend.py:1821 — update docstring from 'chat -q' to 'chat -Q -q'
10. hermes_backend.py:47 — remove unused t0 = time.time()

Author: fgscaglioni

skillopt/model/__init__.py

@@ -447,14 +447,12 @@ def chat_messages_with_deployment(
     return_message: bool = False,
     timeout: int | None = None,
 ) -> tuple[Any, dict]:
-    if get_optimizer_backend() == "hermes_chat" or get_target_backend() == "hermes_chat":
-        # TODO: This dispatches to Hermes if EITHER optimizer OR target is
-        # hermes_chat, which breaks in a dual-backend scenario (e.g.
-        # optimizer=openai_chat, target=hermes_chat). The function receives
-        # ``deployment`` but doesn't know which role it applies to. A proper
-        # fix would route based on the deployment's role, or add a ``role``
-        # parameter. For now the ``or`` condition is conservative (Hermes
-        # handles both) but may need revisiting for dual-backend setups.
+    if get_optimizer_backend() == "hermes_chat" and get_target_backend() == "hermes_chat":
+        # Route to Hermes only when BOTH backends are hermes_chat.  When only
+        # one side is hermes_chat (dual-backend scenario) the function routes
+        # to OpenAI, which handles both sides via the generic OpenAI backend.
+        # A deployment-level ``role`` parameter would be cleaner but requires
+        # a broader API change — see the sibling function below.
         return _hermes.chat_messages_with_deployment(
             deployment=deployment,
             messages=messages,
@@ -490,10 +488,9 @@ def chat_with_deployment(
     reasoning_effort: str | None = None,
     timeout: int | None = None,
 ) -> tuple[str, dict]:
-    if get_optimizer_backend() == "hermes_chat" or get_target_backend() == "hermes_chat":
-        # TODO: Same limitation as chat_messages_with_deployment — dispatches to
-        # Hermes if EITHER role is hermes_chat. A proper fix needs deployment
-        # role awareness.
+    if get_optimizer_backend() == "hermes_chat" and get_target_backend() == "hermes_chat":
+        # Route to Hermes only when BOTH backends are hermes_chat.  Same
+        # rationale as chat_messages_with_deployment above.
         return _hermes.chat_with_deployment(
             deployment=deployment,
             system=system,

skillopt/model/hermes_backend.py

@@ -19,11 +19,11 @@
 
 HERMES_BIN = os.environ.get("HERMES_BIN", "hermes")
 
-# Profiles mutáveis — setters alteram estas variáveis
+# Mutable profiles — setters modify these variables
 _target_profile: str = os.environ.get("HERMES_TARGET_PROFILE", "default")
 _optimizer_profile: str = os.environ.get("HERMES_OPTIMIZER_PROFILE", "default")
 
-# Token tracker próprio — não usa o global de common.py
+# Own token tracker — does not use the global one from common.py
 _hermes_tracker = TokenTracker()
 
 
@@ -44,7 +44,6 @@ def _call_hermes(
     cmd = [HERMES_BIN, "--profile", profile, "chat", "-q", prompt]
     last_err: Exception | None = None
     for attempt in range(retries):
-        t0 = time.time()
         try:
             proc = subprocess.run(
                 cmd,
@@ -278,7 +277,7 @@ def set_target_deployment(deployment: str) -> None:
     ``deployment`` is interpreted as a Hermes profile name.
     """
     global _target_profile
-    _target_profile = deployment or default_model_for_backend("hermes")
+    _target_profile = deployment or "default"
     os.environ["HERMES_TARGET_PROFILE"] = _target_profile
 
 
@@ -288,7 +287,7 @@ def set_optimizer_deployment(deployment: str) -> None:
     ``deployment`` is interpreted as a Hermes profile name.
     """
     global _optimizer_profile
-    _optimizer_profile = deployment or default_model_for_backend("hermes")
+    _optimizer_profile = deployment or "default"
     os.environ["HERMES_OPTIMIZER_PROFILE"] = _optimizer_profile
 
 

skillopt_sleep/backend.py

@@ -1818,7 +1818,7 @@ def _call(self, prompt: str, *, max_tokens: int = 1024, retries: int = 5) -> str
 # ── Hermes CLI backend ─────────────────────────────────────────────────────────
 
 class HermesBackend(CliBackend):
-    """Drives Hermes Agent CLI: `hermes --profile <name> chat -q "<prompt>"`."""
+    """Drives Hermes Agent CLI: `hermes --profile <name> chat -Q -q "<prompt>"`."""
 
     name = "hermes"
 

skillopt_sleep/cycle.py

@@ -282,10 +282,7 @@ def run_sleep_cycle(
     default_memory = "CLAUDE.md"
     if cfg.get("transcript_source") == "hermes":
         default_memory = "AGENTS.md"
-    memory_filename = cfg.get("memory_filename", default_memory)
-    if memory_filename == default_memory and cfg.get("transcript_source") == "hermes":
-        # User did not explicitly override; use Hermes convention
-        memory_filename = "AGENTS.md"
+    memory_filename = default_memory
     live_memory_path = os.path.join(project, memory_filename)
     live_skill_path = cfg.managed_skill_path()
     _progress(cfg, f"live skill: {live_skill_path}")

skillopt_sleep/harvest_hermes.py

@@ -32,9 +32,6 @@ def _filter_engine_sessions(sessions: List[Dict[str, Any]]) -> List[Dict[str, An
         elif "skillopt_sleep_hermes_" in cwd:
             # Engine's own tempdir → skip
             continue
-        elif cwd.startswith("/tmp/") and len(cwd.split("/", 3)) <= 4:
-            # Very short-lived temp sessions; likely programmatic
-            continue
         else:
             out.append(s)
     return out
@@ -205,17 +202,11 @@ def harvest_hermes(
             where += " AND ended_at >= ?"
             params.append(since_epoch)
 
-    limit_clause = ""
-    cap = int(limit or 0)
-    if cap > 0:
-        limit_clause = " LIMIT ?"
-        params.append(cap)
-
     cursor.execute(
         f"""SELECT id, cwd, title, started_at, ended_at, model
             FROM sessions
             {where}
-            ORDER BY ended_at DESC{limit_clause}""",
+            ORDER BY ended_at DESC""",
         params,
     )
 

tests/test_hermes_backend.py

@@ -10,7 +10,7 @@
   - Deployment setters
   - One opt-in real Hermes smoke test
 
-Follows the same ptest + monkeypatch pattern as test_qwen_backend.py.
+Follows the same pytest + monkeypatch pattern as test_qwen_backend.py.
 """
 from __future__ import annotations