From 4ad78a98fbe103eedd5aa605d8f6f631dc3a9be3 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Wed, 29 Jul 2026 11:49:45 -0700 Subject: [PATCH] fix(observability): derive tool metrics from runtime metadata Signed-off-by: Alex Fournier --- docs/observability/relay-shared-metrics.md | 5 +- .../observability/relay_shared_metrics.py | 23 +++++- .../observability/shared_metrics_contract.py | 49 +++++------ tests/hermes_cli/test_relay_shared_metrics.py | 34 ++++---- .../test_relay_shared_metrics_runtime.py | 81 +++++++++++++++++++ 5 files changed, 145 insertions(+), 47 deletions(-) diff --git a/docs/observability/relay-shared-metrics.md b/docs/observability/relay-shared-metrics.md index d48261d6c2..f37ae66c72 100644 --- a/docs/observability/relay-shared-metrics.md +++ b/docs/observability/relay-shared-metrics.md @@ -91,7 +91,10 @@ conversation session during context compression. Each tool invocation is represented by a Relay tool lifecycle named `hermes.tool_call`. The terminal counter contains only bounded tool category, outcome, approval outcome, latency, and explicit retry-count buckets. Hermes -does not infer retries from repeated tool names or adjacent calls; when the +derives the category from the toolset already declared in its runtime registry; +custom and unrecognized toolsets collapse to `other` rather than exporting +tool or plugin names. Hermes does not infer retries from repeated tool names or +adjacent calls; when the hook does not provide an explicit retry relationship, the retry bucket is `unknown`. Approval decisions are emitted as `hermes.tool_approval` marks and recorded as attributed to a tool call or explicitly `unattributed`. Tool names, diff --git a/hermes_cli/observability/relay_shared_metrics.py b/hermes_cli/observability/relay_shared_metrics.py index ec8dfc13d9..b2cde6e028 100644 --- a/hermes_cli/observability/relay_shared_metrics.py +++ b/hermes_cli/observability/relay_shared_metrics.py @@ -371,7 +371,7 @@ class _Runtime: return outcome = tool_approval_outcome(event) tool_call_id = str(event.get("tool_call_id") or "") - attribution = "tool_call" if tool_call_id else "unattributed" + attribution = "unattributed" with session.lock: if session.closing: return @@ -389,6 +389,7 @@ class _Runtime: tool_call = matches[0] if len(matches) == 1 else None if tool_call is not None: tool_call.approval_outcome = outcome + attribution = "tool_call" self._run_in_task( task, self.relay.scope.event, @@ -967,9 +968,9 @@ def observe_lifecycle(hook_name: str, **kwargs: Any) -> None: elif hook_name == "pre_api_request": runtime.start_model_call(kwargs) elif hook_name == "pre_tool_call": - runtime.start_tool_call(kwargs) + runtime.start_tool_call(_with_runtime_toolset(kwargs)) elif hook_name == "post_tool_call": - runtime.record_tool_call(kwargs) + runtime.record_tool_call(_with_runtime_toolset(kwargs)) elif hook_name == "post_approval_response": runtime.record_approval(kwargs) elif hook_name == "post_api_request": @@ -990,6 +991,22 @@ def observe_lifecycle(hook_name: str, **kwargs: Any) -> None: ) +def _with_runtime_toolset(event: dict[str, Any]) -> dict[str, Any]: + """Attach the toolset already declared by Hermes's runtime registry.""" + if event.get("toolset"): + return event + tool_name = str(event.get("tool_name") or "") + if not tool_name: + return event + try: + from tools.registry import registry + + toolset = registry.get_toolset_for_tool(tool_name) + except Exception: + toolset = None + return {**event, "toolset": toolset or "other"} + + def prepare_session_start() -> None: """Register the subscriber before any producer opens the session scope.""" if enabled(): diff --git a/hermes_cli/observability/shared_metrics_contract.py b/hermes_cli/observability/shared_metrics_contract.py index a2eff337d3..490ae136fa 100644 --- a/hermes_cli/observability/shared_metrics_contract.py +++ b/hermes_cli/observability/shared_metrics_contract.py @@ -147,20 +147,6 @@ TOOL_LATENCY_BUCKETS: frozenset[str] = frozenset({ }) TOOL_RETRY_BUCKETS: frozenset[str] = COUNT_BUCKETS | frozenset({"unknown"}) -_TOOL_NAMES_BY_CATEGORY: dict[str, frozenset[str]] = { - "code_execution": frozenset({"execute_code"}), - "communication": frozenset({"discord", "email", "meet"}), - "computer_use": frozenset({"computer_use"}), - "delegation": frozenset({"delegate_task"}), - "file": frozenset({"patch", "read_file", "search_files", "write_file"}), - "memory": frozenset({"memory", "session_search"}), - "planning": frozenset({"clarify", "todo"}), - "scheduler": frozenset({"cronjob"}), - "skill": frozenset({"skill_manage", "skill_view", "skills_list"}), - "terminal": frozenset({"close_terminal", "process", "read_terminal", "terminal"}), - "web": frozenset({"web_extract", "web_search", "x_search"}), -} - _COUNTER_DIMENSION_VALUES: dict[str, dict[str, frozenset[str]]] = { TASK_STARTED_METRIC: { "entrypoint": TASK_ENTRYPOINTS, @@ -491,26 +477,33 @@ def count_bucket(count: int) -> str: def tool_category(kwargs: dict[str, Any]) -> str: - """Map a raw Hermes tool name to a stable low-cardinality category.""" - name = str(kwargs.get("tool_name") or "").strip().lower() - if not name: + """Map Hermes registry toolset metadata to a low-cardinality category.""" + toolset = str(kwargs.get("toolset") or "").strip().lower() + if not toolset: return "unknown" - if name.startswith(("mcp.", "mcp_", "mcp__")): + if toolset in TOOL_CATEGORIES: + return toolset + if toolset.startswith("mcp"): return "mcp" - for category, names in _TOOL_NAMES_BY_CATEGORY.items(): - if name in names: - return category - if name.startswith("browser_"): + if toolset.startswith("browser"): return "browser" - if name.startswith(("vision_", "image_", "video_", "text_to_speech")): + if toolset.startswith(("image", "tts", "video", "vision")): return "media" - if name.startswith("ha_"): + if toolset.startswith("homeassistant"): return "home_automation" - if name.startswith("kanban_"): + if toolset in {"clarify", "kanban", "todo"}: return "planning" - if name.startswith("project_"): - return "project" - if name.startswith(("discord_", "email_", "feishu_", "slack_", "sms_", "yb_")): + if toolset == "session_search": + return "memory" + if toolset == "cronjob": + return "scheduler" + if toolset == "skills": + return "skill" + if toolset == "x_search": + return "web" + if toolset.startswith( + ("discord", "email", "feishu", "hermes-yuanbao", "slack", "sms") + ): return "communication" return "other" diff --git a/tests/hermes_cli/test_relay_shared_metrics.py b/tests/hermes_cli/test_relay_shared_metrics.py index e4e70450c6..609e7a2bbf 100644 --- a/tests/hermes_cli/test_relay_shared_metrics.py +++ b/tests/hermes_cli/test_relay_shared_metrics.py @@ -237,27 +237,31 @@ def test_package_schema_matches_the_tool_contract(): @pytest.mark.parametrize( - ("name", "expected"), + ("toolset", "expected"), [ ("", "unknown"), - ("read_file", "file"), + ("file", "file"), ("terminal", "terminal"), - ("execute_code", "code_execution"), - ("delegate_task", "delegation"), - ("skill_manage", "skill"), - ("browser_navigate", "browser"), - ("image_generate", "media"), - ("ha_call_service", "home_automation"), - ("kanban_create", "planning"), - ("project_switch", "project"), + ("code_execution", "code_execution"), + ("delegation", "delegation"), + ("skills", "skill"), + ("browser-cdp", "browser"), + ("image_gen", "media"), + ("homeassistant", "home_automation"), + ("kanban", "planning"), + ("project", "project"), ("discord", "communication"), - ("feishu_doc_read", "communication"), - ("mcp__github__get_issue", "mcp"), - ("private_plugin_tool", "other"), + ("feishu_doc", "communication"), + ("mcp-github", "mcp"), + ("private_plugin", "other"), ], ) -def test_tool_category_is_bounded(name, expected): - assert tool_category({"tool_name": name}) == expected +def test_tool_category_uses_bounded_runtime_toolsets(toolset, expected): + assert tool_category({"toolset": toolset}) == expected + + +def test_tool_category_does_not_classify_raw_tool_names(): + assert tool_category({"tool_name": "read_file"}) == "unknown" @pytest.mark.parametrize( diff --git a/tests/hermes_cli/test_relay_shared_metrics_runtime.py b/tests/hermes_cli/test_relay_shared_metrics_runtime.py index a8bdb8e17b..4fc96e522e 100644 --- a/tests/hermes_cli/test_relay_shared_metrics_runtime.py +++ b/tests/hermes_cli/test_relay_shared_metrics_runtime.py @@ -252,6 +252,7 @@ def test_direct_runtime_records_without_enabling_a_plugin(direct_runtime, tmp_pa **base, tool_call_id="sensitive-tool-call", tool_name="terminal", + toolset="terminal", args={"command": "sensitive-command"}, ) lifecycle.invoke_hook( @@ -267,6 +268,7 @@ def test_direct_runtime_records_without_enabling_a_plugin(direct_runtime, tmp_pa **base, tool_call_id="sensitive-tool-call", tool_name="terminal", + toolset="terminal", args={"command": "sensitive-command"}, result={"output": "sensitive-tool-result"}, status="ok", @@ -2877,6 +2879,85 @@ def test_approval_without_tool_context_is_counted_as_unattributed(direct_runtime } +def test_approval_with_unmatched_tool_id_is_counted_as_unattributed(direct_runtime): + base = { + "session_id": "s1", + "task_id": "t1", + "turn_id": "turn-1", + "platform": "cli", + } + + lifecycle.invoke_hook("pre_llm_call", **base) + lifecycle.invoke_hook( + "post_approval_response", + **base, + tool_call_id="spoofed-tool-call", + choice="deny", + command="must-not-pass", + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=False, + failed=True, + interrupted=False, + turn_exit_reason="approval_denied", + ) + lifecycle.finalize_session(session_id="s1") + + [approval] = [ + event + for event in direct_runtime.events + if event[0] == "scope.event" and event[1] == "hermes.tool_approval" + ] + assert approval[2]["data"] == { + "attribution": "unattributed", + "outcome": "denied", + } + + +def test_tool_category_comes_from_runtime_registry_metadata( + direct_runtime, + monkeypatch, +): + from tools.registry import registry + + monkeypatch.setattr( + registry, + "get_toolset_for_tool", + lambda name: "terminal" if name == "runtime_only_tool" else None, + ) + base = { + "session_id": "s1", + "task_id": "t1", + "turn_id": "turn-1", + "platform": "cli", + } + lifecycle.invoke_hook("pre_llm_call", **base) + lifecycle.invoke_hook( + "post_tool_call", + **base, + tool_call_id="tool-1", + tool_name="runtime_only_tool", + result={"output": "private"}, + status="ok", + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=True, + failed=False, + interrupted=False, + turn_exit_reason="text_response(stop)", + ) + lifecycle.finalize_session(session_id="s1") + + [tool_end] = [ + event for event in direct_runtime.events if event[0] == "tool.call_end" + ] + assert tool_end[2]["tool_category"] == "terminal" + + def test_task_terminal_counts_explicit_retry_with_new_request_id(direct_runtime): base = { "session_id": "s1",