Skip to content

Commit ddbdbe7

Browse files
teknium1Ryan
authored andcommitted
perf(cli): skip eager plugin discovery on known built-in subcommands (NousResearch#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.
1 parent f78e48f commit ddbdbe7

2 files changed

Lines changed: 334 additions & 35 deletions

File tree

hermes_cli/main.py

Lines changed: 154 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5363,11 +5363,16 @@ def cmd_version(args):
53635363
# Show Python version
53645364
print(f"Python: {sys.version.split()[0]}")
53655365

5366-
# Check for key dependencies
5366+
# Check for key dependencies. Use importlib.metadata rather than
5367+
# ``import openai`` — the SDK drags in ~800ms of pydantic-backed type
5368+
# modules just to expose ``__version__``. Metadata lookup is ~2ms.
53675369
try:
5368-
import openai
5370+
from importlib.metadata import version as _pkg_version, PackageNotFoundError
53695371

5370-
print(f"OpenAI SDK: {openai.__version__}")
5372+
try:
5373+
print(f"OpenAI SDK: {_pkg_version('openai')}")
5374+
except PackageNotFoundError:
5375+
print("OpenAI SDK: Not installed")
53715376
except ImportError:
53725377
print("OpenAI SDK: Not installed")
53735378

@@ -8793,6 +8798,113 @@ def _build_provider_choices() -> list[str]:
87938798
]
87948799

87958800

