From cffc1df5169f44bb372bcbb8ae317cec81cb68cc Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:42:44 -0700 Subject: [PATCH] refactor(tools): group H layout compaction (AST-neutral bracket hugging) --- tools/spill_safety.py | 12 +--- tools/tool_backend_helpers.py | 39 +++-------- tools/tool_result_storage.py | 35 +++------- tools/tool_search.py | 110 +++++++++----------------------- tools/tool_search_catalog.py | 11 +--- tools/tool_search_validation.py | 15 ++--- 6 files changed, 60 insertions(+), 162 deletions(-) diff --git a/tools/spill_safety.py b/tools/spill_safety.py index 62fa4dea78..69b68ec172 100644 --- a/tools/spill_safety.py +++ b/tools/spill_safety.py @@ -20,11 +20,7 @@ import stat from pathlib import Path from typing import IO -__all__ = [ - "ensure_spill_dir", - "open_exclusive", - "write_text_exclusive", -] +__all__ = ["ensure_spill_dir", "open_exclusive", "write_text_exclusive"] # O_NOFOLLOW is POSIX-only; on Windows O_EXCL alone already refuses every # pre-existing path. @@ -51,8 +47,7 @@ def open_exclusive( private: bool = True, overwrite: bool = False, encoding: str = "utf-8", - errors: str = "strict", -) -> IO[str]: + errors: str = "strict") -> IO[str]: """Open ``path`` for writing via exclusive create; never follows a link. ``overwrite=True`` first unlinks an existing path (``lstat``-checked, so only the link itself is removed and directories are refused), then creates @@ -83,8 +78,7 @@ def write_text_exclusive( private: bool = True, overwrite: bool = False, encoding: str = "utf-8", - errors: str = "strict", -) -> None: + errors: str = "strict") -> None: """``Path.write_text`` equivalent that refuses to follow symlinks.""" with open_exclusive( path, private=private, overwrite=overwrite, encoding=encoding, errors=errors diff --git a/tools/tool_backend_helpers.py b/tools/tool_backend_helpers.py index 09152df819..5a8aba8fc1 100644 --- a/tools/tool_backend_helpers.py +++ b/tools/tool_backend_helpers.py @@ -36,10 +36,7 @@ def managed_nous_tools_enabled(*, force_fresh: bool = False) -> bool: def nous_tool_gateway_unavailable_message( - capability: str = "the Nous Tool Gateway", - *, - force_fresh: bool = False, -) -> str: + capability: str = "the Nous Tool Gateway", *, force_fresh: bool = False) -> str: """Return account-aware guidance for an unavailable Nous Tool Gateway path.""" try: from hermes_cli.nous_account import ( @@ -55,8 +52,7 @@ def nous_tool_gateway_unavailable_message( pass return ( f"{capability} is unavailable. Run `hermes model` to refresh your " - "Nous Portal login and billing status." - ) + "Nous Portal login and billing status.") def normalize_browser_cloud_provider(value: object | None) -> str: @@ -81,9 +77,7 @@ def has_direct_modal_credentials() -> bool: except (PermissionError, OSError): modal_file_exists = False return bool( - (os.getenv("MODAL_TOKEN_ID") and os.getenv("MODAL_TOKEN_SECRET")) - or modal_file_exists - ) + (os.getenv("MODAL_TOKEN_ID") and os.getenv("MODAL_TOKEN_SECRET")) or modal_file_exists) def resolve_modal_backend_state( @@ -91,8 +85,7 @@ def resolve_modal_backend_state( *, has_direct: bool, managed_ready: bool, - managed_enabled: bool | None = None, -) -> Dict[str, Any]: + managed_enabled: bool | None = None) -> Dict[str, Any]: """Resolve direct vs managed Modal backend: ``direct``/``managed`` are exclusive; ``auto`` prefers managed when available, else direct.""" requested_mode = coerce_modal_mode(modal_mode) @@ -111,8 +104,7 @@ def resolve_modal_backend_state( "has_direct": has_direct, "managed_ready": managed_ready, "managed_mode_blocked": requested_mode == "managed" and not managed_enabled, - "selected_backend": selected_backend, - } + "selected_backend": selected_backend} def _scoped_credential(name: str) -> str: @@ -138,11 +130,7 @@ def _dotenv_value(env_var: str) -> str: def resolve_provider_secret( - env_var: str, - provider_id: str, - config_value: str = "", - env_getter=None, -) -> str: + env_var: str, provider_id: str, config_value: str = "", env_getter=None) -> str: """Resolve a voice-provider API key (single owner for STT/TTS lookup). Order: explicit ``config_value`` -> profile secret scope / env -> ``.env`` @@ -194,8 +182,7 @@ def resolve_openai_audio_api_key() -> str: a raw ``os.environ`` read could bill another profile's account under multiplex.""" return ( resolve_provider_secret("VOICE_TOOLS_OPENAI_KEY", "") - or resolve_provider_secret("OPENAI_API_KEY", "openai-api") - ) + or resolve_provider_secret("OPENAI_API_KEY", "openai-api")) def prefers_gateway(config_section: str) -> bool: @@ -215,16 +202,11 @@ def prefers_gateway(config_section: str) -> bool: NOUS_MANAGED_PROVIDER = "nous" # Per-capability keys that also count as "this category has been configured". -_EXTRA_SELECTION_KEYS = { - "web": ("search_backend", "extract_backend"), -} +_EXTRA_SELECTION_KEYS = {"web": ("search_backend", "extract_backend")} # Key(s) carrying the category's provider selection. ``browser.backend`` is the # DRIVER choice (browser-use CLI vs built-in), not the cloud provider — excluded. -_SELECTION_NAME_KEYS = { - "browser": ("cloud_provider",), - "web": ("backend",), -} +_SELECTION_NAME_KEYS = {"browser": ("cloud_provider",), "web": ("backend",)} _DEFAULT_NAME_KEYS = ("provider", "backend", "cloud_provider") @@ -298,8 +280,7 @@ def selection_error(section: str, selection_name: str, failure: str) -> str: failure = removed_backend_note(section, selection_name) or failure return ( f"{section} is configured to use {selection_name} (set via hermes " - f"tools), but {failure}. Run 'hermes tools' to change it." - ) + f"tools), but {failure}. Run 'hermes tools' to change it.") def fal_key_is_configured() -> bool: diff --git a/tools/tool_result_storage.py b/tools/tool_result_storage.py index 58cc91d906..775fcc4ca2 100644 --- a/tools/tool_result_storage.py +++ b/tools/tool_result_storage.py @@ -19,11 +19,7 @@ import shlex import threading import time -from tools.budget_config import ( - DEFAULT_PREVIEW_SIZE_CHARS, - BudgetConfig, - DEFAULT_BUDGET, -) +from tools.budget_config import DEFAULT_PREVIEW_SIZE_CHARS, BudgetConfig, DEFAULT_BUDGET logger = logging.getLogger(__name__) PERSISTED_OUTPUT_TAG = "" @@ -187,11 +183,7 @@ def _write_to_sandbox(content: str, remote_path: str, env) -> bool: def _build_persisted_message( - preview: str, - has_more: bool, - original_size: int, - file_path: str, -) -> str: + preview: str, has_more: bool, original_size: int, file_path: str) -> str: """Build the replacement block.""" size_kb = original_size / 1024 size_str = f"{size_kb / 1024:.1f} MB" if size_kb >= 1024 else f"{size_kb:.1f} KB" @@ -205,8 +197,7 @@ def _build_persisted_message( "remote API; the full result is already on disk.\n\n" f"Preview (first {len(preview)} chars):\n" + preview + ("\n..." if has_more else "") - + f"\n{PERSISTED_OUTPUT_CLOSING_TAG}" - ) + + f"\n{PERSISTED_OUTPUT_CLOSING_TAG}") _PERSISTED_PATH_RE = re.compile(r"^Full output saved to: (.+)$", re.MULTILINE) @@ -228,8 +219,7 @@ def maybe_persist_tool_result( tool_use_id: str, env=None, config: BudgetConfig = DEFAULT_BUDGET, - threshold: int | float | None = None, -) -> str: + threshold: int | float | None = None) -> str: """Layer 2: persist an oversized result, return preview + path. ``threshold`` overrides ``config.resolve_threshold(tool_name)``. Falls back to inline truncation when no write location succeeds.""" @@ -243,8 +233,7 @@ def maybe_persist_tool_result( def _persisted(path: str, host_suffix: str = "") -> str: logger.info( "Persisted large tool result: %s (%s, %d chars -> %s%s)", - tool_name, tool_use_id, len(content), path, host_suffix, - ) + tool_name, tool_use_id, len(content), path, host_suffix) return _build_persisted_message(preview, has_more, len(content), path) # Always persist host-side first: cache/spillover is the single canonical home. @@ -269,20 +258,15 @@ def maybe_persist_tool_result( logger.info( "Inline-truncating large tool result: %s (%d chars, no sandbox write)", - tool_name, len(content), - ) + tool_name, len(content)) return ( f"{preview}\n\n" f"[Truncated: tool response was {len(content):,} chars. " - f"Full output could not be saved to sandbox.]" - ) + f"Full output could not be saved to sandbox.]") def enforce_turn_budget( - tool_messages: list[dict], - env=None, - config: BudgetConfig = DEFAULT_BUDGET, -) -> list[dict]: + tool_messages: list[dict], env=None, config: BudgetConfig = DEFAULT_BUDGET) -> list[dict]: """Layer 3: persist the largest non-persisted results first until the turn's aggregate is under budget. Mutates the list in-place and returns it.""" candidates = [] @@ -303,8 +287,7 @@ def enforce_turn_budget( tool_use_id = tool_messages[idx].get("tool_call_id", f"budget_{idx}") replacement = maybe_persist_tool_result( content=content, tool_name=_BUDGET_TOOL_NAME, tool_use_id=tool_use_id, - env=env, config=config, threshold=0, - ) + env=env, config=config, threshold=0) if replacement != content: total_size += len(replacement) - size tool_messages[idx]["content"] = replacement diff --git a/tools/tool_search.py b/tools/tool_search.py index a10952f1e4..333683a070 100644 --- a/tools/tool_search.py +++ b/tools/tool_search.py @@ -22,25 +22,12 @@ from typing import Any, Dict, Iterable, List, Optional, Tuple from tools.registry import tool_error from tools.tool_search_names import ( # noqa: F401 — re-exported public names - BRIDGE_TOOL_NAMES, - TOOL_CALL_NAME, - TOOL_DESCRIBE_NAME, - TOOL_SEARCH_NAME, + BRIDGE_TOOL_NAMES, TOOL_CALL_NAME, TOOL_DESCRIBE_NAME, TOOL_SEARCH_NAME, ) from tools.tool_search_catalog import ( # noqa: F401 — re-exported public/test names - CHARS_PER_TOKEN, - CatalogEntry, - _corpus_stats, - _entry_search_text, - _fn, - _listing_group_label, - _registry_entry, - _short_desc, - _stem, - _tokenize, - build_catalog, - build_catalog_listing_with_form, - search_catalog, + CHARS_PER_TOKEN, CatalogEntry, _corpus_stats, _entry_search_text, _fn, + _listing_group_label, _registry_entry, _short_desc, _stem, _tokenize, + build_catalog, build_catalog_listing_with_form, search_catalog, ) from tools.tool_search_validation import validate_deferred_call_args # noqa: F401 @@ -94,9 +81,7 @@ class ToolSearchConfig: # A list replaces the curated default wholesale; anything else = curated. defer_tools=( frozenset(str(n).strip() for n in defer_raw if str(n).strip()) - if isinstance(defer_raw, (list, tuple, set)) else None - ), - ) + if isinstance(defer_raw, (list, tuple, set)) else None)) _TRI_STATE_ALIASES = {"true": "on", "1": "on", "yes": "on", "false": "off", "0": "off", "no": "off"} @@ -202,8 +187,7 @@ def _tool_def_names(tool_defs: Iterable[Dict[str, Any]]) -> Iterable[str]: def classify_tools( tool_defs: List[Dict[str, Any]], - defer_tools: Optional[frozenset] = None, -) -> Tuple[List[Dict[str, Any]], List[Dict[str, Any]]]: + defer_tools: Optional[frozenset] = None) -> Tuple[List[Dict[str, Any]], List[Dict[str, Any]]]: """Split a tool-defs list into (visible, deferrable). Bridge tools are dropped (they are re-added after classification).""" visible: List[Dict[str, Any]] = [] @@ -236,8 +220,7 @@ def estimate_tokens_from_schemas(tool_defs: Iterable[Dict[str, Any]]) -> int: def should_activate( config: ToolSearchConfig, deferrable_tokens: int, - context_length: Optional[int], -) -> bool: + context_length: Optional[int]) -> bool: """``"off"`` never activates; ``"on"``/``"auto"`` activate whenever any deferrable tool exists. ``"auto"`` is an alias of ``"on"`` reserved for a future budget-gated mode — do not distinguish them without that design. @@ -280,8 +263,7 @@ def _search_description(deferred_count: int, listing: Optional[str], listing_for "tool's description. Follow with " f"`{TOOL_DESCRIBE_NAME}` to load full parameter schemas, " f"then `{TOOL_CALL_NAME}` to invoke. Tools listed at the top of this " - "system prompt are already available and do not need to be searched." - ) + "system prompt are already available and do not need to be searched.") if not listing: return desc if listing_form == "groups": @@ -290,28 +272,24 @@ def _search_description(deferred_count: int, listing: Optional[str], listing_for "through this bridge. For any request in these domains, search " "here FIRST — do not claim the capability is unavailable and do " "not substitute a generic tool (terminal/browser) without " - "searching.\n\n" + listing - ) + "searching.\n\n" + listing) desc += ( "\n\nEvery deferred capability is listed below. If a tool name " "appears here, do NOT claim it is unavailable — load it with " f"`{TOOL_DESCRIBE_NAME}` (skip `{TOOL_SEARCH_NAME}` when you " - "already see the exact name)." - ) + "already see the exact name).") if listing_form == "mixed": desc += ( " For servers marked 'names not listed', the tools exist " f"too — find them with `{TOOL_SEARCH_NAME}` before " - "concluding anything is missing." - ) + "concluding anything is missing.") return desc + "\n\n" + listing def bridge_tool_schemas( deferred_count: int, listing: Optional[str] = None, - listing_form: str = "", -) -> List[Dict[str, Any]]: + listing_form: str = "") -> List[Dict[str, Any]]: """Bridge tool schemas injected in place of deferred tools. Kept short — every byte is paid on every turn. ``listing`` is embedded in the tool_search description; ``listing_form`` picks the framing (per-tool forms say "skip @@ -384,8 +362,7 @@ def assemble_tool_defs( tool_defs: List[Dict[str, Any]], *, context_length: Optional[int] = None, - config: Optional[ToolSearchConfig] = None, -) -> AssemblyResult: + config: Optional[ToolSearchConfig] = None) -> AssemblyResult: """Return the tool-defs list the model should actually see: passthrough when inactive, otherwise deferrable tools replaced by the three bridge tools. Idempotent — bridge tools already present are stripped first.""" @@ -403,8 +380,7 @@ def assemble_tool_defs( tool_defs=incoming, activated=False, deferred_count=len(deferrable), deferred_tokens=deferrable_tokens, threshold_tokens=int((context_length or 0) * (config.threshold_pct / 100.0)), - tier=0, - ) + tier=0) listing, listing_form = None, "none" listing_budget = listing_token_budget(config, context_length) @@ -416,13 +392,11 @@ def assemble_tool_defs( logger.info( "tool_search activated (tier %d): %d core/visible tools kept, %d deferred " "(~%d tokens), listing %s (budget ~%d tokens)", - tier, len(visible), len(deferrable), deferrable_tokens, listing_form, listing_budget, - ) + tier, len(visible), len(deferrable), deferrable_tokens, listing_form, listing_budget) return AssemblyResult( tool_defs=visible + bridge, activated=True, deferred_count=len(deferrable), deferred_tokens=deferrable_tokens, threshold_tokens=listing_budget, - tier=tier, listing_form=listing_form, - ) + tier=tier, listing_form=listing_form) def is_bridge_tool(name: str) -> bool: @@ -443,8 +417,7 @@ def _shared_tool_record(entry: CatalogEntry) -> Dict[str, Any]: "source": entry.source, "source_name": entry.source_name, "description": (entry.description or "")[:400], # cap chatty MCP descriptions - "required": [r[:64] for r in required if isinstance(r, str)][:32], - } + "required": [r[:64] for r in required if isinstance(r, str)][:32]} def _available_source_summary(catalog: List[CatalogEntry]) -> List[Dict[str, Any]]: @@ -519,8 +492,7 @@ def dispatch_tool_search(args: Dict[str, Any], "This query returned no lexical matches, but the sources above " "are connected and their tools remain available. Retry " "tool_search with the service name plus a concrete action or " - "object before concluding the capability is unavailable." - ) + "object before concluding the capability is unavailable.") results.append(group) return json.dumps({ "queries": queries, @@ -560,16 +532,13 @@ def dispatch_tool_describe(args: Dict[str, Any], if fn is not None: tools[name] = { "description": fn.get("description", ""), - "parameters": fn.get("parameters", {}), - } + "parameters": fn.get("parameters", {})} elif _registry_entry(name) is not None and not is_deferrable_tool_name( - name, load_config_readonly().effective_defer_tools - ): + name, load_config_readonly().effective_defer_tools): # Registered but bridge/core/GUI-surface: a real name, wrong door. errors[name] = ( f"'{name}' is not a deferrable tool. If you see it in the tools list " - "already, call it directly; otherwise check the spelling against tool_search." - ) + "already, call it directly; otherwise check the spelling against tool_search.") else: not_found.append(name) @@ -590,8 +559,7 @@ def scoped_deferrable_names(tool_defs: List[Dict[str, Any]]) -> frozenset[str]: defer_tools = load_config_readonly().effective_defer_tools return frozenset( name for name in _tool_def_names(tool_defs) - if name and is_deferrable_tool_name(name, defer_tools) - ) + if name and is_deferrable_tool_name(name, defer_tools)) def resolve_underlying_call(args: Dict[str, Any]) -> Tuple[Optional[str], Dict[str, Any], Optional[str]]: @@ -616,34 +584,16 @@ def resolve_underlying_call(args: Dict[str, Any]) -> Tuple[Optional[str], Dict[s if not is_deferrable_tool_name(name, load_config_readonly().effective_defer_tools): return None, {}, ( f"'{name}' is not a deferrable tool. If it appears in the model-facing tools " - "list already, call it directly instead of via tool_call." - ) + "list already, call it directly instead of via tool_call.") return name, raw_args, None __all__ = [ - "TOOL_SEARCH_NAME", - "TOOL_DESCRIBE_NAME", - "TOOL_CALL_NAME", - "BRIDGE_TOOL_NAMES", - "ToolSearchConfig", - "CatalogEntry", - "AssemblyResult", - "load_config", - "is_deferrable_tool_name", - "classify_tools", - "estimate_tokens_from_schemas", - "should_activate", - "build_catalog", - "build_catalog_listing_with_form", - "listing_token_budget", - "search_catalog", - "bridge_tool_schemas", - "assemble_tool_defs", - "is_bridge_tool", - "dispatch_tool_search", - "dispatch_tool_describe", - "resolve_underlying_call", - "scoped_deferrable_names", - "validate_deferred_call_args", + "TOOL_SEARCH_NAME", "TOOL_DESCRIBE_NAME", "TOOL_CALL_NAME", "BRIDGE_TOOL_NAMES", + "ToolSearchConfig", "CatalogEntry", "AssemblyResult", "load_config", + "is_deferrable_tool_name", "classify_tools", "estimate_tokens_from_schemas", + "should_activate", "build_catalog", "build_catalog_listing_with_form", "listing_token_budget", + "search_catalog", "bridge_tool_schemas", "assemble_tool_defs", "is_bridge_tool", + "dispatch_tool_search", "dispatch_tool_describe", "resolve_underlying_call", + "scoped_deferrable_names", "validate_deferred_call_args", ] diff --git a/tools/tool_search_catalog.py b/tools/tool_search_catalog.py index f371d047f9..986c73f672 100644 --- a/tools/tool_search_catalog.py +++ b/tools/tool_search_catalog.py @@ -122,8 +122,7 @@ def build_catalog(tool_defs: List[Dict[str, Any]]) -> List[CatalogEntry]: schema=td, source=source, source_name=source_name, - _tokens=_tokenize(_entry_search_text(td, source_label)), - )) + _tokens=_tokenize(_entry_search_text(td, source_label)))) return catalog @@ -167,8 +166,7 @@ def search_catalog( query: str, limit: int = 5, *, - corpus_stats: Optional[_CorpusStats] = None, -) -> List[CatalogEntry]: + corpus_stats: Optional[_CorpusStats] = None) -> List[CatalogEntry]: """Top-``limit`` catalog entries for ``query`` by BM25 (exact name match ranks first). Falls back to a name-substring match only when NO query token appears in any document (e.g. "hub" vs ``github_*``); the IDF @@ -226,10 +224,7 @@ def _listing_group_label(source_name: str) -> str: def build_catalog_listing_with_form( - deferrable: List[Dict[str, Any]], - *, - max_tokens: int = 4000, -) -> Tuple[Optional[str], str]: + deferrable: List[Dict[str, Any]], *, max_tokens: int = 4000) -> Tuple[Optional[str], str]: """Render the skills-style deferred-catalog manifest: ``- name: short desc`` lines grouped under a heading per source (MCP server / plugin toolset). diff --git a/tools/tool_search_validation.py b/tools/tool_search_validation.py index 39d8658253..17e3513516 100644 --- a/tools/tool_search_validation.py +++ b/tools/tool_search_validation.py @@ -25,8 +25,7 @@ def _schema_for_local_validation(node: Any) -> Any: # Literal keywords hold instance data, not schemas: copy byte-for-byte. normalized = { key: copy.deepcopy(value) if key in _SCHEMA_LITERAL_KEYS else _schema_for_local_validation(value) - for key, value in node.items() if key != "nullable" - } + for key, value in node.items() if key != "nullable"} if node.get("nullable") is not True: return normalized schema_type = normalized.get("type") @@ -56,8 +55,7 @@ def _schema_has_external_ref(node: Any) -> bool: return any( _schema_has_external_ref(value) for key, value in node.items() - if key not in _SCHEMA_LITERAL_KEYS - ) + if key not in _SCHEMA_LITERAL_KEYS) def _validation_path(error: Any) -> str: @@ -76,8 +74,7 @@ def _validation_path(error: Any) -> str: def _validation_error(message: str, *, path: str, constraint: str, parameters: Any) -> str: return tool_error( message, path=path, constraint=constraint, parameters=parameters, - hint="Retry tool_call with 'arguments' matching the parameters schema above.", - ) + hint="Retry tool_call with 'arguments' matching the parameters schema above.") def validate_deferred_call_args(name: str, args: Dict[str, Any]) -> Optional[str]: @@ -106,8 +103,7 @@ def validate_deferred_call_args(name: str, args: Dict[str, Any]) -> Optional[str return _validation_error( f"tool_call to '{name}' is missing required argument(s): " f"{', '.join(missing)}. The tool was NOT invoked.", - path="arguments", constraint="required", parameters=params, - ) + path="arguments", constraint="required", parameters=params) validation_schema = _schema_for_local_validation(params) if _schema_has_external_ref(validation_schema): @@ -143,8 +139,7 @@ def validate_deferred_call_args(name: str, args: Dict[str, Any]) -> Optional[str return _validation_error( f"tool_call to '{name}' failed argument validation at {path} " f"({constraint}): {detail}. The tool was NOT invoked.", - path=path, constraint=constraint, parameters=params, - ) + path=path, constraint=constraint, parameters=params) except Exception: # pragma: no cover — never block dispatch on validator bugs logger.debug("validate_deferred_call_args failed for %s", name, exc_info=True) return None