perf(curator): trim dead tool-schema from the LLM review fork
The curator LLM review loop (_run_llm_review) built its AIAgent without enabled_toolsets, so it advertised the full default catalog (~30 tools plus the context_engine lcm_* family) on every call. The fork uses only four tools, fixed by its own system prompt, with no dispatch path to the rest, so ~26 tool schemas shipped on every request as dead weight: ~7K input tokens per call on a loop that makes 50-100 calls per consolidation pass. Restrict the fork to enabled_toolsets=["skills", "terminal"], the same tools the prompt already names. Behavior-neutral: the prompt held the model to these tools and nothing routed calls to the others. Mirrors the background_review fork (background_review.py:788-794). Call-site only; AIAgent already forwards the kwarg. Adds test_review_fork_restricts_toolsets_to_skills_and_terminal (captures the constructor kwarg) and test_review_fork_toolset_surface_is_skills_plus_terminal (pins the resolved surface).
This commit is contained in:
@@ -1923,6 +1923,7 @@ def _run_llm_review(prompt: str) -> Dict[str, Any]:
|
||||
credential_pool=_credential_pool,
|
||||
request_overrides=_request_overrides,
|
||||
**_agent_kwargs,
|
||||
enabled_toolsets=["skills", "terminal"],
|
||||
# Umbrella-building over a large skill collection is worth a
|
||||
# high iteration ceiling — the pass typically takes 50-100
|
||||
# API calls against hundreds of candidate skills. The
|
||||
|
||||
@@ -699,3 +699,80 @@ def test_review_fork_uses_runtime_model_and_output_cap(curator_env, monkeypatch)
|
||||
assert captured["max_tokens"] == 1234
|
||||
|
||||
|
||||
|
||||
|
||||
def test_review_fork_restricts_toolsets_to_skills_and_terminal(curator_env, monkeypatch):
|
||||
"""The curator LLM fork must advertise only the skills + terminal toolsets.
|
||||
|
||||
Without ``enabled_toolsets=["skills", "terminal"]`` on the AIAgent(...) call
|
||||
in ``_run_llm_review``, ``enabled_toolsets`` defaults to None and init_agent
|
||||
grants the fork the full default catalog (~30 tools) plus the context_engine
|
||||
(lcm_*) tools, billing ~7K wasted schema tokens on every one of the fork's
|
||||
50-100 API calls per consolidation pass. The prompt (curator.py:509-523)
|
||||
confines the model to four tools in natural language, but only this kwarg
|
||||
filters the advertised request schema. Capturing the constructor kwarg is
|
||||
the sole assertion that distinguishes fixed from unfixed code.
|
||||
"""
|
||||
curator = curator_env["curator"]
|
||||
|
||||
# curator_env stubs _run_llm_review wholesale; exercise the real
|
||||
# implementation, so reload the module to restore it.
|
||||
import importlib
|
||||
importlib.reload(curator)
|
||||
|
||||
captured = {}
|
||||
|
||||
class _StubAgent:
|
||||
def __init__(self, *args, **kwargs):
|
||||
captured["enabled_toolsets"] = kwargs.get("enabled_toolsets", "UNSET")
|
||||
self._memory_write_origin = "assistant_tool"
|
||||
self._memory_nudge_interval = 0
|
||||
self._skill_nudge_interval = 0
|
||||
self._session_messages = []
|
||||
|
||||
def run_conversation(self, user_message=None, **kwargs):
|
||||
return {"final_response": "no change"}
|
||||
|
||||
def close(self):
|
||||
pass
|
||||
|
||||
monkeypatch.setattr("run_agent.AIAgent", _StubAgent)
|
||||
|
||||
meta = curator._run_llm_review("review prompt")
|
||||
|
||||
# error is None proves the fork was actually constructed (capture ran).
|
||||
assert meta.get("error") is None, meta.get("error")
|
||||
assert captured.get("enabled_toolsets") == ["skills", "terminal"], (
|
||||
"curator review fork did not pass enabled_toolsets=['skills', "
|
||||
"'terminal'] to AIAgent; the full default tool catalog (plus lcm_* "
|
||||
"context_engine tools) would be advertised; got "
|
||||
f"{captured.get('enabled_toolsets')!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_review_fork_toolset_surface_is_skills_plus_terminal():
|
||||
"""Documentary check on the static surface the fork's kwarg resolves to.
|
||||
|
||||
Registry-independent (include_registry=False) so a plugin-registered tool
|
||||
tagged into these toolsets cannot flake the membership checks. This
|
||||
documents the intended surface (the four prompt-named tools present, dead
|
||||
default and lcm_* schema absent) but does not itself guard the call-site
|
||||
kwarg. No exact-set pin: intentional additions to either toolset must not
|
||||
fail this test.
|
||||
"""
|
||||
from toolsets import resolve_toolset
|
||||
|
||||
surface = set(resolve_toolset("skills", include_registry=False)) | set(
|
||||
resolve_toolset("terminal", include_registry=False)
|
||||
)
|
||||
|
||||
# The four prompt-named tools are all present.
|
||||
assert "skills_list" in surface
|
||||
assert "skill_view" in surface
|
||||
assert "skill_manage" in surface
|
||||
assert "terminal" in surface
|
||||
|
||||
# Representative dropped default + context_engine tools are absent.
|
||||
assert "read_file" not in surface
|
||||
assert "web_search" not in surface
|
||||
assert "lcm_grep" not in surface
|
||||
|
||||
Reference in New Issue
Block a user