8801+
# Top-level subcommands that argparse knows about WITHOUT running plugin
8802+
# discovery. Used to short-circuit eager plugin imports (which can take
8803+
# 500ms+ pulling in google.cloud.pubsub_v1, aiohttp, grpc, etc.) when the
8804+
# user's invocation clearly doesn't need any plugin-registered subcommand.
8805+
#
8806+
# Keep this in sync with the ``subparsers.add_parser("NAME", ...)`` calls
8807+
# below in ``main()``. Missing an entry here only costs a one-time
8808+
# discovery; extra entries here would let a plugin command silently fail
8809+
# to parse.
8810+
_BUILTIN_SUBCOMMANDS = frozenset(
8811+
{
8812+
"acp", "auth", "backup", "checkpoints", "claw", "completion",
8813+
"config", "cron", "curator", "dashboard", "debug", "doctor",
8814+
"dump", "fallback", "gateway", "hooks", "import", "insights",
8815+
"kanban", "login", "logout", "logs", "mcp", "memory", "model",
8816+
"pairing", "plugins", "profile", "sessions", "setup", "skills",
8817+
"slack", "status", "tools", "uninstall", "update", "version",
8818+
"webhook", "whatsapp", "chat",
8819+
# Help-ish invocations — plugin commands not being listed in
8820+
# top-level --help is an acceptable trade-off for skipping an
8821+
# expensive eager import of every bundled plugin module.
8822+
"help",
8823+
}
8824+
)
8825+
8826+
8827+
# Top-level flags that take a value. Needed by ``_first_positional_argv``
8828+
# so that in ``hermes -m gpt5 chat``, ``gpt5`` is correctly skipped as a
8829+
# flag value rather than misclassified as a subcommand. Kept in sync with
8830+
# the top-level flags declared in ``hermes_cli/_parser.py``.
8831+
#
8832+
# Correctness-safe either way: missing an entry here only makes the
8833+
# fast-path bail out too eagerly (we run plugin discovery when we didn't
8834+
# need to); extra entries would make us skip a real positional.
8835+
_TOP_LEVEL_VALUE_FLAGS = frozenset(
8836+
{
8837+
"-z", "--oneshot",
8838+
"-m", "--model",
8839+
"--provider",
8840+
"-t", "--toolsets",
8841+
"-r", "--resume",
8842+
"-s", "--skills",
8843+
# ``-c / --continue`` is nargs='?' (optional value). Treat it as
8844+
# value-taking: if the next token is a subcommand-looking word
8845+
# the user almost certainly meant it as the session name, and
8846+
# either interpretation keeps us on the safe side.
8847+
"-c", "--continue",
8848+
}
8849+
)
8850+
8851+
8852+
def _first_positional_argv() -> str | None:
8853+
"""Return the first non-flag, non-flag-value token in ``sys.argv[1:]``.
8854+
8855+
Used by ``main()`` to decide whether plugin discovery has to run at
8856+
argparse-setup time. Handles common invocations like
8857+
``hermes -m gpt5 --provider openai chat "msg"`` by skipping the
8858+
values attached to known top-level flags.
8859+
8860+
Does NOT fully simulate argparse — unknown ``--foo=bar`` / ``--foo
8861+
bar`` flags degrade gracefully (``bar`` may be wrongly classified as
8862+
a positional, which at worst forces a one-time plugin discovery).
8863+
"""
8864+
argv = sys.argv[1:]
8865+
i = 0
8866+
while i < len(argv):
8867+
tok = argv[i]
8868+
if tok == "--":
8869+
# Everything after ``--`` is positional.
8870+
if i + 1 < len(argv):
8871+
return argv[i + 1]
8872+
return None
8873+
if tok.startswith("-"):
8874+
# ``--flag=value`` carries its value inline — single token.
8875+
if "=" in tok:
8876+
i += 1
8877+
continue
8878+
if tok in _TOP_LEVEL_VALUE_FLAGS and i + 1 < len(argv):
8879+
i += 2
8880+
continue
8881+
i += 1
8882+
continue
8883+
return tok
8884+
return None
8885+
8886+
8887+
def _plugin_cli_discovery_needed() -> bool:
8888+
"""True when the CLI might be invoking a plugin-registered subcommand.
8889+
8890+
Returning False lets ``main()`` skip plugin discovery entirely during
8891+
argparse setup, saving ~500-650ms per invocation for users whose
8892+
enabled plugins don't contribute any CLI command.
8893+
"""
8894+
first = _first_positional_argv()
8895+
if first is None:
8896+
# Bare ``hermes`` or only flags → defaults to ``chat``.
8897+
return False
8898+
if first in _BUILTIN_SUBCOMMANDS:
8899+
return False
8900+
# Unknown token — could be a plugin subcommand, OR a chat prompt
8901+
# starting with a non-flag word. Either way we need discovery: if it
8902+
# IS a plugin command, argparse needs the subparser; if it's a chat
8903+
# prompt, argparse will route it via positional handling and the
8904+
# extra discovery cost is amortized over a full agent run anyway.
8905+
return True
8906+
8907+
87968908
def main():
87978909
"""Main entry point for hermes CLI."""
87988910
# Force UTF-8 stdio on Windows before anything prints. No-op elsewhere.
@@ -10077,39 +10189,46 @@ def cmd_plugins(args):
1007710189
# Plugin CLI commands — dynamically registered by memory/general plugins.
1007810190
# Plugins provide a register_cli(subparser) function that builds their
1007910191
# own argparse tree. No hardcoded plugin commands in main.py.
10192+
#
10193+
# Skipped when the invocation is already targeting a known built-in
10194+
# subcommand — ``hermes --help``, ``hermes version``, ``hermes logs``,
10195+
# etc. This avoids eagerly importing every bundled plugin module
10196+
# (google.cloud.pubsub_v1, aiohttp, grpc, PIL …) which costs
10197+
# 500-650ms on typical installs.
1008010198
# =========================================================================
10081-
try:
10082-
from plugins.memory import discover_plugin_cli_commands
10083-
from hermes_cli.plugins import discover_plugins, get_plugin_manager
10084-
10085-
seen_plugin_commands = set()
10086-
for cmd_info in discover_plugin_cli_commands():
10087-
plugin_parser = subparsers.add_parser(
10088-
cmd_info["name"],
10089-
help=cmd_info["help"],
10090-
description=cmd_info.get("description", ""),
10091-
formatter_class=__import__("argparse").RawDescriptionHelpFormatter,
10092-
)
10093-
cmd_info["setup_fn"](plugin_parser)
10094-
if cmd_info.get("handler_fn") is not None:
10095-
plugin_parser.set_defaults(func=cmd_info["handler_fn"])
10096-
seen_plugin_commands.add(cmd_info["name"])
10097-
10098-
discover_plugins()
10099-
for cmd_info in get_plugin_manager()._cli_commands.values():
10100-
if cmd_info["name"] in seen_plugin_commands:
10101-
continue
10102-
plugin_parser = subparsers.add_parser(
10103-
cmd_info["name"],
10104-
help=cmd_info["help"],
10105-
description=cmd_info.get("description", ""),
10106-
formatter_class=__import__("argparse").RawDescriptionHelpFormatter,
10107-
)
10108-
cmd_info["setup_fn"](plugin_parser)
10109-
if cmd_info.get("handler_fn") is not None:
10110-
plugin_parser.set_defaults(func=cmd_info["handler_fn"])
10111-
except Exception as _exc:
10112-
logging.getLogger(__name__).debug("Plugin CLI discovery failed: %s", _exc)
10199+
if _plugin_cli_discovery_needed():
10200+
try:
10201+
from plugins.memory import discover_plugin_cli_commands
10202+
from hermes_cli.plugins import discover_plugins, get_plugin_manager
10203+
10204+
seen_plugin_commands = set()
10205+
for cmd_info in discover_plugin_cli_commands():
10206+
plugin_parser = subparsers.add_parser(
10207+
cmd_info["name"],
10208+
help=cmd_info["help"],
10209+
description=cmd_info.get("description", ""),
10210+
formatter_class=__import__("argparse").RawDescriptionHelpFormatter,
10211+
)
10212+
cmd_info["setup_fn"](plugin_parser)
10213+
if cmd_info.get("handler_fn") is not None:
10214+
plugin_parser.set_defaults(func=cmd_info["handler_fn"])
10215+
seen_plugin_commands.add(cmd_info["name"])
10216+
10217+
discover_plugins()
10218+
for cmd_info in get_plugin_manager()._cli_commands.values():
10219+
if cmd_info["name"] in seen_plugin_commands:
10220+
continue
10221+
plugin_parser = subparsers.add_parser(
10222+
cmd_info["name"],
10223+
help=cmd_info["help"],
10224+
description=cmd_info.get("description", ""),
10225+
formatter_class=__import__("argparse").RawDescriptionHelpFormatter,
10226+
)
10227+
cmd_info["setup_fn"](plugin_parser)
10228+
if cmd_info.get("handler_fn") is not None:
10229+
plugin_parser.set_defaults(func=cmd_info["handler_fn"])
10230+
except Exception as _exc:
10231+
logging.getLogger(__name__).debug("Plugin CLI discovery failed: %s", _exc)
1011310232

