fix: log missing async-subagent tools at DEBUG, not WARNING (#378)
* fix: log missing async-subagent tools at DEBUG, not WARNING * fix: distinguish load_subagents callers via async_swap_pending flag --------- Co-authored-by: Xi Zhang <106144707+X-iZhang@users.noreply.github.com>
This commit is contained in:
@@ -499,9 +499,14 @@ def _build_base_kwargs(
|
||||
tool_registry["tavily_search"] = tavily_search
|
||||
base_tools = [think_tool, skill_manager]
|
||||
|
||||
# ``async_swap_pending=True`` because ``_maybe_swap_async_subagents``
|
||||
# below re-resolves tools for async subagents against the deployed
|
||||
# graph's own registry (via ``subagents/_factory.py``). A tool missing
|
||||
# from ``tool_registry`` for an async spec logs at DEBUG, not WARNING.
|
||||
subs = load_subagents(
|
||||
SUBAGENTS_CONFIG,
|
||||
tool_registry=tool_registry,
|
||||
async_swap_pending=True,
|
||||
)
|
||||
_ensure_general_purpose_subagent(subs)
|
||||
_inject_subagent_middleware(
|
||||
@@ -569,9 +574,12 @@ def load_mcp_and_build_kwargs(
|
||||
|
||||
mcp_main = mcp_by_agent.pop("main", [])
|
||||
|
||||
# Same rationale as ``_build_base_kwargs``: async subagents get
|
||||
# re-resolved downstream by the deployed graph's factory registry.
|
||||
subs = load_subagents(
|
||||
SUBAGENTS_CONFIG,
|
||||
tool_registry=registry,
|
||||
async_swap_pending=True,
|
||||
)
|
||||
|
||||
_ensure_general_purpose_subagent(subs)
|
||||
|
||||
+46
-13
@@ -114,6 +114,7 @@ def load_subagents(
|
||||
*,
|
||||
tool_registry: dict[str, Any],
|
||||
prompt_refs: dict[str, str] | None = None,
|
||||
async_swap_pending: bool = False,
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Load subagent definitions from a directory of YAML files and wire up tools.
|
||||
|
||||
@@ -139,6 +140,17 @@ def load_subagents(
|
||||
tools: [tavily_search, think_tool]
|
||||
system_prompt: |
|
||||
...
|
||||
|
||||
``async_swap_pending`` tells the loader that an async-subagent swap will
|
||||
run downstream against a different tool registry (typically the deployed
|
||||
graph's own registry in ``subagents/_factory.py``). When True, a tool
|
||||
missing from ``tool_registry`` for an ``async: true`` spec is logged at
|
||||
DEBUG (it will be re-resolved by the async graph). When False, the
|
||||
caller IS the terminal registry — any missing tool is a genuine typo
|
||||
and logs at WARNING. Sync in-process callers
|
||||
(``EvoScientist._build_base_kwargs``, ``load_mcp_and_build_kwargs``)
|
||||
should pass True; the factory
|
||||
(``subagents/_factory.build_async_subagent_graph``) leaves it False.
|
||||
"""
|
||||
prompt_refs = prompt_refs or {}
|
||||
|
||||
@@ -212,18 +224,9 @@ def load_subagents(
|
||||
if "skills" in spec:
|
||||
subagent["skills"] = spec["skills"]
|
||||
|
||||
if "tools" in spec:
|
||||
resolved = []
|
||||
for t in spec["tools"]:
|
||||
if t in tool_registry:
|
||||
resolved.append(tool_registry[t])
|
||||
else:
|
||||
logger.warning(
|
||||
"Subagent %r: tool %r not in registry, skipping", name, t
|
||||
)
|
||||
subagent["tools"] = resolved
|
||||
|
||||
# Internal field: carries the ``async:`` yaml flag through to
|
||||
# Compute the async flag up-front so the tools-resolution block below
|
||||
# can pick the right log level for missing tools. Internal field:
|
||||
# carries the ``async:`` yaml flag through to
|
||||
# ``_maybe_swap_async_subagents`` so the swap doesn't need a second
|
||||
# yaml pass to discover async-flagged agents. Underscore prefix marks
|
||||
# it as internal — must be popped before passing to deepagents.
|
||||
@@ -236,6 +239,31 @@ def load_subagents(
|
||||
f"Subagent {name!r}: 'async' must be a boolean, "
|
||||
f"got {type(async_val).__name__}: {async_val!r}"
|
||||
)
|
||||
|
||||
if "tools" in spec:
|
||||
resolved = []
|
||||
for t in spec["tools"]:
|
||||
if t in tool_registry:
|
||||
resolved.append(tool_registry[t])
|
||||
elif async_val and async_swap_pending:
|
||||
# Caller expects an async-subagent swap downstream against
|
||||
# a different registry. The in-process registry is
|
||||
# intentionally minimal for async agents — a miss here is
|
||||
# not degraded state.
|
||||
logger.debug(
|
||||
"Subagent %r: tool %r not in the in-process registry; "
|
||||
"resolved by the async graph",
|
||||
name,
|
||||
t,
|
||||
)
|
||||
else:
|
||||
# Terminal registry (factory) OR sync subagent — a missing
|
||||
# tool is a genuine typo and won't be resolved anywhere.
|
||||
logger.warning(
|
||||
"Subagent %r: tool %r not in registry, skipping", name, t
|
||||
)
|
||||
subagent["tools"] = resolved
|
||||
|
||||
subagent["_async"] = async_val
|
||||
|
||||
return subagent
|
||||
@@ -252,12 +280,17 @@ def load_subagent(
|
||||
*,
|
||||
tool_registry: dict[str, Any],
|
||||
prompt_refs: dict[str, str] | None = None,
|
||||
async_swap_pending: bool = False,
|
||||
) -> dict[str, Any]:
|
||||
"""Load a single sub-agent by name from YAML."""
|
||||
"""Load a single sub-agent by name from YAML.
|
||||
|
||||
See :func:`load_subagents` for ``async_swap_pending`` semantics.
|
||||
"""
|
||||
for agent in load_subagents(
|
||||
config_path,
|
||||
tool_registry=tool_registry,
|
||||
prompt_refs=prompt_refs,
|
||||
async_swap_pending=async_swap_pending,
|
||||
):
|
||||
if agent.get("name") == name:
|
||||
return agent
|
||||
|
||||
@@ -139,3 +139,100 @@ def test_non_dict_spec_error_includes_filename_and_name(tmp_path):
|
||||
)
|
||||
with pytest.raises(ValueError, match=r"weird\.yaml.*weird-agent"):
|
||||
load_subagents(config_path, tool_registry={})
|
||||
|
||||
|
||||
def test_missing_tool_on_sync_subagent_logs_warning(tmp_path, caplog):
|
||||
"""Sync sub-agents with a tool missing from the registry log at WARNING.
|
||||
|
||||
Sync sub-agents run in-process under the main agent and rely on the
|
||||
in-process registry to wire every tool they declare. A missing tool
|
||||
IS a genuine degradation — surfaces it as a warning.
|
||||
"""
|
||||
config_path = _write_yaml(
|
||||
tmp_path,
|
||||
"planner.yaml",
|
||||
"""
|
||||
planner-agent:
|
||||
description: Plans experiments
|
||||
system_prompt: ""
|
||||
tools: [nonexistent_tool]
|
||||
""",
|
||||
)
|
||||
with caplog.at_level("DEBUG", logger="EvoScientist.utils"):
|
||||
subs = load_subagents(config_path, tool_registry={})
|
||||
assert subs[0]["_async"] is False
|
||||
assert subs[0]["tools"] == []
|
||||
warnings = [r for r in caplog.records if r.levelname == "WARNING"]
|
||||
assert any("nonexistent_tool" in r.getMessage() for r in warnings)
|
||||
|
||||
|
||||
def test_missing_tool_on_async_subagent_logs_debug_when_swap_pending(tmp_path, caplog):
|
||||
"""Sync in-process callers pass ``async_swap_pending=True`` — a tool
|
||||
missing for an ``async: true`` spec logs at DEBUG because the async
|
||||
graph's own registry will re-resolve it downstream
|
||||
(``subagents/_factory.py``).
|
||||
|
||||
Regression guard for the spurious startup WARNING that pre-fix logs
|
||||
fired on every ``EvoSci`` startup even though the tool was wired at
|
||||
runtime by the deployed graph.
|
||||
"""
|
||||
config_path = _write_yaml(
|
||||
tmp_path,
|
||||
"scheduler.yaml",
|
||||
"""
|
||||
scheduler:
|
||||
description: Fires on cron
|
||||
system_prompt: ""
|
||||
tools: [nonexistent_tool]
|
||||
async: true
|
||||
""",
|
||||
)
|
||||
with caplog.at_level("DEBUG", logger="EvoScientist.utils"):
|
||||
subs = load_subagents(config_path, tool_registry={}, async_swap_pending=True)
|
||||
assert subs[0]["_async"] is True
|
||||
assert subs[0]["tools"] == []
|
||||
warnings = [r for r in caplog.records if r.levelname == "WARNING"]
|
||||
assert not any("nonexistent_tool" in r.getMessage() for r in warnings)
|
||||
debugs = [r for r in caplog.records if r.levelname == "DEBUG"]
|
||||
assert any(
|
||||
"nonexistent_tool" in r.getMessage() and "async graph" in r.getMessage()
|
||||
for r in debugs
|
||||
)
|
||||
|
||||
|
||||
def test_missing_tool_on_async_subagent_logs_warning_at_terminal_registry(
|
||||
tmp_path, caplog
|
||||
):
|
||||
"""When ``async_swap_pending`` is False (the default), the caller IS the
|
||||
terminal registry — the factory boundary
|
||||
(``subagents/_factory.build_async_subagent_graph``). A tool missing for
|
||||
an ``async: true`` spec is a genuine typo that won't be resolved anywhere
|
||||
downstream, so log at WARNING.
|
||||
|
||||
Without this guard, an earlier version of the fix (unconditional DEBUG for
|
||||
every async spec) silently hid factory-boundary typos. Reviewer flagged
|
||||
this as the last remaining gap.
|
||||
"""
|
||||
config_path = _write_yaml(
|
||||
tmp_path,
|
||||
"scheduler.yaml",
|
||||
"""
|
||||
scheduler:
|
||||
description: Fires on cron
|
||||
system_prompt: ""
|
||||
tools: [nonexistent_tool]
|
||||
async: true
|
||||
""",
|
||||
)
|
||||
with caplog.at_level("DEBUG", logger="EvoScientist.utils"):
|
||||
# Default: async_swap_pending=False → factory-boundary semantics.
|
||||
subs = load_subagents(config_path, tool_registry={})
|
||||
assert subs[0]["_async"] is True
|
||||
assert subs[0]["tools"] == []
|
||||
warnings = [r for r in caplog.records if r.levelname == "WARNING"]
|
||||
assert any("nonexistent_tool" in r.getMessage() for r in warnings)
|
||||
debugs = [r for r in caplog.records if r.levelname == "DEBUG"]
|
||||
assert not any(
|
||||
"nonexistent_tool" in r.getMessage() and "async graph" in r.getMessage()
|
||||
for r in debugs
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user