fix(agents): clean up skeleton dep stubs + address review feedback (#1449)

Author: Alexey Tyurin

docs/plans/tool-loader.mdx

@@ -275,9 +275,11 @@ otherwise `self.tool_loader is None` and the agent stays on the legacy path.
 **Bundles (cohesion groups, pulled in whole on a member match):** `rag_query`,
 `rag_index`, `file_search`, `file_browse`, `file_edit`, `data`, `shell`,
 `clipboard`, `desktop`, `vision`, `memory`, `loop_control`. CORE ∪ all bundle
-members must equal the 37-tool `doc` registry exactly — enforced at runtime by
-`ToolLoader.validate_registry` and in CI by `test_chat_tool_bundles.py`, so a new
-doc tool forces a conscious bundling decision.
+members must equal the 37-tool `doc` registry exactly. The drift guard is the CI
+test `test_chat_tool_bundles.py` (it compares both sets, so a new doc tool forces
+a conscious bundling decision); `ToolLoader.validate_registry`, called once on
+first `select`, additionally fails loudly at runtime if a CORE/bundle name is
+missing from the registry.
 
 **Selection each turn** = CORE ∪ semantically-matched tools (+ their bundles),
 accumulated into a session-scoped *loaded set* that only grows, then sorted. The

src/gaia/agents/base/tool_loader.py

@@ -167,8 +167,14 @@ def session_disabled(self) -> bool:
     def validate_registry(self, registry: Dict[str, dict]) -> None:
         """Raise if CORE or any bundle names a tool absent from *registry*.
 
-        Fails loudly on drift so a new doc-profile tool forces a conscious
-        bundling decision rather than silently slipping through unselected.
+        Checks the config→registry direction only (no dead/misspelled CORE or
+        bundle names). The host agent calls this once on first activation
+        (ChatAgent does, since its doc CORE∪bundles must cover the registry) so
+        genuine drift fails loudly at runtime. ``select`` itself does *not* call
+        it — the loader tolerates bundle members absent from the registry by
+        design. The reverse direction (a *registry* tool not covered by
+        CORE/bundles, which would silently never be surfaced) is the CI bundle
+        test's job — it compares both sets exactly.
         """
         names = set(registry)
         missing_core = sorted(self._core - names)

src/gaia/agents/chat/agent.py

@@ -343,6 +343,7 @@ def __init__(self, config: Optional[ChatAgentConfig] = None):
         # loader never imports MemoryMixin; they resolve lazily on first select(),
         # by which point init_memory() has probed the embedder.
         self._dynamic_tools_native_warned = False
+        self._dynamic_tools_validated = False
         self.tool_loader = self._maybe_build_tool_loader()
 
         # Initialize memory subsystem (before super().__init__ which calls _register_tools)
@@ -521,6 +522,11 @@ def _select_tools_for_turn(self, user_input: str) -> Optional[List[str]]:
         if not self._dynamic_tools_active():
             return None
         self._maybe_warn_native_tool_gap()
+        if not self._dynamic_tools_validated:
+            # Fail loudly on first activation if a CORE/bundle name doesn't exist
+            # in the live registry (drift). The reverse direction is the CI test.
+            self.tool_loader.validate_registry(self._tools_registry)
+            self._dynamic_tools_validated = True
         query = self._build_tool_selection_query(user_input)
         return self.tool_loader.select(query, self._tools_registry)
 

src/gaia/agents/chat/tool_bundles.py

@@ -7,10 +7,13 @@
 pulled in whole when any member is semantically matched.
 
 ``DOC_CORE_TOOLS`` ∪ all ``DOC_BUNDLES`` members must equal the doc-profile
-registry **exactly** — enforced at runtime by ``ToolLoader.validate_registry``
-(fails loudly on drift) and in CI by ``tests/unit/test_chat_tool_bundles.py``. A
-new doc-profile tool therefore forces a conscious bundling decision instead of
-silently shipping unselected.
+registry **exactly**. The drift guard is the CI test
+``tests/unit/test_chat_tool_bundles.py`` — it compares both sets and fails the
+build if a registry tool is uncovered *or* a configured name is absent, so a new
+doc-profile tool forces a conscious bundling decision instead of silently
+shipping unselected. At runtime, ``ToolLoader.validate_registry`` (called once on
+first ``select``) additionally fails loudly if any CORE/bundle name is missing
+from the registry (the config→registry direction only).
 """
 
 from __future__ import annotations

src/gaia/eval/tool_cost.py

@@ -88,14 +88,24 @@
 )
 
 
-def _ensure_optional_deps_stubbed() -> None:
-    """Stub missing heavy optional deps so the chat agent can import."""
+def _ensure_optional_deps_stubbed() -> List[str]:
+    """Stub missing heavy optional deps so the chat agent can import.
+
+    Returns the names this call newly stubbed, so the caller can remove them
+    again — a leaked ``MagicMock`` makes ``import faiss`` *succeed* for later
+    tests that probe for the real dep, turning their graceful-skip into a hard
+    failure. Modules already present (real or stubbed by someone else) are left
+    untouched and not returned.
+    """
+    stubbed: List[str] = []
     for mod in _OPTIONAL_DEPS:
         if mod in sys.modules:
             continue
         if importlib.util.find_spec(mod) is not None:
             continue
         sys.modules[mod] = MagicMock()
+        stubbed.append(mod)
+    return stubbed
 
 
 def get_tokenizer() -> Optional[Any]:
@@ -156,50 +166,60 @@ def build_doc_agent_skeleton(
 
     The freshly registered tools are snapshotted into the instance via
     ``_instance_tools``; the global registry is restored on the way out.
-    """
-    _ensure_optional_deps_stubbed()
-
-    from gaia.agents.chat.agent import ChatAgent, ChatAgentConfig
 
-    cfg = ChatAgentConfig(
-        rag_documents=[],
-        streaming=False,
-        silent_mode=True,
-        prompt_profile=profile,
-    )
+    Cleans up after itself: any ``MagicMock`` dep stubs this call adds are
+    removed from ``sys.modules`` on exit. Without that, a leaked ``faiss``/
+    ``pypdf`` mock makes ``import faiss`` *succeed* for later tests that probe
+    for the real dep — turning their graceful-skip into a hard failure. (Only
+    the stubs are removed; gaia modules stay cached, since the consumers that
+    matter import these deps lazily, so dropping the stub is enough.)
+    """
+    stubbed = _ensure_optional_deps_stubbed()
+    try:
+        from gaia.agents.chat.agent import ChatAgent, ChatAgentConfig
 
-    with _isolated_registry() as tools_mod:
-        stack = contextlib.ExitStack()
-        stack.enter_context(
-            patch("gaia.agents.base.agent.Agent.__init__", return_value=None)
+        cfg = ChatAgentConfig(
+            rag_documents=[],
+            streaming=False,
+            silent_mode=True,
+            prompt_profile=profile,
         )
-        if deterministic:
-            # No npx -> search_documentation skipped; scrub PERPLEXITY_API_KEY
-            # -> search_web skipped. clear=True + filtered copy removes the key
-            # for the duration and restores the full environment afterwards.
-            stack.enter_context(patch("shutil.which", return_value=None))
-            scrubbed = {
-                k: v for k, v in os.environ.items() if k != "PERPLEXITY_API_KEY"
-            }
-            stack.enter_context(patch.dict(os.environ, scrubbed, clear=True))
-
-        with stack:
-            agent = ChatAgent.__new__(ChatAgent)
-            agent.config = cfg
-            agent._instance_tools = None
-            agent.model_id = "Gemma-4-E4B-it-GGUF"
-            agent._memory_store = MagicMock()  # non-None -> memory tools register
-            agent.rag = MagicMock()
-            agent.console = MagicMock()
-            # Satisfy ChatAgent.__del__ so GC of the skeleton stays quiet.
-            agent.observers = []
-            agent._web_client = None
-            agent._fs_index = None
-            agent._scratchpad = None
-            agent._register_tools()
-            agent._instance_tools = dict(tools_mod._TOOL_REGISTRY)
-
-    return agent
+
+        with _isolated_registry() as tools_mod:
+            stack = contextlib.ExitStack()
+            stack.enter_context(
+                patch("gaia.agents.base.agent.Agent.__init__", return_value=None)
+            )
+            if deterministic:
+                # No npx -> search_documentation skipped; scrub PERPLEXITY_API_KEY
+                # -> search_web skipped. clear=True + filtered copy removes the key
+                # for the duration and restores the full environment afterwards.
+                stack.enter_context(patch("shutil.which", return_value=None))
+                scrubbed = {
+                    k: v for k, v in os.environ.items() if k != "PERPLEXITY_API_KEY"
+                }
+                stack.enter_context(patch.dict(os.environ, scrubbed, clear=True))
+
+            with stack:
+                agent = ChatAgent.__new__(ChatAgent)
+                agent.config = cfg
+                agent._instance_tools = None
+                agent.model_id = "Gemma-4-E4B-it-GGUF"
+                agent._memory_store = MagicMock()  # non-None -> memory tools register
+                agent.rag = MagicMock()
+                agent.console = MagicMock()
+                # Satisfy ChatAgent.__del__ so GC of the skeleton stays quiet.
+                agent.observers = []
+                agent._web_client = None
+                agent._fs_index = None
+                agent._scratchpad = None
+                agent._register_tools()
+                agent._instance_tools = dict(tools_mod._TOOL_REGISTRY)
+
+        return agent
+    finally:
+        for mod in stubbed:
+            sys.modules.pop(mod, None)
 
 
 def _render_paths(

src/gaia/eval/tool_recall.py

@@ -132,6 +132,14 @@ def parse_loaded_sets_from_log(text: str) -> List[List[List[str]]]:
 
     A new scenario begins at each ``"turn": 1`` selection line (the loader resets
     its turn counter per conversation).
+
+    Assumption: every scenario emits a ``turn == 1`` line. A turn-1 *embedder
+    failure* session-disables the loader before ``_log_selection`` runs, so that
+    scenario emits no ``TOOL_LOADER`` line and its boundary is missed — quietly
+    shifting the per-scenario alignment. This is an eval-tooling edge case (the
+    embedder is up for a real recall run); if it happens, the per-scenario /
+    per-turn count mismatch surfaces as an alignment warning in
+    :func:`compute_recall` rather than passing silently.
     """
     scenarios: List[List[List[str]]] = []
     current: List[List[str]] = []
@@ -167,8 +175,8 @@ def _model_is_native(scorecard: Dict) -> bool:
         from gaia.llm.lemonade_client import is_tool_calling_model
 
         return is_tool_calling_model(model)
-    except Exception:
-        # If we can't classify, treat as native so misses don't hard-fail.
+    except ImportError:
+        # Can't import the classifier — treat as native so misses don't hard-fail.
         return True
 
 

tests/unit/test_chat_dynamic_tools.py

@@ -45,6 +45,7 @@ def _bare_agent(**attrs) -> ChatAgent:
     a.tool_loader = None
     a._memory_store = object()
     a._dynamic_tools_native_warned = False
+    a._dynamic_tools_validated = False
     a.model_id = None
     for k, v in attrs.items():
         setattr(a, k, v)