1011410233
# =========================================================================
1011510234
# curator command — background skill maintenance
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
"""Guards for CLI startup performance regression.
2+
3+
``hermes_cli.main`` skips eager plugin discovery at argparse-setup time
4+
when the invocation is clearly targeting a known built-in subcommand.
5+
This saves 500-650ms on ``hermes --help``, ``hermes version``,
6+
``hermes logs``, etc., by not importing ``google.cloud.pubsub_v1``,
7+
``aiohttp``, ``grpc``, and friends.
8+
9+
Two invariants:
10+
11+
1. ``_BUILTIN_SUBCOMMANDS`` must contain every subcommand that is actually
12+
registered by ``main()``. If an entry is missing, plugin discovery
13+
runs unnecessarily for that command (correctness-safe, just slow).
14+
If an entry is PRESENT but the subcommand doesn't exist, a plugin
15+
could shadow the name — also bad.
16+
17+
2. ``_plugin_cli_discovery_needed()`` returns the right answer for the
18+
flag/positional parsing cases it's meant to handle.
19+
"""
20+
21+
from __future__ import annotations
22+
23+
import io
24+
import re
25+
import sys
26+
from contextlib import redirect_stdout
27+
from unittest.mock import patch
28+
29+
import pytest
30+
31+
from hermes_cli.main import (
32+
_BUILTIN_SUBCOMMANDS,
33+
_first_positional_argv,
34+
_plugin_cli_discovery_needed,
35+
)
36+
37+
38+
# ── helper: grab the live set of top-level subcommands from argparse ───────
39+
40+
41+
def _live_subcommand_names() -> set[str]:
42+
"""Run ``hermes --help`` in-process and parse the subcommand block.
43+
44+
We patch ``_plugin_cli_discovery_needed`` to always return False so
45+
plugin-registered commands aren't included — we're validating the
46+
built-in-only set.
47+
"""
48+
from hermes_cli import main as _main
49+
50+
argv_backup = sys.argv[:]
51+
sys.argv = ["hermes", "--help"]
52+
buf = io.StringIO()
53+
try:
54+
with patch.object(_main, "_plugin_cli_discovery_needed", return_value=False):
55+
with redirect_stdout(buf):
56+
with pytest.raises(SystemExit):
57+
_main.main()
58+
finally:
59+
sys.argv = argv_backup
60+
61+
text = buf.getvalue()
62+
# argparse prints "{chat,model,...}" somewhere in the help output
63+
m = re.search(r"\{([a-zA-Z0-9_,\-]+)\}", text)
64+
assert m, f"Could not find subcommand group in --help output:\n{text[:500]}"
65+
return set(m.group(1).split(","))
66+
67+
68+
# ── _first_positional_argv ─────────────────────────────────────────────────
69+
70+
71+
@pytest.mark.parametrize(
72+
"argv,expected",
73+
[
74+
(["hermes"], None),
75+
(["hermes", "--help"], None),
76+
(["hermes", "-h"], None),
77+
(["hermes", "--version"], None),
78+
(["hermes", "-w"], None),
79+
# -p / --profile is stripped from sys.argv by
80+
# _apply_profile_override() at import time, so it never reaches
81+
# _first_positional_argv. We test with just -w / --tui here.
82+
(["hermes", "-w", "--tui"], None),
83+
(["hermes", "version"], "version"),
84+
(["hermes", "--tui", "chat"], "chat"),
85+
(["hermes", "-w", "logs"], "logs"),
86+
(["hermes", "chat", "hello world"], "chat"),
87+
(["hermes", "gateway", "run"], "gateway"),
88+
# Top-level value-taking flags: the value should be skipped.
89+
(["hermes", "-m", "gpt5", "chat"], "chat"),
90+
(["hermes", "--model", "gpt5", "chat", "hi"], "chat"),
91+
(["hermes", "-m", "gpt5", "--provider", "openai", "chat"], "chat"),
92+
(["hermes", "-z", "hello world"], None),
93+
(["hermes", "-z", "hello", "chat"], "chat"),
94+
(["hermes", "--model=gpt5", "chat"], "chat"), # inline form
95+
(["hermes", "--", "chat"], "chat"), # -- terminator
96+
(["hermes", "-w", "--"], None),
97+
# Unknown positional after skipped flags → plugin-cmd candidate.
98+
(["hermes", "some-plugin-cmd"], "some-plugin-cmd"),
99+
(["hermes", "-m", "gpt5", "some-plugin-cmd"], "some-plugin-cmd"),
100+
],
101+
)
102+
def test_first_positional_argv(argv, expected):
103+
with patch.object(sys, "argv", argv):
104+
assert _first_positional_argv() == expected
105+
106+
107+
# ── _plugin_cli_discovery_needed ───────────────────────────────────────────
108+
109+
110+
@pytest.mark.parametrize(
111+
"argv",
112+
[
113+
["hermes"], # bare → chat
114+
["hermes", "--help"], # top-level help
115+
["hermes", "-h"],
116+
["hermes", "version"], # known built-in
117+
["hermes", "logs"],
118+
["hermes", "gateway", "run"],
119+
["hermes", "--tui"],
120+
["hermes", "-w", "--tui"],
121+
["hermes", "chat", "hi"],
122+
["hermes", "help"], # accepted built-in-ish
123+
["hermes", "-m", "gpt5", "chat"], # flag-value-skipping
124+
],
125+
)
126+
def test_discovery_skipped_for_builtins(argv):
127+
with patch.object(sys, "argv", argv):
128+
assert _plugin_cli_discovery_needed() is False
129+
130+
131+
@pytest.mark.parametrize(
132+
"argv",
133+
[
134+
["hermes", "meet", "join"], # potential google_meet plugin
135+
["hermes", "honcho", "status"], # potential memory plugin
136+
["hermes", "unknown-subcmd"],
137+
],
138+
)
139+
def test_discovery_runs_for_unknown_positional(argv):
140+
with patch.object(sys, "argv", argv):
141+
assert _plugin_cli_discovery_needed() is True
142+
143+
144+
# ── _BUILTIN_SUBCOMMANDS ↔ argparse registration parity ────────────────────
145+
146+
147+
def test_builtin_set_covers_every_registered_subcommand():
148+
"""Every subcommand registered in main() must appear in the set.
149+
150+
Missing entries cause a slow-path regression (correctness stays
151+
fine — discovery just runs unnecessarily).
152+
"""
153+
live = _live_subcommand_names()
154+
# "help" is synthetic — an argparse-implicit convenience we include
155+
# in the set so ``hermes help <cmd>`` skips discovery; it won't show
156+
# up as a subparser in the --help output.
157+
declared = _BUILTIN_SUBCOMMANDS - {"help"}
158+
missing_from_declaration = live - declared
159+
assert not missing_from_declaration, (
160+
f"_BUILTIN_SUBCOMMANDS is missing these live subcommands: "
161+
f"{sorted(missing_from_declaration)}. Add them to "
162+
f"hermes_cli/main.py::_BUILTIN_SUBCOMMANDS so plugin discovery "
163+
f"can be skipped when the user targets them."
164+
)
165+
166+
167+
def test_builtin_set_has_no_phantom_entries():
168+
"""No entry in the set should refer to a subcommand that no longer exists.
169+
170+
A phantom entry means plugin discovery gets incorrectly skipped for
171+
a name that — if a plugin actually registered it — would fail to
172+
parse. Keeps the set honest.
173+
"""
174+
live = _live_subcommand_names()
175+
allowed_synthetic = {"help"}
176+
phantom = _BUILTIN_SUBCOMMANDS - live - allowed_synthetic
177+
assert not phantom, (
178+
f"_BUILTIN_SUBCOMMANDS has entries that are not registered as "
179+
f"top-level subparsers: {sorted(phantom)}"
180+
)

0 commit comments

Comments
 (0)