Skip to content

Commit c4495a0

Browse files
authored
fix(agents): dispatch Python-call-style embedded tool syntax (#2573)
On the NPU (non-tool-calling) path, the model sometimes emits a tool call as Python-call syntax, e.g. `remember(fact="...", category="...")`, instead of the JSON shape the prompt teaches. The parser only recognized JSON, so that text fell through to the plain-text answer path — the raw syntax was shown to the user and the tool never ran (in one case the model even claimed the action succeeded when it hadn't). `_extract_embedded_tool_call` now falls back to a Python-call-syntax detector: only names matching a tool actually registered on the agent are treated as calls (so ordinary prose isn't misfired on), arguments are parsed with `ast.literal_eval`, and a matched tool name with unparseable arguments raises a loud `ValueError` instead of being echoed — reusing the existing tool-call-parse-error recovery loop in `process_query`. Closes #2521 **Not fixed here (flagging per plan):** tool-call logging is inconsistent — some tools that execute and mutate state produce no `log_tool_call` entry (e.g. `set_low_priority_sender`), while others do (`list_inbox`, `triage_inbox`). This made diagnosing #2521 harder (database inspection was the only reliable signal). Filing separately rather than expanding this PR's scope. **Also per plan:** this touches core agent tool-call parsing, which normally requires an eval run before merge — intentionally not run here per the orchestrator's instruction; category to check before merge is `agent_behavior` / tool-call related scenarios. ## Test plan - [x] New failing-first unit tests in `tests/unit/agents/test_embedded_function_call_syntax.py` reproduce the exact reported response (`remember(fact="TechCrunch emails are low priority", category="preference")`) and fail before the fix - [x] `.venv-wt/bin/python -m pytest tests/unit/agents/test_embedded_function_call_syntax.py -q` → 8 passed - [x] `.venv-wt/bin/python -m pytest tests/unit/agents/ -q` → 486 passed, 60 skipped (3 pre-existing failures unrelated to this change — missing `fastapi`/`[ui]` extra in the test env) - [x] `python util/lint.py --all` → all checks pass - [ ] Not run (per instructions): `gaia eval agent`, and no NPU/FastFlowLM hardware test — this defect only reproduces on that backend
1 parent f6103a0 commit c4495a0

2 files changed

Lines changed: 350 additions & 3 deletions

File tree

src/gaia/agents/base/agent.py

Lines changed: 121 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,42 @@ def _fix(m: re.Match) -> str:
228228
return re.sub(r"\\(.)", _fix, s)
229229

230230

231+
def _find_matching_close_paren(text: str, open_pos: int) -> Optional[int]:
232+
"""Return the index of the ``)`` that closes ``text[open_pos]`` (a ``(``).
233+
234+
Depth- and quote-aware (mirrors the brace matcher used for embedded JSON
235+
tool calls) so a ``)`` inside a quoted argument value doesn't close the
236+
call early. Returns ``None`` if the call is never terminated.
237+
"""
238+
depth = 0
239+
in_str = False
240+
quote_char = ""
241+
escape = False
242+
for j in range(open_pos, len(text)):
243+
ch = text[j]
244+
if escape:
245+
escape = False
246+
continue
247+
if ch == "\\":
248+
escape = True
249+
continue
250+
if in_str:
251+
if ch == quote_char:
252+
in_str = False
253+
continue
254+
if ch in ("'", '"'):
255+
in_str = True
256+
quote_char = ch
257+
continue
258+
if ch == "(":
259+
depth += 1
260+
elif ch == ")":
261+
depth -= 1
262+
if depth == 0:
263+
return j
264+
return None
265+
266+
231267
# Suffix appended to the last tool-result message when ``single_tool_per_turn``
232268
# agents have completed their one tool call. The model sees this and emits a
233269
# short final reply instead of calling another tool. Greppable for fixtures
@@ -920,16 +956,25 @@ def _extract_embedded_tool_call(self, response: str) -> Optional[Dict[str, Any]]
920956
1. ≥1 unfenced candidate → return the first (unchanged — zero regression).
921957
2. else exactly one fenced candidate → return it (the fix for #1428).
922958
3. else >1 fenced, 0 unfenced → ambiguous (looks like docs) → None + warning.
923-
4. else → None.
959+
4. else → fall back to Python-call syntax detection (#2521), e.g.
960+
``remember(fact="...", category="preference")``.
924961
925962
This method finds the JSON block using brace-depth matching and returns
926963
the parsed tool call if it contains a "tool" key. Returns None if no
927964
embedded tool call is found, allowing the caller to treat the response
928965
as plain text.
966+
967+
Raises:
968+
ValueError: propagated from the Python-call-syntax fallback (#2521)
969+
when a *registered* tool's name is followed by an argument list
970+
that can't be parsed — a loud failure rather than echoing the
971+
raw syntax to the user as an answer.
929972
"""
930-
# Quick check: must contain "tool" to be worth scanning
973+
# Quick check: must contain "tool" to be worth scanning for the JSON
974+
# shape. Responses without it may still carry the #2521 Python-call
975+
# shape below (e.g. no literal "tool" substring at all).
931976
if '"tool"' not in response:
932-
return None
977+
return self._extract_function_call_tool_syntax(response)
933978

934979
# Build a set of character ranges inside code fences (```...```)
935980
_code_ranges: list[tuple[int, int]] = []
@@ -1043,6 +1088,79 @@ def _parse_candidate(raw: str) -> Optional[Dict[str, Any]]:
10431088
)
10441089
return None
10451090

1091+
# Rule 4: the "tool" marker was present but matched no JSON-shaped
1092+
# candidate (e.g. it appeared in unrelated text) — fall back to the
1093+
# Python-call syntax detector (#2521) before giving up.
1094+
return self._extract_function_call_tool_syntax(response)
1095+
1096+
_FUNC_CALL_NAME_RE = re.compile(r"\b([A-Za-z_][A-Za-z0-9_]*)\s*\(")
1097+
1098+
def _extract_function_call_tool_syntax(
1099+
self, response: str
1100+
) -> Optional[Dict[str, Any]]:
1101+
"""Detect a Python-call-style tool invocation embedded in text (#2521).
1102+
1103+
On the non-tool-calling (embedded-JSON) path the prompt only ever
1104+
teaches the JSON shape ``{"tool": "name", "tool_args": {...}}``, but
1105+
some models — observed on the FastFlowLM/NPU backend — instead emit
1106+
a bare Python-style call, e.g.::
1107+
1108+
remember(fact="TechCrunch emails are low priority", category="preference")
1109+
1110+
Without this, that text falls through to the plain-text answer path:
1111+
the raw call syntax is shown to the user and the tool never runs.
1112+
1113+
Only names that match a tool actually **registered** on this agent
1114+
are treated as calls, so ordinary prose that happens to contain
1115+
"word(...)" (code snippets, examples) is left as plain text. A name
1116+
match with an argument list that can't be parsed is a loud failure
1117+
(raises ``ValueError``) rather than being echoed to the user.
1118+
1119+
Returns:
1120+
``{"tool": name, "tool_args": {...}}`` on a successful match, or
1121+
``None`` if no registered-tool call syntax is present.
1122+
1123+
Raises:
1124+
ValueError: a registered tool's name is followed by an argument
1125+
list that could not be parsed as Python literals.
1126+
"""
1127+
registry = self._tools_registry
1128+
if not registry:
1129+
return None
1130+
1131+
for match in self._FUNC_CALL_NAME_RE.finditer(response):
1132+
name = match.group(1)
1133+
if name not in registry:
1134+
continue
1135+
1136+
open_paren = match.end() - 1
1137+
close_paren = _find_matching_close_paren(response, open_paren)
1138+
if close_paren is None:
1139+
raise ValueError(
1140+
f"Detected an unterminated call to tool '{name}' — cannot "
1141+
"execute it. Raw text: "
1142+
f"{response[match.start():match.start() + 200]!r}"
1143+
)
1144+
1145+
call_src = response[match.start() : close_paren + 1]
1146+
try:
1147+
node = ast.parse(call_src, mode="eval").body
1148+
if not isinstance(node, ast.Call) or node.args:
1149+
raise ValueError("expected a call with only keyword arguments")
1150+
tool_args: Dict[str, Any] = {}
1151+
for kw in node.keywords:
1152+
if kw.arg is None:
1153+
raise ValueError("**kwargs expansion is not supported")
1154+
tool_args[kw.arg] = ast.literal_eval(kw.value)
1155+
except (SyntaxError, ValueError) as exc:
1156+
raise ValueError(
1157+
f"Detected a call to tool '{name}' but could not parse "
1158+
f"its arguments: {exc}. Raw call: {call_src[:200]!r}"
1159+
) from exc
1160+
1161+
logger.debug("[PARSE] Extracted function-call-syntax tool call: %s", name)
1162+
return {"tool": name, "tool_args": tool_args}
1163+
10461164
return None
10471165

10481166
def _extract_json_from_response(self, response: str) -> Optional[Dict[str, Any]]:
Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
# Copyright(C) 2025-2026 Advanced Micro Devices, Inc. All rights reserved.
2+
# SPDX-License-Identifier: MIT
3+
"""Unit tests for #2521: embedded tool-call syntax echoed as text on the
4+
non-tool-calling (NPU / FastFlowLM) path.
5+
6+
On that path the model sometimes emits a Python-call-style invocation
7+
instead of the JSON shape the prompt teaches, e.g.::
8+
9+
remember(fact="TechCrunch emails are low priority", category="preference")
10+
11+
Before this fix ``_extract_embedded_tool_call`` only recognised the JSON
12+
shape (``{"tool": ..., "tool_args": ...}``); the call-syntax text fell
13+
through to the plain-text answer path, so the raw syntax reached the user
14+
and the tool never ran.
15+
"""
16+
17+
import json
18+
from unittest.mock import MagicMock, patch
19+
20+
import pytest
21+
22+
from gaia.agents.base.agent import Agent
23+
24+
25+
class _DummyAgent(Agent):
26+
"""Minimal concrete Agent for testing."""
27+
28+
def _get_system_prompt(self) -> str:
29+
return "You are a test agent."
30+
31+
def _register_tools(self) -> None:
32+
pass
33+
34+
def _create_console(self):
35+
from gaia.agents.base.console import AgentConsole
36+
37+
return AgentConsole()
38+
39+
40+
@pytest.fixture
41+
def agent():
42+
with patch("gaia.agents.base.agent.AgentSDK"):
43+
a = _DummyAgent(silent_mode=True, skip_lemonade=True)
44+
a.streaming = False
45+
return a
46+
47+
48+
def _register_remember(agent, result=None):
49+
"""Register a fake ``remember`` tool in this instance's snapshot only."""
50+
calls = []
51+
result = result if result is not None else {"status": "success"}
52+
53+
def _remember(**kwargs):
54+
calls.append(kwargs)
55+
return result
56+
57+
agent._instance_tools = {
58+
"remember": {
59+
"name": "remember",
60+
"description": "stub",
61+
"parameters": {
62+
"fact": {"type": "string", "required": True},
63+
"category": {"type": "string", "required": False},
64+
},
65+
"function": _remember,
66+
"atomic": True,
67+
}
68+
}
69+
return calls
70+
71+
72+
# ---------------------------------------------------------------------------
73+
# 1. Core guard: bare function-call syntax dispatches the tool
74+
# ---------------------------------------------------------------------------
75+
76+
77+
class TestFunctionCallSyntaxParsing:
78+
def test_bare_call_is_parsed_as_tool_call(self, agent):
79+
_register_remember(agent)
80+
response = (
81+
'remember(fact="TechCrunch emails are low priority", '
82+
'category="preference")'
83+
)
84+
parsed = agent._parse_llm_response(response)
85+
assert parsed.get("tool") == "remember"
86+
assert parsed.get("tool_args") == {
87+
"fact": "TechCrunch emails are low priority",
88+
"category": "preference",
89+
}
90+
# The raw call syntax must not be echoed as an "answer".
91+
assert "answer" not in parsed or parsed.get("answer") != response
92+
93+
def test_unregistered_name_is_left_as_plain_text(self, agent):
94+
"""A word(...) pattern that isn't a registered tool stays plain text."""
95+
_register_remember(agent)
96+
response = 'Please call cleanup(now="true") if needed.'
97+
parsed = agent._parse_llm_response(response)
98+
assert not parsed.get("tool")
99+
assert parsed.get("answer") == response
100+
101+
def test_call_followed_by_prose_still_dispatches(self, agent):
102+
"""A success-claiming sentence after the call must not become the
103+
final answer -- the tool call is dispatched instead (same class as
104+
#2520: never let the model claim an unexecuted action succeeded)."""
105+
_register_remember(agent)
106+
response = (
107+
'remember(fact="TechCrunch emails are low priority", '
108+
'category="preference")\n'
109+
"I have updated my preferences to treat emails from "
110+
"TechCrunch as low priority."
111+
)
112+
parsed = agent._parse_llm_response(response)
113+
assert parsed.get("tool") == "remember"
114+
assert parsed.get("tool_args") == {
115+
"fact": "TechCrunch emails are low priority",
116+
"category": "preference",
117+
}
118+
# The trailing success claim must not surface as the answer.
119+
assert parsed.get("answer") != response
120+
assert "answer" not in parsed or "updated my preferences" not in (
121+
parsed.get("answer") or ""
122+
)
123+
124+
125+
# ---------------------------------------------------------------------------
126+
# 2. Unparseable call -> loud, actionable failure (never echoed)
127+
# ---------------------------------------------------------------------------
128+
129+
130+
class TestUnparseableCallRaises:
131+
def test_unquoted_kwarg_value_raises_actionable_error(self, agent):
132+
"""A registered tool name followed by malformed args must raise --
133+
not be echoed back to the user as plain text."""
134+
_register_remember(agent)
135+
response = "remember(fact=TechCrunch is low priority, category=preference)"
136+
with pytest.raises(ValueError, match="remember"):
137+
agent._parse_llm_response(response)
138+
139+
def test_unterminated_call_raises_actionable_error(self, agent):
140+
_register_remember(agent)
141+
response = 'remember(fact="TechCrunch emails are low priority"'
142+
with pytest.raises(ValueError, match="remember"):
143+
agent._parse_llm_response(response)
144+
145+
146+
# ---------------------------------------------------------------------------
147+
# 3. Native tool-calling models are unaffected
148+
# ---------------------------------------------------------------------------
149+
150+
151+
class TestNativeToolCallingUnaffected:
152+
def test_native_sentinel_envelope_unaffected(self, agent):
153+
"""The __tool_calls__ sentinel path is handled before any embedded
154+
extraction and must keep working unchanged."""
155+
_register_remember(agent)
156+
response = json.dumps(
157+
{
158+
"__tool_calls__": [
159+
{
160+
"function": {
161+
"name": "remember",
162+
"arguments": json.dumps(
163+
{
164+
"fact": "TechCrunch emails are low priority",
165+
"category": "preference",
166+
}
167+
),
168+
}
169+
}
170+
]
171+
}
172+
)
173+
parsed = agent._parse_llm_response(response)
174+
assert parsed["tool"] == "remember"
175+
assert parsed["tool_args"]["category"] == "preference"
176+
177+
def test_plain_prose_without_any_registered_tool_name_untouched(self, agent):
178+
_register_remember(agent)
179+
response = "Sure, I can help with that. What would you like to know?"
180+
parsed = agent._parse_llm_response(response)
181+
assert not parsed.get("tool")
182+
assert parsed.get("answer") == response
183+
184+
185+
# ---------------------------------------------------------------------------
186+
# 4. End-to-end: process_query dispatches the tool and never leaks raw syntax
187+
# ---------------------------------------------------------------------------
188+
189+
190+
class TestProcessQueryDispatchesEmbeddedFunctionCallSyntax:
191+
def _stub_chat(self, agent, *responses):
192+
responses = list(responses)
193+
chat = MagicMock()
194+
195+
def _send(*_, **__):
196+
r = responses.pop(0)
197+
resp = MagicMock()
198+
resp.text = r
199+
resp.stats = {}
200+
return resp
201+
202+
chat.send_messages = MagicMock(side_effect=_send)
203+
agent.chat = chat
204+
return chat
205+
206+
def test_embedded_call_executes_and_raw_syntax_never_reaches_user(self, agent):
207+
calls = _register_remember(agent)
208+
step1 = (
209+
'remember(fact="TechCrunch emails are low priority", '
210+
'category="preference")'
211+
)
212+
step2 = json.dumps({"thought": "done", "answer": "Got it, noted."})
213+
self._stub_chat(agent, step1, step2)
214+
215+
result = agent.process_query(
216+
"From now on treat anything from TechCrunch as low priority.",
217+
max_steps=5,
218+
)
219+
220+
# The tool actually ran.
221+
assert calls == [
222+
{
223+
"fact": "TechCrunch emails are low priority",
224+
"category": "preference",
225+
}
226+
]
227+
text = result.get("result") if isinstance(result, dict) else str(result)
228+
# Raw internal call syntax never reached the user-visible output.
229+
assert "remember(" not in (text or "")

0 commit comments

Comments
 (0)