mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
refactor: codebase-wide lint cleanup — unused imports, dead code, and inefficient patterns (#5821)
Comprehensive cleanup across 80 files based on automated (ruff, pyflakes, vulture)
and manual analysis of the entire codebase.
Changes by category:
Unused imports removed (~95 across 55 files):
- Removed genuinely unused imports from all major subsystems
- agent/, hermes_cli/, tools/, gateway/, plugins/, cron/
- Includes imports in try/except blocks that were truly unused
(vs availability checks which were left alone)
Unused variables removed (~25):
- Removed dead variables: connected, inner, channels, last_exc,
source, new_server_names, verify, pconfig, default_terminal,
result, pending_handled, temperature, loop
- Dropped unused argparse subparser assignments in hermes_cli/main.py
(12 instances of add_parser() where result was never used)
Dead code removed:
- run_agent.py: Removed dead ternary (None if False else None) and
surrounding unreachable branch in identity fallback
- run_agent.py: Removed write-only attribute _last_reported_tool
- hermes_cli/providers.py: Removed dead @property decorator on
module-level function (decorator has no effect outside a class)
- gateway/run.py: Removed unused MCP config load before reconnect
- gateway/platforms/slack.py: Removed dead SessionSource construction
Undefined name bugs fixed (would cause NameError at runtime):
- batch_runner.py: Added missing logger = logging.getLogger(__name__)
- tools/environments/daytona.py: Added missing Dict and Path imports
Unnecessary global statements removed (14):
- tools/terminal_tool.py: 5 functions declared global for dicts
they only mutated via .pop()/[key]=value (no rebinding)
- tools/browser_tool.py: cleanup thread loop only reads flag
- tools/rl_training_tool.py: 4 functions only do dict mutations
- tools/mcp_oauth.py: only reads the global
- hermes_time.py: only reads cached values
Inefficient patterns fixed:
- startswith/endswith tuple form: 15 instances of
x.startswith('a') or x.startswith('b') consolidated to
x.startswith(('a', 'b'))
- len(x)==0 / len(x)>0: 13 instances replaced with pythonic
truthiness checks (not x / bool(x))
- in dict.keys(): 5 instances simplified to in dict
- Redefined unused name: removed duplicate _strip_mdv2 import in
send_message_tool.py
Other fixes:
- hermes_cli/doctor.py: Replaced undefined logger.debug() with pass
- hermes_cli/config.py: Consolidated chained .endswith() calls
Test results: 3934 passed, 17 failed (all pre-existing on main),
19 skipped. Zero regressions.
This commit is contained in:
parent
afe6c63c52
commit
d0ffb111c2
80 changed files with 81 additions and 210 deletions
|
|
@ -27,9 +27,7 @@ import json
|
|||
import logging
|
||||
import os
|
||||
import threading
|
||||
import time
|
||||
import uuid
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict, Optional
|
||||
|
||||
import requests
|
||||
|
|
@ -445,7 +443,7 @@ def camofox_get_images(task_id: Optional[str] = None) -> str:
|
|||
lines = snapshot.split("\n")
|
||||
for i, line in enumerate(lines):
|
||||
stripped = line.strip()
|
||||
if stripped.startswith("- img ") or stripped.startswith("img "):
|
||||
if stripped.startswith(("- img ", "img ")):
|
||||
alt_match = re.search(r'img\s+"([^"]*)"', stripped)
|
||||
alt = alt_match.group(1) if alt_match else ""
|
||||
# Look for URL on the next line
|
||||
|
|
|
|||
|
|
@ -191,7 +191,7 @@ def _resolve_cdp_override(cdp_url: str) -> str:
|
|||
return raw
|
||||
|
||||
discovery_url = raw
|
||||
if lowered.startswith("ws://") or lowered.startswith("wss://"):
|
||||
if lowered.startswith(("ws://", "wss://")):
|
||||
if raw.count(":") == 2 and raw.rstrip("/").rsplit(":", 1)[-1].isdigit() and "/" not in raw.split(":", 2)[-1]:
|
||||
discovery_url = ("http://" if lowered.startswith("ws://") else "https://") + raw.split("://", 1)[1]
|
||||
else:
|
||||
|
|
@ -458,8 +458,6 @@ def _browser_cleanup_thread_worker():
|
|||
Runs every 30 seconds and checks for sessions that haven't been used
|
||||
within the BROWSER_SESSION_INACTIVITY_TIMEOUT period.
|
||||
"""
|
||||
global _cleanup_running
|
||||
|
||||
while _cleanup_running:
|
||||
try:
|
||||
_cleanup_inactive_browser_sessions()
|
||||
|
|
|
|||
|
|
@ -693,7 +693,6 @@ def _execute_remote(
|
|||
the remote environment, and tool calls are proxied through a polling
|
||||
thread that communicates via request/response files.
|
||||
"""
|
||||
from tools.terminal_tool import _interrupt_event
|
||||
|
||||
_cfg = _load_config()
|
||||
timeout = _cfg.get("timeout", DEFAULT_TIMEOUT)
|
||||
|
|
|
|||
|
|
@ -150,7 +150,6 @@ def _validate_cron_script_path(script: Optional[str]) -> Optional[str]:
|
|||
if not script or not script.strip():
|
||||
return None # empty/None = clearing the field, always OK
|
||||
|
||||
from pathlib import Path
|
||||
from hermes_constants import get_hermes_home
|
||||
|
||||
raw = script.strip()
|
||||
|
|
|
|||
|
|
@ -26,7 +26,6 @@ import json
|
|||
import logging
|
||||
import os
|
||||
import uuid
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict
|
||||
|
||||
from hermes_constants import get_hermes_home
|
||||
|
|
|
|||
|
|
@ -12,7 +12,8 @@ import shlex
|
|||
import threading
|
||||
import uuid
|
||||
import warnings
|
||||
from typing import Optional
|
||||
from pathlib import Path
|
||||
from typing import Dict, Optional
|
||||
|
||||
from tools.environments.base import BaseEnvironment
|
||||
from tools.interrupt import is_interrupted
|
||||
|
|
|
|||
|
|
@ -11,11 +11,10 @@ import os
|
|||
import shlex
|
||||
import shutil
|
||||
import subprocess
|
||||
import tempfile
|
||||
import threading
|
||||
import uuid
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict, Optional
|
||||
from typing import Dict, Optional
|
||||
|
||||
from hermes_constants import get_hermes_home
|
||||
from tools.environments.base import BaseEnvironment
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ import threading
|
|||
import webbrowser
|
||||
from http.server import BaseHTTPRequestHandler, HTTPServer
|
||||
from pathlib import Path
|
||||
from typing import Any, Optional
|
||||
from typing import Any
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -54,7 +54,7 @@ logger = logging.getLogger(__name__)
|
|||
|
||||
_OAUTH_AVAILABLE = False
|
||||
try:
|
||||
from mcp.client.auth import OAuthClientProvider, TokenStorage
|
||||
from mcp.client.auth import OAuthClientProvider
|
||||
from mcp.shared.auth import (
|
||||
OAuthClientInformationFull,
|
||||
OAuthClientMetadata,
|
||||
|
|
@ -320,7 +320,6 @@ async def _wait_for_callback() -> tuple[str, str | None]:
|
|||
OAuthNonInteractiveError: If the callback times out (no user present
|
||||
to complete the browser auth).
|
||||
"""
|
||||
global _oauth_port
|
||||
assert _oauth_port is not None, "OAuth callback port not set"
|
||||
|
||||
# The callback server is already running (started in build_oauth_auth).
|
||||
|
|
|
|||
|
|
@ -260,7 +260,7 @@ class MemoryStore:
|
|||
entries = self._entries_for(target)
|
||||
matches = [(i, e) for i, e in enumerate(entries) if old_text in e]
|
||||
|
||||
if len(matches) == 0:
|
||||
if not matches:
|
||||
return {"success": False, "error": f"No entry matched '{old_text}'."}
|
||||
|
||||
if len(matches) > 1:
|
||||
|
|
@ -310,7 +310,7 @@ class MemoryStore:
|
|||
entries = self._entries_for(target)
|
||||
matches = [(i, e) for i, e in enumerate(entries) if old_text in e]
|
||||
|
||||
if len(matches) == 0:
|
||||
if not matches:
|
||||
return {"success": False, "error": f"No entry matched '{old_text}'."}
|
||||
|
||||
if len(matches) > 1:
|
||||
|
|
|
|||
|
|
@ -567,7 +567,7 @@ async def rl_select_environment(name: str) -> str:
|
|||
|
||||
TIP: Read the returned file_path to understand how the environment works.
|
||||
"""
|
||||
global _current_env, _current_config, _env_config_cache
|
||||
global _current_env, _current_config
|
||||
|
||||
_initialize_environments()
|
||||
|
||||
|
|
@ -673,8 +673,6 @@ async def rl_edit_config(field: str, value: Any) -> str:
|
|||
Returns:
|
||||
JSON string with updated config or error message
|
||||
"""
|
||||
global _current_config
|
||||
|
||||
if not _current_env:
|
||||
return json.dumps({
|
||||
"error": "No environment selected. Use rl_select_environment(name) first.",
|
||||
|
|
@ -727,8 +725,6 @@ async def rl_start_training() -> str:
|
|||
Returns:
|
||||
JSON string with run_id and initial status
|
||||
"""
|
||||
global _active_runs
|
||||
|
||||
if not _current_env:
|
||||
return json.dumps({
|
||||
"error": "No environment selected. Use rl_select_environment(name) first.",
|
||||
|
|
@ -829,8 +825,6 @@ async def rl_check_status(run_id: str) -> str:
|
|||
Returns:
|
||||
JSON string with run status and metrics
|
||||
"""
|
||||
global _last_status_check
|
||||
|
||||
# Check rate limiting
|
||||
now = time.time()
|
||||
if run_id in _last_status_check:
|
||||
|
|
@ -1311,7 +1305,7 @@ async def rl_test_inference(
|
|||
"avg_accuracy": round(
|
||||
sum(m.get("accuracy", 0) for m in working_models) / len(working_models), 3
|
||||
) if working_models else 0,
|
||||
"environment_working": len(working_models) > 0,
|
||||
"environment_working": bool(working_models),
|
||||
"output_directory": str(test_output_dir),
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -432,7 +432,7 @@ async def _send_telegram(token, chat_id, message, media_files=None, thread_id=No
|
|||
else:
|
||||
# Reuse the gateway adapter's format_message for markdown→MarkdownV2
|
||||
try:
|
||||
from gateway.platforms.telegram import TelegramAdapter, _strip_mdv2
|
||||
from gateway.platforms.telegram import TelegramAdapter
|
||||
_adapter = TelegramAdapter.__new__(TelegramAdapter)
|
||||
formatted = _adapter.format_message(message)
|
||||
except Exception:
|
||||
|
|
|
|||
|
|
@ -430,7 +430,7 @@ class GitHubSource(SkillSource):
|
|||
continue
|
||||
|
||||
dir_name = entry["name"]
|
||||
if dir_name.startswith(".") or dir_name.startswith("_"):
|
||||
if dir_name.startswith((".", "_")):
|
||||
continue
|
||||
|
||||
prefix = path.rstrip("/")
|
||||
|
|
@ -1163,7 +1163,7 @@ class SkillsShSource(SkillSource):
|
|||
if entry.get("type") != "dir":
|
||||
continue
|
||||
dir_name = entry["name"]
|
||||
if dir_name.startswith(".") or dir_name.startswith("_"):
|
||||
if dir_name.startswith((".", "_")):
|
||||
continue
|
||||
if dir_name in ("skills", ".agents", ".claude"):
|
||||
continue # already tried
|
||||
|
|
@ -1382,7 +1382,7 @@ class ClawHubSource(SkillSource):
|
|||
if isinstance(tags, list):
|
||||
return [str(t) for t in tags]
|
||||
if isinstance(tags, dict):
|
||||
return [str(k) for k in tags.keys() if str(k) != "latest"]
|
||||
return [str(k) for k in tags if str(k) != "latest"]
|
||||
return []
|
||||
|
||||
@staticmethod
|
||||
|
|
|
|||
|
|
@ -72,12 +72,10 @@ import logging
|
|||
from hermes_constants import get_hermes_home
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
from enum import Enum
|
||||
from pathlib import Path
|
||||
from typing import Dict, Any, List, Optional, Set, Tuple
|
||||
|
||||
import yaml
|
||||
from tools.registry import registry
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
|
|||
|
|
@ -720,8 +720,6 @@ def _create_environment(env_type: str, image: str, cwd: str, timeout: int,
|
|||
|
||||
def _cleanup_inactive_envs(lifetime_seconds: int = 300):
|
||||
"""Clean up environments that have been inactive for longer than lifetime_seconds."""
|
||||
global _active_environments, _last_activity
|
||||
|
||||
current_time = time.time()
|
||||
|
||||
# Check the process registry -- skip cleanup for sandboxes with active
|
||||
|
|
@ -784,8 +782,6 @@ def _cleanup_inactive_envs(lifetime_seconds: int = 300):
|
|||
|
||||
def _cleanup_thread_worker():
|
||||
"""Background thread worker that periodically cleans up inactive environments."""
|
||||
global _cleanup_running
|
||||
|
||||
while _cleanup_running:
|
||||
try:
|
||||
config = _get_env_config()
|
||||
|
|
@ -831,7 +827,7 @@ def get_active_environments_info() -> Dict[str, Any]:
|
|||
|
||||
# Calculate total disk usage (per-task to avoid double-counting)
|
||||
total_size = 0
|
||||
for task_id in _active_environments.keys():
|
||||
for task_id in _active_environments:
|
||||
scratch_dir = _get_scratch_dir()
|
||||
pattern = f"hermes-*{task_id[:8]}*"
|
||||
import glob
|
||||
|
|
@ -848,8 +844,6 @@ def get_active_environments_info() -> Dict[str, Any]:
|
|||
|
||||
def cleanup_all_environments():
|
||||
"""Clean up ALL active environments. Use with caution."""
|
||||
global _active_environments, _last_activity
|
||||
|
||||
task_ids = list(_active_environments.keys())
|
||||
cleaned = 0
|
||||
|
||||
|
|
@ -877,8 +871,6 @@ def cleanup_all_environments():
|
|||
|
||||
def cleanup_vm(task_id: str):
|
||||
"""Manually clean up a specific environment by task_id."""
|
||||
global _active_environments, _last_activity
|
||||
|
||||
# Remove from tracking dicts while holding the lock, but defer the
|
||||
# actual (potentially slow) env.cleanup() call to outside the lock
|
||||
# so other tool calls aren't blocked.
|
||||
|
|
@ -1043,8 +1035,6 @@ def terminal_tool(
|
|||
# Force run after user confirmation
|
||||
# Note: force parameter is internal only, not exposed to model API
|
||||
"""
|
||||
global _active_environments, _last_activity
|
||||
|
||||
try:
|
||||
# Get configuration
|
||||
config = _get_env_config()
|
||||
|
|
|
|||
|
|
@ -85,7 +85,7 @@ class TodoStore:
|
|||
|
||||
def has_items(self) -> bool:
|
||||
"""Check if there are any items in the list."""
|
||||
return len(self._items) > 0
|
||||
return bool(self._items)
|
||||
|
||||
def format_for_injection(self) -> Optional[str]:
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -550,7 +550,6 @@ def text_to_speech_tool(
|
|||
if edge_available:
|
||||
logger.info("Generating speech with Edge TTS...")
|
||||
try:
|
||||
loop = asyncio.get_running_loop()
|
||||
import concurrent.futures
|
||||
with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool:
|
||||
pool.submit(
|
||||
|
|
|
|||
|
|
@ -82,7 +82,7 @@ def _validate_image_url(url: str) -> bool:
|
|||
return False
|
||||
|
||||
# Basic HTTP/HTTPS URL check
|
||||
if not (url.startswith("http://") or url.startswith("https://")):
|
||||
if not url.startswith(("http://", "https://")):
|
||||
return False
|
||||
|
||||
# Parse to ensure we at least have a network location; still allow URLs
|
||||
|
|
|
|||
|
|
@ -108,7 +108,7 @@ def detect_audio_environment() -> dict:
|
|||
)
|
||||
|
||||
return {
|
||||
"available": len(warnings) == 0,
|
||||
"available": not warnings,
|
||||
"warnings": warnings,
|
||||
"notices": notices,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,7 +12,6 @@ from __future__ import annotations
|
|||
|
||||
import fnmatch
|
||||
import logging
|
||||
import os
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue