mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-09 03:11:58 +00:00
perf(cli): skip eager plugin discovery on known built-in subcommands (#22120)
`hermes --help` drops from ~700ms to ~180ms; `hermes version` from ~950ms to ~240ms. ~4-5x startup speedup on inspection / diagnostic invocations. Changes: - hermes_cli/main.py: gate the argparse-setup `discover_plugins()` call behind `_plugin_cli_discovery_needed()`. Eager plugin imports (google.cloud.pubsub_v1, aiohttp, grpc, PIL) cost 500-650ms and are pure waste when the user is running a built-in subcommand that doesn't take plugin extensions (`--help`, `version`, `logs`, `config`, `sessions`, etc.). New `_BUILTIN_SUBCOMMANDS` frozenset + `_first_positional_argv` helper handle flag-value skipping (`-m gpt5 chat` → still fast). - hermes_cli/main.py: `cmd_version` now reads the OpenAI SDK version via `importlib.metadata` (~2ms) instead of `import openai` (~800ms of pydantic type-module loading). Agent-running paths (`hermes chat`, `hermes gateway run`) are unaffected — the second `discover_plugins()` call later in `main()` still runs so plugin hooks / tools wire up normally. Tests: - tests/hermes_cli/test_startup_plugin_gating.py: parity test guards the `_BUILTIN_SUBCOMMANDS` set against drift (every registered subparser must be declared; no phantom entries). Behavior tests for flag-value skipping, `--` terminator, inline `--flag=value` form. 37 tests.
This commit is contained in:
parent
7a4d5c123a
commit
5089596685
2 changed files with 332 additions and 33 deletions
|
|
@ -5363,11 +5363,16 @@ def cmd_version(args):
|
|||
# Show Python version
|
||||
print(f"Python: {sys.version.split()[0]}")
|
||||
|
||||
# Check for key dependencies
|
||||
# Check for key dependencies. Use importlib.metadata rather than
|
||||
# ``import openai`` — the SDK drags in ~800ms of pydantic-backed type
|
||||
# modules just to expose ``__version__``. Metadata lookup is ~2ms.
|
||||
try:
|
||||
import openai
|
||||
from importlib.metadata import version as _pkg_version, PackageNotFoundError
|
||||
|
||||
print(f"OpenAI SDK: {openai.__version__}")
|
||||
try:
|
||||
print(f"OpenAI SDK: {_pkg_version('openai')}")
|
||||
except PackageNotFoundError:
|
||||
print("OpenAI SDK: Not installed")
|
||||
except ImportError:
|
||||
print("OpenAI SDK: Not installed")
|
||||
|
||||
|
|
@ -8793,6 +8798,113 @@ def _build_provider_choices() -> list[str]:
|
|||
]
|
||||
|
||||
|
||||
# Top-level subcommands that argparse knows about WITHOUT running plugin
|
||||
# discovery. Used to short-circuit eager plugin imports (which can take
|
||||
# 500ms+ pulling in google.cloud.pubsub_v1, aiohttp, grpc, etc.) when the
|
||||
# user's invocation clearly doesn't need any plugin-registered subcommand.
|
||||
#
|
||||
# Keep this in sync with the ``subparsers.add_parser("NAME", ...)`` calls
|
||||
# below in ``main()``. Missing an entry here only costs a one-time
|
||||
# discovery; extra entries here would let a plugin command silently fail
|
||||
# to parse.
|
||||
_BUILTIN_SUBCOMMANDS = frozenset(
|
||||
{
|
||||
"acp", "auth", "backup", "checkpoints", "claw", "completion",
|
||||
"config", "cron", "curator", "dashboard", "debug", "doctor",
|
||||
"dump", "fallback", "gateway", "hooks", "import", "insights",
|
||||
"kanban", "login", "logout", "logs", "mcp", "memory", "model",
|
||||
"pairing", "plugins", "profile", "sessions", "setup", "skills",
|
||||
"slack", "status", "tools", "uninstall", "update", "version",
|
||||
"webhook", "whatsapp", "chat",
|
||||
# Help-ish invocations — plugin commands not being listed in
|
||||
# top-level --help is an acceptable trade-off for skipping an
|
||||
# expensive eager import of every bundled plugin module.
|
||||
"help",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
# Top-level flags that take a value. Needed by ``_first_positional_argv``
|
||||
# so that in ``hermes -m gpt5 chat``, ``gpt5`` is correctly skipped as a
|
||||
# flag value rather than misclassified as a subcommand. Kept in sync with
|
||||
# the top-level flags declared in ``hermes_cli/_parser.py``.
|
||||
#
|
||||
# Correctness-safe either way: missing an entry here only makes the
|
||||
# fast-path bail out too eagerly (we run plugin discovery when we didn't
|
||||
# need to); extra entries would make us skip a real positional.
|
||||
_TOP_LEVEL_VALUE_FLAGS = frozenset(
|
||||
{
|
||||
"-z", "--oneshot",
|
||||
"-m", "--model",
|
||||
"--provider",
|
||||
"-t", "--toolsets",
|
||||
"-r", "--resume",
|
||||
"-s", "--skills",
|
||||
# ``-c / --continue`` is nargs='?' (optional value). Treat it as
|
||||
# value-taking: if the next token is a subcommand-looking word
|
||||
# the user almost certainly meant it as the session name, and
|
||||
# either interpretation keeps us on the safe side.
|
||||
"-c", "--continue",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _first_positional_argv() -> str | None:
|
||||
"""Return the first non-flag, non-flag-value token in ``sys.argv[1:]``.
|
||||
|
||||
Used by ``main()`` to decide whether plugin discovery has to run at
|
||||
argparse-setup time. Handles common invocations like
|
||||
``hermes -m gpt5 --provider openai chat "msg"`` by skipping the
|
||||
values attached to known top-level flags.
|
||||
|
||||
Does NOT fully simulate argparse — unknown ``--foo=bar`` / ``--foo
|
||||
bar`` flags degrade gracefully (``bar`` may be wrongly classified as
|
||||
a positional, which at worst forces a one-time plugin discovery).
|
||||
"""
|
||||
argv = sys.argv[1:]
|
||||
i = 0
|
||||
while i < len(argv):
|
||||
tok = argv[i]
|
||||
if tok == "--":
|
||||
# Everything after ``--`` is positional.
|
||||
if i + 1 < len(argv):
|
||||
return argv[i + 1]
|
||||
return None
|
||||
if tok.startswith("-"):
|
||||
# ``--flag=value`` carries its value inline — single token.
|
||||
if "=" in tok:
|
||||
i += 1
|
||||
continue
|
||||
if tok in _TOP_LEVEL_VALUE_FLAGS and i + 1 < len(argv):
|
||||
i += 2
|
||||
continue
|
||||
i += 1
|
||||
continue
|
||||
return tok
|
||||
return None
|
||||
|
||||
|
||||
def _plugin_cli_discovery_needed() -> bool:
|
||||
"""True when the CLI might be invoking a plugin-registered subcommand.
|
||||
|
||||
Returning False lets ``main()`` skip plugin discovery entirely during
|
||||
argparse setup, saving ~500-650ms per invocation for users whose
|
||||
enabled plugins don't contribute any CLI command.
|
||||
"""
|
||||
first = _first_positional_argv()
|
||||
if first is None:
|
||||
# Bare ``hermes`` or only flags → defaults to ``chat``.
|
||||
return False
|
||||
if first in _BUILTIN_SUBCOMMANDS:
|
||||
return False
|
||||
# Unknown token — could be a plugin subcommand, OR a chat prompt
|
||||
# starting with a non-flag word. Either way we need discovery: if it
|
||||
# IS a plugin command, argparse needs the subparser; if it's a chat
|
||||
# prompt, argparse will route it via positional handling and the
|
||||
# extra discovery cost is amortized over a full agent run anyway.
|
||||
return True
|
||||
|
||||
|
||||
def main():
|
||||
"""Main entry point for hermes CLI."""
|
||||
# Force UTF-8 stdio on Windows before anything prints. No-op elsewhere.
|
||||
|
|
@ -10077,39 +10189,46 @@ Examples:
|
|||
# Plugin CLI commands — dynamically registered by memory/general plugins.
|
||||
# Plugins provide a register_cli(subparser) function that builds their
|
||||
# own argparse tree. No hardcoded plugin commands in main.py.
|
||||
#
|
||||
# Skipped when the invocation is already targeting a known built-in
|
||||
# subcommand — ``hermes --help``, ``hermes version``, ``hermes logs``,
|
||||
# etc. This avoids eagerly importing every bundled plugin module
|
||||
# (google.cloud.pubsub_v1, aiohttp, grpc, PIL …) which costs
|
||||
# 500-650ms on typical installs.
|
||||
# =========================================================================
|
||||
try:
|
||||
from plugins.memory import discover_plugin_cli_commands
|
||||
from hermes_cli.plugins import discover_plugins, get_plugin_manager
|
||||
if _plugin_cli_discovery_needed():
|
||||
try:
|
||||
from plugins.memory import discover_plugin_cli_commands
|
||||
from hermes_cli.plugins import discover_plugins, get_plugin_manager
|
||||
|
||||
seen_plugin_commands = set()
|
||||
for cmd_info in discover_plugin_cli_commands():
|
||||
plugin_parser = subparsers.add_parser(
|
||||
cmd_info["name"],
|
||||
help=cmd_info["help"],
|
||||
description=cmd_info.get("description", ""),
|
||||
formatter_class=__import__("argparse").RawDescriptionHelpFormatter,
|
||||
)
|
||||
cmd_info["setup_fn"](plugin_parser)
|
||||
if cmd_info.get("handler_fn") is not None:
|
||||
plugin_parser.set_defaults(func=cmd_info["handler_fn"])
|
||||
seen_plugin_commands.add(cmd_info["name"])
|
||||
seen_plugin_commands = set()
|
||||
for cmd_info in discover_plugin_cli_commands():
|
||||
plugin_parser = subparsers.add_parser(
|
||||
cmd_info["name"],
|
||||
help=cmd_info["help"],
|
||||
description=cmd_info.get("description", ""),
|
||||
formatter_class=__import__("argparse").RawDescriptionHelpFormatter,
|
||||
)
|
||||
cmd_info["setup_fn"](plugin_parser)
|
||||
if cmd_info.get("handler_fn") is not None:
|
||||
plugin_parser.set_defaults(func=cmd_info["handler_fn"])
|
||||
seen_plugin_commands.add(cmd_info["name"])
|
||||
|
||||
discover_plugins()
|
||||
for cmd_info in get_plugin_manager()._cli_commands.values():
|
||||
if cmd_info["name"] in seen_plugin_commands:
|
||||
continue
|
||||
plugin_parser = subparsers.add_parser(
|
||||
cmd_info["name"],
|
||||
help=cmd_info["help"],
|
||||
description=cmd_info.get("description", ""),
|
||||
formatter_class=__import__("argparse").RawDescriptionHelpFormatter,
|
||||
)
|
||||
cmd_info["setup_fn"](plugin_parser)
|
||||
if cmd_info.get("handler_fn") is not None:
|
||||
plugin_parser.set_defaults(func=cmd_info["handler_fn"])
|
||||
except Exception as _exc:
|
||||
logging.getLogger(__name__).debug("Plugin CLI discovery failed: %s", _exc)
|
||||
discover_plugins()
|
||||
for cmd_info in get_plugin_manager()._cli_commands.values():
|
||||
if cmd_info["name"] in seen_plugin_commands:
|
||||
continue
|
||||
plugin_parser = subparsers.add_parser(
|
||||
cmd_info["name"],
|
||||
help=cmd_info["help"],
|
||||
description=cmd_info.get("description", ""),
|
||||
formatter_class=__import__("argparse").RawDescriptionHelpFormatter,
|
||||
)
|
||||
cmd_info["setup_fn"](plugin_parser)
|
||||
if cmd_info.get("handler_fn") is not None:
|
||||
plugin_parser.set_defaults(func=cmd_info["handler_fn"])
|
||||
except Exception as _exc:
|
||||
logging.getLogger(__name__).debug("Plugin CLI discovery failed: %s", _exc)
|
||||
|
||||
# =========================================================================
|
||||
# curator command — background skill maintenance
|
||||
|
|
|
|||
180
tests/hermes_cli/test_startup_plugin_gating.py
Normal file
180
tests/hermes_cli/test_startup_plugin_gating.py
Normal file
|
|
@ -0,0 +1,180 @@
|
|||
"""Guards for CLI startup performance regression.
|
||||
|
||||
``hermes_cli.main`` skips eager plugin discovery at argparse-setup time
|
||||
when the invocation is clearly targeting a known built-in subcommand.
|
||||
This saves 500-650ms on ``hermes --help``, ``hermes version``,
|
||||
``hermes logs``, etc., by not importing ``google.cloud.pubsub_v1``,
|
||||
``aiohttp``, ``grpc``, and friends.
|
||||
|
||||
Two invariants:
|
||||
|
||||
1. ``_BUILTIN_SUBCOMMANDS`` must contain every subcommand that is actually
|
||||
registered by ``main()``. If an entry is missing, plugin discovery
|
||||
runs unnecessarily for that command (correctness-safe, just slow).
|
||||
If an entry is PRESENT but the subcommand doesn't exist, a plugin
|
||||
could shadow the name — also bad.
|
||||
|
||||
2. ``_plugin_cli_discovery_needed()`` returns the right answer for the
|
||||
flag/positional parsing cases it's meant to handle.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
import re
|
||||
import sys
|
||||
from contextlib import redirect_stdout
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli.main import (
|
||||
_BUILTIN_SUBCOMMANDS,
|
||||
_first_positional_argv,
|
||||
_plugin_cli_discovery_needed,
|
||||
)
|
||||
|
||||
|
||||
# ── helper: grab the live set of top-level subcommands from argparse ───────
|
||||
|
||||
|
||||
def _live_subcommand_names() -> set[str]:
|
||||
"""Run ``hermes --help`` in-process and parse the subcommand block.
|
||||
|
||||
We patch ``_plugin_cli_discovery_needed`` to always return False so
|
||||
plugin-registered commands aren't included — we're validating the
|
||||
built-in-only set.
|
||||
"""
|
||||
from hermes_cli import main as _main
|
||||
|
||||
argv_backup = sys.argv[:]
|
||||
sys.argv = ["hermes", "--help"]
|
||||
buf = io.StringIO()
|
||||
try:
|
||||
with patch.object(_main, "_plugin_cli_discovery_needed", return_value=False):
|
||||
with redirect_stdout(buf):
|
||||
with pytest.raises(SystemExit):
|
||||
_main.main()
|
||||
finally:
|
||||
sys.argv = argv_backup
|
||||
|
||||
text = buf.getvalue()
|
||||
# argparse prints "{chat,model,...}" somewhere in the help output
|
||||
m = re.search(r"\{([a-zA-Z0-9_,\-]+)\}", text)
|
||||
assert m, f"Could not find subcommand group in --help output:\n{text[:500]}"
|
||||
return set(m.group(1).split(","))
|
||||
|
||||
|
||||
# ── _first_positional_argv ─────────────────────────────────────────────────
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"argv,expected",
|
||||
[
|
||||
(["hermes"], None),
|
||||
(["hermes", "--help"], None),
|
||||
(["hermes", "-h"], None),
|
||||
(["hermes", "--version"], None),
|
||||
(["hermes", "-w"], None),
|
||||
# -p / --profile is stripped from sys.argv by
|
||||
# _apply_profile_override() at import time, so it never reaches
|
||||
# _first_positional_argv. We test with just -w / --tui here.
|
||||
(["hermes", "-w", "--tui"], None),
|
||||
(["hermes", "version"], "version"),
|
||||
(["hermes", "--tui", "chat"], "chat"),
|
||||
(["hermes", "-w", "logs"], "logs"),
|
||||
(["hermes", "chat", "hello world"], "chat"),
|
||||
(["hermes", "gateway", "run"], "gateway"),
|
||||
# Top-level value-taking flags: the value should be skipped.
|
||||
(["hermes", "-m", "gpt5", "chat"], "chat"),
|
||||
(["hermes", "--model", "gpt5", "chat", "hi"], "chat"),
|
||||
(["hermes", "-m", "gpt5", "--provider", "openai", "chat"], "chat"),
|
||||
(["hermes", "-z", "hello world"], None),
|
||||
(["hermes", "-z", "hello", "chat"], "chat"),
|
||||
(["hermes", "--model=gpt5", "chat"], "chat"), # inline form
|
||||
(["hermes", "--", "chat"], "chat"), # -- terminator
|
||||
(["hermes", "-w", "--"], None),
|
||||
# Unknown positional after skipped flags → plugin-cmd candidate.
|
||||
(["hermes", "some-plugin-cmd"], "some-plugin-cmd"),
|
||||
(["hermes", "-m", "gpt5", "some-plugin-cmd"], "some-plugin-cmd"),
|
||||
],
|
||||
)
|
||||
def test_first_positional_argv(argv, expected):
|
||||
with patch.object(sys, "argv", argv):
|
||||
assert _first_positional_argv() == expected
|
||||
|
||||
|
||||
# ── _plugin_cli_discovery_needed ───────────────────────────────────────────
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"argv",
|
||||
[
|
||||
["hermes"], # bare → chat
|
||||
["hermes", "--help"], # top-level help
|
||||
["hermes", "-h"],
|
||||
["hermes", "version"], # known built-in
|
||||
["hermes", "logs"],
|
||||
["hermes", "gateway", "run"],
|
||||
["hermes", "--tui"],
|
||||
["hermes", "-w", "--tui"],
|
||||
["hermes", "chat", "hi"],
|
||||
["hermes", "help"], # accepted built-in-ish
|
||||
["hermes", "-m", "gpt5", "chat"], # flag-value-skipping
|
||||
],
|
||||
)
|
||||
def test_discovery_skipped_for_builtins(argv):
|
||||
with patch.object(sys, "argv", argv):
|
||||
assert _plugin_cli_discovery_needed() is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"argv",
|
||||
[
|
||||
["hermes", "meet", "join"], # potential google_meet plugin
|
||||
["hermes", "honcho", "status"], # potential memory plugin
|
||||
["hermes", "unknown-subcmd"],
|
||||
],
|
||||
)
|
||||
def test_discovery_runs_for_unknown_positional(argv):
|
||||
with patch.object(sys, "argv", argv):
|
||||
assert _plugin_cli_discovery_needed() is True
|
||||
|
||||
|
||||
# ── _BUILTIN_SUBCOMMANDS ↔ argparse registration parity ────────────────────
|
||||
|
||||
|
||||
def test_builtin_set_covers_every_registered_subcommand():
|
||||
"""Every subcommand registered in main() must appear in the set.
|
||||
|
||||
Missing entries cause a slow-path regression (correctness stays
|
||||
fine — discovery just runs unnecessarily).
|
||||
"""
|
||||
live = _live_subcommand_names()
|
||||
# "help" is synthetic — an argparse-implicit convenience we include
|
||||
# in the set so ``hermes help <cmd>`` skips discovery; it won't show
|
||||
# up as a subparser in the --help output.
|
||||
declared = _BUILTIN_SUBCOMMANDS - {"help"}
|
||||
missing_from_declaration = live - declared
|
||||
assert not missing_from_declaration, (
|
||||
f"_BUILTIN_SUBCOMMANDS is missing these live subcommands: "
|
||||
f"{sorted(missing_from_declaration)}. Add them to "
|
||||
f"hermes_cli/main.py::_BUILTIN_SUBCOMMANDS so plugin discovery "
|
||||
f"can be skipped when the user targets them."
|
||||
)
|
||||
|
||||
|
||||
def test_builtin_set_has_no_phantom_entries():
|
||||
"""No entry in the set should refer to a subcommand that no longer exists.
|
||||
|
||||
A phantom entry means plugin discovery gets incorrectly skipped for
|
||||
a name that — if a plugin actually registered it — would fail to
|
||||
parse. Keeps the set honest.
|
||||
"""
|
||||
live = _live_subcommand_names()
|
||||
allowed_synthetic = {"help"}
|
||||
phantom = _BUILTIN_SUBCOMMANDS - live - allowed_synthetic
|
||||
assert not phantom, (
|
||||
f"_BUILTIN_SUBCOMMANDS has entries that are not registered as "
|
||||
f"top-level subparsers: {sorted(phantom)}"
|
||||
)
|
||||
Loading…
Add table
Add a link
Reference in a new issue