From c1e1de0f6de354706431d5c754f8950d4268da4c Mon Sep 17 00:00:00 2001 From: Zeno Wang Date: Thu, 30 Apr 2026 02:23:08 +0800 Subject: [PATCH 1/3] feat(skills): add outcome_tracker to wire up dead bump_use counter --- agent/outcome_tracker.py | 51 +++++++++++ tests/agent/test_outcome_tracker.py | 132 ++++++++++++++++++++++++++++ 2 files changed, 183 insertions(+) create mode 100644 agent/outcome_tracker.py create mode 100644 tests/agent/test_outcome_tracker.py diff --git a/agent/outcome_tracker.py b/agent/outcome_tracker.py new file mode 100644 index 000000000000..e3265d372218 --- /dev/null +++ b/agent/outcome_tracker.py @@ -0,0 +1,51 @@ +"""Wire up the dead bump_use counter by scanning session messages for skill_view calls. + +Called once at session end from AIAgent.shutdown_memory_provider. Best-effort: +failures are logged at DEBUG and never propagate to the caller. +""" +from __future__ import annotations + +import json +import logging +from typing import Any + +logger = logging.getLogger(__name__) + + +def record_session_outcome(messages: list[dict[str, Any]]) -> None: + """Increment use_count for every skill viewed during the session.""" + skill_names = _extract_skill_views(messages) + if not skill_names: + return + try: + from tools.skill_usage import bump_use + for name in skill_names: + bump_use(name) + except Exception as exc: + logger.debug("outcome_tracker: bump_use failed: %s", exc) + + +def _extract_skill_views(messages: list[dict[str, Any]]) -> set[str]: + """Return the set of skill names loaded via skill_view in this session. + + Parses OpenAI-format assistant messages (role='assistant', tool_calls=[...]). + Malformed entries are silently skipped. + """ + seen: set[str] = set() + for msg in messages: + if not isinstance(msg, dict) or msg.get("role") != "assistant": + continue + for tc in msg.get("tool_calls") or []: + if not isinstance(tc, dict): + continue + func = tc.get("function") or {} + if func.get("name") != "skill_view": + continue + try: + args = json.loads(func.get("arguments", "{}")) + name = args.get("name") or args.get("skill_name") + if name: + seen.add(str(name)) + except Exception: + pass + return seen diff --git a/tests/agent/test_outcome_tracker.py b/tests/agent/test_outcome_tracker.py new file mode 100644 index 000000000000..1a2d15ccdfa1 --- /dev/null +++ b/tests/agent/test_outcome_tracker.py @@ -0,0 +1,132 @@ +"""Tests for agent/outcome_tracker.py""" +from __future__ import annotations + +import json +from unittest.mock import patch + + +def _skill_view_msg(skill_name: str) -> dict: + return { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "call_test", + "type": "function", + "function": { + "name": "skill_view", + "arguments": json.dumps({"name": skill_name}), + }, + } + ], + } + + +def _other_tool_msg(tool_name: str) -> dict: + return { + "role": "assistant", + "content": None, + "tool_calls": [ + {"function": {"name": tool_name, "arguments": json.dumps({"path": "/tmp/x"})}} + ], + } + + +class TestExtractSkillViews: + def test_extracts_single_skill_name(self): + from agent.outcome_tracker import _extract_skill_views + assert _extract_skill_views([_skill_view_msg("coding-skill")]) == {"coding-skill"} + + def test_extracts_multiple_unique_skills(self): + from agent.outcome_tracker import _extract_skill_views + msgs = [_skill_view_msg("skill-a"), _skill_view_msg("skill-b")] + assert _extract_skill_views(msgs) == {"skill-a", "skill-b"} + + def test_deduplicates_repeated_views(self): + from agent.outcome_tracker import _extract_skill_views + msgs = [_skill_view_msg("skill-a"), _skill_view_msg("skill-a")] + assert _extract_skill_views(msgs) == {"skill-a"} + + def test_ignores_other_tool_calls(self): + from agent.outcome_tracker import _extract_skill_views + msgs = [_other_tool_msg("read_file"), _other_tool_msg("bash")] + assert _extract_skill_views(msgs) == set() + + def test_ignores_non_assistant_roles(self): + from agent.outcome_tracker import _extract_skill_views + msgs = [ + {"role": "user", "content": "use the coding skill"}, + {"role": "tool", "content": "skill content", "tool_call_id": "call_1"}, + ] + assert _extract_skill_views(msgs) == set() + + def test_handles_empty_messages(self): + from agent.outcome_tracker import _extract_skill_views + assert _extract_skill_views([]) == set() + + def test_handles_malformed_arguments_gracefully(self): + from agent.outcome_tracker import _extract_skill_views + msgs = [ + { + "role": "assistant", + "tool_calls": [ + {"function": {"name": "skill_view", "arguments": "not-valid-json"}} + ], + } + ] + assert _extract_skill_views(msgs) == set() + + def test_handles_missing_tool_calls_key(self): + from agent.outcome_tracker import _extract_skill_views + assert _extract_skill_views([{"role": "assistant", "content": "hello"}]) == set() + + +class TestRecordSessionOutcome: + def test_calls_bump_use_for_each_skill(self): + from agent.outcome_tracker import record_session_outcome + bumped: list[str] = [] + with patch("tools.skill_usage.bump_use", side_effect=lambda n: bumped.append(n)): + record_session_outcome([_skill_view_msg("skill-a"), _skill_view_msg("skill-b")]) + assert set(bumped) == {"skill-a", "skill-b"} + + def test_no_op_on_empty_messages(self): + from agent.outcome_tracker import record_session_outcome + with patch("tools.skill_usage.bump_use") as mock_bump: + record_session_outcome([]) + mock_bump.assert_not_called() + + def test_no_op_when_no_skill_views(self): + from agent.outcome_tracker import record_session_outcome + with patch("tools.skill_usage.bump_use") as mock_bump: + record_session_outcome([_other_tool_msg("read_file")]) + mock_bump.assert_not_called() + + def test_bump_use_failure_does_not_raise(self): + from agent.outcome_tracker import record_session_outcome + with patch("tools.skill_usage.bump_use", side_effect=RuntimeError("disk full")): + record_session_outcome([_skill_view_msg("my-skill")]) # must not raise + + +class TestShutdownHook: + def test_shutdown_memory_provider_calls_record_session_outcome(self): + """shutdown_memory_provider calls record_session_outcome after our edit. + + NOTE: This test is EXPECTED TO FAIL in Task 1. The hook in run_agent.py + will be added in Task 2. Write the test now so Task 2 can verify it passes. + """ + import run_agent as ra + + recorded: list[list] = [] + + class FakeAgent: + _memory_manager = None + session_id = "test" + shutdown_memory_provider = ra.AIAgent.shutdown_memory_provider + + messages = [_skill_view_msg("test-skill")] + with patch("agent.outcome_tracker.record_session_outcome", + side_effect=lambda m: recorded.append(m)): + FakeAgent().shutdown_memory_provider(messages) + + assert len(recorded) == 1 + assert recorded[0] == messages From b0924d64bddf695ba2b0a8731764964ede13f37d Mon Sep 17 00:00:00 2001 From: Zeno Wang Date: Thu, 30 Apr 2026 02:27:45 +0800 Subject: [PATCH 2/3] feat(skills): hook outcome_tracker into shutdown_memory_provider --- run_agent.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/run_agent.py b/run_agent.py index dc6c0e13d22e..e20859e16013 100644 --- a/run_agent.py +++ b/run_agent.py @@ -4519,7 +4519,12 @@ def shutdown_memory_provider(self, messages: list = None) -> None: ) except Exception: pass - + try: + from agent.outcome_tracker import record_session_outcome + record_session_outcome(messages or []) + except Exception: + pass + def commit_memory_session(self, messages: list = None) -> None: """Trigger end-of-session extraction without tearing providers down. Called when session_id rotates (e.g. /new, context compression); From a2a90260fc76067d971c86bb8a1a25df20a99cbe Mon Sep 17 00:00:00 2001 From: Zeno Wang Date: Thu, 30 Apr 2026 02:40:23 +0800 Subject: [PATCH 3/3] feat(skills): configurable extra_review_prompt quality gate via config --- run_agent.py | 4 + tests/agent/test_skill_review_extra.py | 116 +++++++++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 tests/agent/test_skill_review_extra.py diff --git a/run_agent.py b/run_agent.py index e20859e16013..9da8aa8edd8f 100644 --- a/run_agent.py +++ b/run_agent.py @@ -1755,9 +1755,11 @@ def __init__( # Skills config: nudge interval for skill creation reminders self._skill_nudge_interval = 10 + self._skill_review_extra = "" try: skills_config = _agent_cfg.get("skills", {}) self._skill_nudge_interval = int(skills_config.get("creation_nudge_interval", 10)) + self._skill_review_extra = str(skills_config.get("extra_review_prompt", "")) except Exception: pass @@ -3470,6 +3472,8 @@ def _spawn_background_review( prompt = self._MEMORY_REVIEW_PROMPT else: prompt = self._SKILL_REVIEW_PROMPT + if review_skills and self._skill_review_extra: + prompt = prompt + "\n\n" + self._skill_review_extra def _run_review(): import contextlib diff --git a/tests/agent/test_skill_review_extra.py b/tests/agent/test_skill_review_extra.py new file mode 100644 index 000000000000..b8039ff4fbb7 --- /dev/null +++ b/tests/agent/test_skill_review_extra.py @@ -0,0 +1,116 @@ +"""Tests for the configurable skill review quality gate in AIAgent.""" +from __future__ import annotations + +from unittest.mock import MagicMock, patch + + +def _fake_routed_client(): + """Return a minimal fake provider client to satisfy AIAgent.__init__.""" + client = MagicMock() + client.api_key = "sk-fake-test-key" + client.base_url = "https://api.openai.com/v1/" + client._default_headers = {} + return client + + +def _make_agent(extra_prompt: str = ""): + """Construct a minimal AIAgent with mocked provider and config.""" + import run_agent as ra + + cfg = {"skills": {"extra_review_prompt": extra_prompt}} if extra_prompt else {} + + with ( + patch("hermes_cli.config.load_config", return_value=cfg), + patch( + "agent.auxiliary_client.resolve_provider_client", + return_value=(_fake_routed_client(), "openai"), + ), + ): + return ra.AIAgent( + model="claude-haiku-4-5-20251001", + quiet_mode=True, + skip_context_files=True, + max_iterations=1, + ) + + +class TestSkillReviewExtra: + def test_defaults_to_empty_string_when_not_configured(self): + agent = _make_agent() + assert agent._skill_review_extra == "" + + def test_loaded_from_config(self): + agent = _make_agent("verify: is this generalizable?") + assert agent._skill_review_extra == "verify: is this generalizable?" + + def test_extra_appended_to_skill_only_review(self): + """When review_skills=True and review_memory=False, extra is appended.""" + import run_agent as ra + + agent = _make_agent("my-quality-gate") + captured: list[str] = [] + + fake_review = MagicMock() + fake_review.run_conversation = lambda user_message, **kw: captured.append(user_message) + + with patch( + "threading.Thread", + side_effect=lambda *a, target, **kw: type("T", (), {"start": lambda s: target()})(), + ): + with patch.object(ra, "AIAgent", return_value=fake_review): + agent._spawn_background_review( + messages_snapshot=[], + review_memory=False, + review_skills=True, + ) + + assert len(captured) == 1 + assert "my-quality-gate" in captured[0] + + def test_extra_NOT_appended_to_memory_only_review(self): + """When review_skills=False, extra prompt must not appear.""" + import run_agent as ra + + agent = _make_agent("my-quality-gate") + captured: list[str] = [] + + fake_review = MagicMock() + fake_review.run_conversation = lambda user_message, **kw: captured.append(user_message) + + with patch( + "threading.Thread", + side_effect=lambda *a, target, **kw: type("T", (), {"start": lambda s: target()})(), + ): + with patch.object(ra, "AIAgent", return_value=fake_review): + agent._spawn_background_review( + messages_snapshot=[], + review_memory=True, + review_skills=False, + ) + + assert len(captured) == 1 + assert "my-quality-gate" not in captured[0] + + def test_no_extra_when_config_is_empty(self): + """When extra_review_prompt is empty, prompt is unchanged.""" + import run_agent as ra + + agent = _make_agent() + captured: list[str] = [] + + fake_review = MagicMock() + fake_review.run_conversation = lambda user_message, **kw: captured.append(user_message) + + with patch( + "threading.Thread", + side_effect=lambda *a, target, **kw: type("T", (), {"start": lambda s: target()})(), + ): + with patch.object(ra, "AIAgent", return_value=fake_review): + agent._spawn_background_review( + messages_snapshot=[], + review_memory=False, + review_skills=True, + ) + + assert len(captured) == 1 + assert captured[0] == agent._SKILL_REVIEW_PROMPT