mirror of
https://github.com/Niko1221/Strata.git
synced 2026-11-07 06:15:58 -10:00
serve: tool-call aliases and wrapper boundaries (port of PR #3556), and an unparsed call is logged (T1)
Ported from #3556 (Amit Tsur): a declared namespaced tool written with its flat alias (mcp__server__tool for mcp__server.tool) keeps its schema while streaming and is reported under the declared name; an ambiguous alias is no alias; a quoted incomplete wrapper (`Format: <tool_call>... </function> and then I explain it.`) stays text; a following call opener separates concatenated calls; streamed JSON is closed only once the wrapper or the next call confirms the boundary; a call cut by the output limit stays incomplete. The Responses-only alias map is gone (the parser resolves aliases now). Reconciled with the 0.2.2 envelope rule: with tools declared, a call for an undeclared name stays text on every route (the PR's tests that expected a native call for an undeclared name now expect text, or a native call only where no tools were declared and declared_only is off: the Responses/Codex compaction route keeps that). The aliases attribute replaces the envelope work's _alias set, so the lenient opener (#3441) and the declared-only gate use the PR's unambiguous alias table. New rail: a call that named a tool the request did not declare is counted (parser.unread) with the names the model wrote (parser.unread_names), and the server's existing log line for unread calls now prints them. Co-authored-by: Amit Tsur <[email protected]>
This commit is contained in:
1 parent
1fa22cd29a
commit
8bf178febf
8 files changed
+388
-32
No files matched your search
+57
-17
@@ -895,8 +895,18 @@ class OutputParser:
|
||||
# the client has no tool to run). The Responses route leaves it off: a Codex compaction request keeps its
|
||||
# conversation's tools in the prompt only, and its calls come back as before.
|
||||
self.declared_only = declared_only
|
||||
self._alias = {k.replace(".", "__", 1) for k in self.schemas if isinstance(k, str) and "." in k}
|
||||
# PR #3556: the native namespace__member spelling of a declared namespace.member tool is accepted without advertising a
|
||||
# duplicate tool. Only an unambiguous alias counts (two namespaces can end in the same member), and an exactly declared
|
||||
# name always wins over an alias.
|
||||
aliases = {}
|
||||
for name in self.schemas:
|
||||
if isinstance(name, str) and "." in name:
|
||||
namespace, member = name.rsplit(".", 1)
|
||||
aliases.setdefault(f"{namespace}__{member}", set()).add(name)
|
||||
self.aliases = {alias: next(iter(names)) for alias, names in aliases.items()
|
||||
if len(names) == 1 and alias not in self.schemas}
|
||||
self.unread = 0 # #3441: `<tool_call>` blocks that opened like a call and were returned as text
|
||||
self.unread_names: list[str] = [] # the names (first few) of those that named a tool the request did not declare
|
||||
# stream_tools: a tool call is also reported while it is being written - "tool_start" (its name and id) as
|
||||
# soon as the name is known, then "tool_args" pieces of its JSON arguments (string parameters character by
|
||||
# character; other types whole, once complete) - before the final "tool_call". Without it, a client sees
|
||||
@@ -930,7 +940,7 @@ class OutputParser:
|
||||
none declared, only where `declared_only` is off."""
|
||||
if not self.schemas:
|
||||
return not self.declared_only
|
||||
return name in self.schemas or name in self._alias
|
||||
return name in self.schemas or name in self.aliases
|
||||
|
||||
def _loose_open(self, after: str):
|
||||
"""#3441: `after` (text after a `<tool_call>`, whitespace dropped) opens a call in a drifted form that names a
|
||||
@@ -945,7 +955,7 @@ class OutputParser:
|
||||
bare = after[1:].strip().lstrip("=").strip()
|
||||
if after[1:].lstrip().startswith(("=", "function")):
|
||||
return "wait"
|
||||
if any(k.startswith(bare) for k in list(self.schemas) + list(self._alias) if isinstance(k, str)):
|
||||
if any(k.startswith(bare) for k in list(self.schemas) + list(self.aliases) if isinstance(k, str)):
|
||||
return "wait"
|
||||
return None
|
||||
|
||||
@@ -981,8 +991,10 @@ class OutputParser:
|
||||
while p >= 0:
|
||||
if self._ok_at(p):
|
||||
gt = self.buf.find(">", p)
|
||||
name = self.buf[p + len(FUNC_START):gt if gt >= 0 else len(self.buf)]
|
||||
if gt < 0 and not any(c.isspace() for c in name):
|
||||
raw_name = self.buf[p + len(FUNC_START):gt if gt >= 0 else len(self.buf)]
|
||||
name = raw_name.strip()
|
||||
name = self.aliases.get(name, name)
|
||||
if gt < 0 and re.fullmatch(r"\s*[^<>\s]*\s*", raw_name):
|
||||
return p, False
|
||||
if gt >= 0 and name in self.schemas:
|
||||
return p, True
|
||||
@@ -1082,7 +1094,7 @@ class OutputParser:
|
||||
def _reset_scan(self):
|
||||
self._call_end_checked = 0 # closing-tag search progress for the current wrapped call
|
||||
self.sp = 0 # how much of self.buf (the call body) the scanner has consumed
|
||||
self.ss = "name" # name -> between -> str|raw -> ... -> done
|
||||
self.ss = "name" # name -> between -> str|raw -> ... -> function_done -> done
|
||||
self.scall = None # the ToolCall being streamed (its id is reused by the final event)
|
||||
self.sfirst = True
|
||||
self.sval_started = False
|
||||
@@ -1116,7 +1128,11 @@ class OutputParser:
|
||||
b = rest.find(">", a + 10) if a >= 0 else -1
|
||||
if b < 0:
|
||||
return out
|
||||
name = rest[a + 10:b]
|
||||
raw_name = rest[a + 10:b]
|
||||
name = raw_name.strip()
|
||||
name = self.aliases.get(name, name)
|
||||
if raw_name != raw_name.strip() and name not in self.schemas:
|
||||
return out
|
||||
self.scall = ToolCall(name=name, arguments={})
|
||||
props = ((self.schemas.get(name) or {}).get("parameters") or {}).get("properties") or {}
|
||||
self.sdeclared = {k: (v or {}).get("type") for k, v in props.items()}
|
||||
@@ -1141,9 +1157,8 @@ class OutputParser:
|
||||
else:
|
||||
self.ss = "raw"
|
||||
elif stripped.startswith("</function>"):
|
||||
args("}")
|
||||
self.sp += len("</function>")
|
||||
self.ss = "done"
|
||||
self.ss = "function_done"
|
||||
else:
|
||||
return out # a tag still arriving (or trailing text): wait
|
||||
elif self.ss == "str":
|
||||
@@ -1191,7 +1206,7 @@ class OutputParser:
|
||||
return out
|
||||
|
||||
def _close_scan(self) -> list[Event]:
|
||||
"""A streamed call that ended without a clean </function>: close its JSON so clients can still parse it."""
|
||||
"""Close streamed JSON only after its call boundary has been confirmed."""
|
||||
out = []
|
||||
if self.scall is None or self.ss == "done":
|
||||
return out
|
||||
@@ -1236,8 +1251,10 @@ class OutputParser:
|
||||
call = None
|
||||
try:
|
||||
name = body[:end].strip()[len("<function="):].split(">", 1)[0].strip()
|
||||
name = self.aliases.get(name, name)
|
||||
if name in self.schemas:
|
||||
call = parse_tool_call(body[:end], self.schemas.get(name))
|
||||
call.name = name
|
||||
except ValueError:
|
||||
pass
|
||||
raw = self.buf[:len(CALL_START) + end + len(CALL_END)]
|
||||
@@ -1393,11 +1410,19 @@ class OutputParser:
|
||||
continue
|
||||
if kind == "wait":
|
||||
after = ""
|
||||
if after and not after.startswith(FUNC_START) and not FUNC_START.startswith(after):
|
||||
is_call = re.match(r"<function=(\s*[^<>\s]+\s*)>", after)
|
||||
if is_call:
|
||||
raw_name = is_call.group(1)
|
||||
name = raw_name.strip()
|
||||
name = self.aliases.get(name, name)
|
||||
if raw_name != raw_name.strip() and name not in self.schemas:
|
||||
is_call = None # only declared names gain whitespace normalization
|
||||
is_partial = FUNC_START.startswith(after) or re.fullmatch(r"<function=\s*[^<>\s]*\s*", after)
|
||||
if not is_call and not is_partial:
|
||||
out.append(Event("content", self._track(self.buf[:i + len(CALL_START)])))
|
||||
self.buf = self.buf[i + len(CALL_START):]
|
||||
continue
|
||||
if not after.startswith(FUNC_START):
|
||||
if not is_call:
|
||||
j = i
|
||||
while j > 0 and self.buf[j - 1] == "\n":
|
||||
j -= 1
|
||||
@@ -1418,6 +1443,9 @@ class OutputParser:
|
||||
self.buf = self.buf[j:]
|
||||
return out
|
||||
if gt >= 0 and (nl < 0 or gt < nl) and not self._callable(after[len(FUNC_START):gt].strip()):
|
||||
self.unread += 1 # logged by the server: otherwise "the model ignored my tool" is the only clue
|
||||
if len(self.unread_names) < 5:
|
||||
self.unread_names.append(after[len(FUNC_START):gt].strip()[:80])
|
||||
out.append(Event("content", self._track(self.buf[:i + len(CALL_START)])))
|
||||
self.buf = self.buf[i + len(CALL_START):]
|
||||
continue
|
||||
@@ -1452,9 +1480,12 @@ class OutputParser:
|
||||
i = self._wrapped_call_end()
|
||||
if i >= 0:
|
||||
drop = len(CALL_END)
|
||||
if self.recover: # a batch in one wrapper: the next call follows this one's
|
||||
f = function_end(self.buf) # </function> (or opens as <parameter=NAME>)
|
||||
nxt = self.buf[f:].lstrip() if f >= 0 else ""
|
||||
# Only a following call opener confirms an omitted wrapper closer.
|
||||
f = function_end(self.buf)
|
||||
nxt = self.buf[f:].lstrip() if f >= 0 else ""
|
||||
if re.match(r"<tool_call>\s*<function=\s*[^<>\s]+\s*>", nxt):
|
||||
i, drop = f, 0
|
||||
if self.recover: # a batch in one wrapper: the next call follows </function>
|
||||
if nxt.startswith(PARAM_START) and self._follower(nxt) == "param":
|
||||
at = len(self.buf) - len(nxt)
|
||||
self.buf = self.buf[:at] + FUNC_START + nxt[len(PARAM_START):]
|
||||
@@ -1496,8 +1527,12 @@ class OutputParser:
|
||||
return out
|
||||
if self.state == "call" and self.stream_tools and self.scall is not None:
|
||||
out += self._scan() # the output ended inside a call that was already announced
|
||||
if self.ss == "done": # only its </tool_call> is missing: the call itself is whole
|
||||
if self.ss == "function_done" and not self.buf[self.sp:].strip() and reason in (None, "stop"):
|
||||
# A natural end immediately after </function> retains native streaming recovery.
|
||||
out += self._close_scan()
|
||||
out.append(Event("tool_call", call=self.scall))
|
||||
elif self.ss == "function_done":
|
||||
out.append(Event("content", self._track(CALL_START + self.buf)))
|
||||
self.buf = ""
|
||||
self._reset_scan()
|
||||
return out
|
||||
@@ -1514,10 +1549,15 @@ class OutputParser:
|
||||
def _finish_call(self, body: str) -> list[Event]:
|
||||
"""A whole call body in the template's form -> its tool_call event. With recover, one that still does not
|
||||
parse is the text it is instead of an error that ends the request."""
|
||||
name = body.strip()[len(FUNC_START):].split(">", 1)[0].strip()
|
||||
raw_name = body.strip()[len(FUNC_START):].split(">", 1)[0]
|
||||
name = raw_name.strip()
|
||||
name = self.aliases.get(name, name)
|
||||
bare, self.bare = self.bare, False
|
||||
try:
|
||||
if raw_name != raw_name.strip() and name not in self.schemas:
|
||||
raise ValueError("whitespace tool name is not declared")
|
||||
call = parse_tool_call(body, self.schemas.get(name))
|
||||
call.name = name
|
||||
except ValueError:
|
||||
if not self.recover:
|
||||
raise
|
||||
|
||||
+3
-7
@@ -265,10 +265,6 @@ def request_tools(req: dict):
|
||||
"required": ["input"]}
|
||||
tools.append({"name": flat, "description": description, "parameters": params})
|
||||
names[flat] = (namespace, name, kind)
|
||||
if namespace:
|
||||
# Qwen writes Codex's MCP tools as `mcp__server__tool` (the flat name it
|
||||
# learned), not `mcp__server.tool`: map that spelling back as well.
|
||||
names.setdefault(f"{namespace}__{name}", (namespace, name, kind))
|
||||
|
||||
for i, tool in enumerate(given):
|
||||
param = f"tools[{i}]"
|
||||
@@ -523,12 +519,12 @@ class Assembler:
|
||||
def index(self) -> int:
|
||||
return len(self.response["output"]) - 1
|
||||
|
||||
def close(self, status="completed") -> list[dict]:
|
||||
def close(self, status=None) -> list[dict]:
|
||||
item, self.item = self.item, None
|
||||
if item is None:
|
||||
return []
|
||||
at, out = self.response["output"].index(item), []
|
||||
item["status"] = status
|
||||
item["status"] = status or ("incomplete" if item["type"] in ("function_call", "custom_tool_call") else "completed")
|
||||
if item["type"] == "message":
|
||||
part = item["content"][0]
|
||||
out += [self.event("response.output_text.done", item_id=item["id"], output_index=at, content_index=0,
|
||||
@@ -606,7 +602,7 @@ class Assembler:
|
||||
item["input"] = value if isinstance(value, str) else json.dumps(ev.call.arguments, ensure_ascii=False)
|
||||
out.append(self.event("response.custom_tool_call_input.delta", item_id=item["id"],
|
||||
output_index=self.index, delta=item["input"]))
|
||||
out += self.close()
|
||||
out += self.close("completed")
|
||||
return out
|
||||
|
||||
def _release(self, text: str) -> list[dict]:
|
||||
|
||||
+2
-1
@@ -5437,9 +5437,10 @@ class Service:
|
||||
print(f"[strata] tool calls inside the thinking: {parser.rescued} read as calls, {parser.refused} kept as "
|
||||
f"reasoning (quoted, or the turn did not end on a stop)", flush=True)
|
||||
if parser.unread:
|
||||
named = getattr(parser, "unread_names", None)
|
||||
print(f"[strata] {parser.unread} <tool_call> block(s) in the answer did not read as a call of a declared "
|
||||
"tool and went back as text (a name the request did not declare, or a form the template does "
|
||||
"not use)", flush=True)
|
||||
"not use)" + (f"; the names the model wrote: {', '.join(named)}" if named else ""), flush=True)
|
||||
if stops is not None and stops.hit is None and stops.held:
|
||||
yield "event", Event("content", stops.flush()) # the held tail was not a stop string after all
|
||||
done = {"finish": finish, "completion_tokens": n, "reused": (timings or {}).get("cache_n", 0),
|
||||
|
||||
@@ -0,0 +1,150 @@
|
||||
"""Native tool parsing regressions; no model or GPU required."""
|
||||
import unittest
|
||||
from serve.frontend import OutputParser
|
||||
|
||||
CALL = "<tool_call>\n<function=mcp__files__read>\n<parameter=path>\n123\n</parameter>\n</function>\n</tool_call>"
|
||||
TOOLS = [{"name": "mcp__files.read", "parameters": {"properties": {"path": {"type": "string"}}}}]
|
||||
|
||||
class NativeTools(unittest.TestCase):
|
||||
def test_unclosed_think_call_and_split_tags(self):
|
||||
for streaming in (False, True):
|
||||
for step in (1, 7, 4096):
|
||||
parser = OutputParser(thinking=True, tools=TOOLS, stream_tools=streaming)
|
||||
events = []
|
||||
text = "Inspect the file.\n" + CALL
|
||||
for i in range(0,len(text),step):
|
||||
events += parser.feed(text[i:i+step])
|
||||
events += parser.finish()
|
||||
calls = [e.call for e in events if e.kind == "tool_call"]
|
||||
self.assertEqual(len(calls),1)
|
||||
self.assertEqual((calls[0].name, calls[0].arguments), ("mcp__files.read", {"path": "123"}))
|
||||
self.assertEqual("".join(e.text for e in events if e.kind == "reasoning"), "Inspect the file.\n")
|
||||
|
||||
|
||||
def test_whitespace_name_keeps_namespace_schema_and_stream_events(self):
|
||||
for thinking, prefix in ((True, "Inspect the file.\n"), (True, "Ready.</think>\n\n"), (False, "")):
|
||||
for whitespace in (" ", "\t", " \t"):
|
||||
text = prefix + CALL.replace("<function=mcp__files__read>",
|
||||
"<function=" + whitespace + "mcp__files__read" + whitespace + ">")
|
||||
for streaming in (False, True):
|
||||
for step in (1, 7, 4096):
|
||||
with self.subTest(thinking=thinking, prefix=prefix, whitespace=whitespace,
|
||||
streaming=streaming, step=step):
|
||||
parser = OutputParser(thinking=thinking, tools=TOOLS, stream_tools=streaming)
|
||||
events = []
|
||||
for i in range(0, len(text), step):
|
||||
events += parser.feed(text[i:i + step])
|
||||
events += parser.finish()
|
||||
calls = [event.call for event in events if event.kind == "tool_call"]
|
||||
self.assertEqual([(c.name, c.arguments) for c in calls],
|
||||
[("mcp__files.read", {"path": "123"})])
|
||||
self.assertTrue(all(event.call.name == "mcp__files.read"
|
||||
for event in events if event.kind == "tool_start"))
|
||||
|
||||
|
||||
def test_padded_unknown_wrapper_stays_text_and_unpadded_stays_native(self):
|
||||
for recover in (False, True):
|
||||
for streaming in (False, True):
|
||||
for step in (1, 7, 4096):
|
||||
for name in (" missing ", "\tmissing\t", "mcp__ files", " "):
|
||||
text = "<tool_call><function=" + name + "></function></tool_call>"
|
||||
parser = OutputParser(thinking=False, tools=TOOLS, stream_tools=streaming, recover=recover)
|
||||
events = []
|
||||
for i in range(0, len(text), step):
|
||||
events += parser.feed(text[i:i + step])
|
||||
events += parser.finish()
|
||||
self.assertFalse(any(event.kind.startswith("tool") for event in events))
|
||||
self.assertEqual("".join(event.text for event in events if event.kind == "content"), text)
|
||||
# an unpadded name nobody declared: text where tools are declared (the 0.2.2 rule, #3075); the Responses
|
||||
# route of a request with no tools (Codex compaction, declared_only off) keeps it as a native call
|
||||
text = "<tool_call><function=missing></function></tool_call>"
|
||||
for tools, declared_only, expected in ((TOOLS, False, []), (TOOLS, True, []), (None, False, ["missing"])):
|
||||
parser = OutputParser(thinking=False, tools=tools, stream_tools=streaming, recover=recover,
|
||||
declared_only=declared_only)
|
||||
events = []
|
||||
for i in range(0, len(text), step):
|
||||
events += parser.feed(text[i:i + step])
|
||||
events += parser.finish()
|
||||
self.assertEqual([event.call.name for event in events if event.kind == "tool_call"], expected)
|
||||
|
||||
|
||||
def test_padded_unknown_second_call_is_not_announced(self):
|
||||
text = CALL[:-len("</tool_call>")] + "<function= missing ></function></tool_call>"
|
||||
for step in (1, 7, 4096):
|
||||
parser = OutputParser(thinking=False, tools=TOOLS, stream_tools=True, recover=True)
|
||||
events = []
|
||||
for i in range(0, len(text), step):
|
||||
events += parser.feed(text[i:i + step])
|
||||
events += parser.finish()
|
||||
self.assertEqual([event.call.name for event in events if event.kind == "tool_call"], ["mcp__files.read"])
|
||||
self.assertEqual([event.call.name for event in events if event.kind == "tool_start"], ["mcp__files.read"])
|
||||
self.assertIn("<function= missing >", "".join(event.text for event in events if event.kind == "content"))
|
||||
|
||||
|
||||
def test_padded_bare_alias_is_declared_and_schema_typed(self):
|
||||
text = CALL.replace("<tool_call>\n", "").replace("\n</tool_call>", "").replace(
|
||||
"<function=mcp__files__read>", "<function= mcp__files__read >")
|
||||
for step in (1, 7, 4096):
|
||||
parser = OutputParser(thinking=False, tools=TOOLS, stream_tools=True, recover=True)
|
||||
events = []
|
||||
for i in range(0, len(text), step):
|
||||
events += parser.feed(text[i:i + step])
|
||||
events += parser.finish()
|
||||
self.assertEqual([(event.call.name, event.call.arguments) for event in events if event.kind == "tool_call"],
|
||||
[("mcp__files.read", {"path": "123"})])
|
||||
starts = [event.call.name for event in events if event.kind == "tool_start"]
|
||||
self.assertEqual(starts, ["mcp__files.read"])
|
||||
|
||||
|
||||
def test_literal_call_markers_remain_reasoning_prose(self):
|
||||
for prefix in ("Quote <tool_call> as prose.", "Use `<tool_call>` in docs.", "Literal marker:\n<tool_call>\nContinue thinking.", "Empty <tool_call><function=> is prose."):
|
||||
for step in (1, 7, 4096):
|
||||
parser=OutputParser(thinking=True,tools=TOOLS,stream_tools=True)
|
||||
text=prefix+"</think>\n\nVisible answer."
|
||||
events=[]
|
||||
for i in range(0,len(text),step):events+=parser.feed(text[i:i+step])
|
||||
events+=parser.finish()
|
||||
self.assertEqual(''.join(e.text for e in events if e.kind=='reasoning'),prefix)
|
||||
self.assertEqual(''.join(e.text for e in events if e.kind=='content'),'Visible answer.')
|
||||
self.assertFalse(any(e.kind.startswith('tool') for e in events))
|
||||
|
||||
|
||||
def test_literal_call_markers_in_content_before_genuine_call(self):
|
||||
prefix = "Use `<tool_call>` in docs. Empty <tool_call><function=> stays text."
|
||||
for streaming in (False, True):
|
||||
for step in (1, 7, 4096):
|
||||
parser = OutputParser(thinking=False, tools=TOOLS, stream_tools=streaming)
|
||||
text = prefix + "\n" + CALL
|
||||
events = []
|
||||
for i in range(0, len(text), step):
|
||||
events += parser.feed(text[i:i+step])
|
||||
events += parser.finish()
|
||||
self.assertEqual(''.join(e.text for e in events if e.kind == 'content'), prefix)
|
||||
calls = [e.call for e in events if e.kind == 'tool_call']
|
||||
self.assertEqual(len(calls), 1)
|
||||
self.assertEqual(calls[0].arguments, {'path': '123'})
|
||||
|
||||
|
||||
def test_namespace_alias_schema_and_exact_name_precedence(self):
|
||||
for streaming in (False, True):
|
||||
parser = OutputParser(thinking=False, tools=TOOLS, stream_tools=streaming)
|
||||
events = parser.feed(CALL) + parser.finish()
|
||||
call = next(e.call for e in events if e.kind == "tool_call")
|
||||
self.assertEqual((call.name,call.arguments), ("mcp__files.read", {"path": "123"}))
|
||||
exact = {"name": "mcp__files__read", "parameters": {"properties": {"path": {"type": "integer"}}}}
|
||||
parser = OutputParser(thinking=False, tools=[*TOOLS,exact], stream_tools=streaming)
|
||||
call = next(e.call for e in parser.feed(CALL)+parser.finish() if e.kind == "tool_call")
|
||||
self.assertEqual((call.name,call.arguments), ("mcp__files__read", {"path": 123}))
|
||||
|
||||
ambiguous = [{"name": "a.b__c"}, {"name": "a__b.c"}]
|
||||
parser = OutputParser(thinking=False, tools=ambiguous)
|
||||
text = CALL.replace("mcp__files__read", "a__b__c")
|
||||
events = parser.feed(text) + parser.finish()
|
||||
# an ambiguous alias is no declared tool: it does not route to the first namespace; it stays text
|
||||
self.assertFalse(any(e.kind == "tool_call" for e in events))
|
||||
self.assertEqual("".join(e.text for e in events if e.kind == "content"), text)
|
||||
self.assertEqual(parser.unread, 1) # and it is counted, so the server logs it
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -517,6 +517,22 @@ class ToolRoundTrip(Server):
|
||||
self.assertIn("<tool_response>\nbefore\n</tool_response>", second_prompt)
|
||||
self.assertEqual(r["output"][-1]["content"][0]["text"], "The file says before.")
|
||||
|
||||
def test_quoted_incomplete_wrapper_is_not_a_completed_call(self):
|
||||
text = "Format: " + CALL[CALL.index("<tool_call>"):].removesuffix("</tool_call>") + "and then I explain it."
|
||||
self.engine.scripts = [self.tok.encode("</think>\n\n" + text + "<|im_end|>", parse_special=True)]
|
||||
self.engine.script = self.engine.scripts[0]
|
||||
for stream in (False, True):
|
||||
code, result = self.post({"model": "m", "input": "explain the format", "tools": TOOLS, "stream": stream})
|
||||
self.assertEqual(code, 200, result)
|
||||
final = result[-1]["response"] if stream else result
|
||||
calls = [item for item in final["output"] if item["type"] == "function_call"]
|
||||
self.assertFalse(any(item["status"] == "completed" for item in calls))
|
||||
for call in calls:
|
||||
with self.assertRaises(ValueError):
|
||||
json.loads(call["arguments"])
|
||||
self.assertEqual("".join(item["content"][0]["text"] for item in final["output"]
|
||||
if item["type"] == "message"), text)
|
||||
|
||||
def test_namespace_call_written_with_double_underscore(self):
|
||||
# Qwen writes Codex's MCP tools in their flat `mcp__server__tool` form; Codex only
|
||||
# runs the call when it comes back with its namespace.
|
||||
@@ -533,6 +549,36 @@ class ToolRoundTrip(Server):
|
||||
self.assertEqual((call["namespace"], call["name"], json.loads(call["arguments"])),
|
||||
("mcp__websearch", "web_search", {"query": "strata"}))
|
||||
|
||||
# Two canonical names share this alias. It must not invoke the first namespace.
|
||||
self.engine.scripts = [self.tok.encode(
|
||||
"</think>\n\n<tool_call>\n<function=a__b__c>\n</function>\n</tool_call><|im_end|>",
|
||||
parse_special=True)]
|
||||
self.engine.script = self.engine.scripts[0]
|
||||
tools = [{"type": "namespace", "name": "a", "tools": [{"type": "function", "name": "b__c"}]},
|
||||
{"type": "namespace", "name": "a__b", "tools": [{"type": "function", "name": "c"}]}]
|
||||
for stream in (False, True):
|
||||
code, r = self.post({"model": "m", "input": "go", "tools": tools, "stream": stream})
|
||||
self.assertEqual(code, 200, r)
|
||||
output = r[-1]["response"]["output"] if stream else r["output"]
|
||||
# not an alias of either: with tools declared (the 0.2.2 rule, #3075) a call for an undeclared name stays text
|
||||
self.assertFalse([item for item in output if item.get("type") == "function_call"], output)
|
||||
self.assertIn("a__b__c", json.dumps(output))
|
||||
|
||||
# A declared exact name wins over alias spellings and keeps its string schema.
|
||||
tools.append({"type": "function", "name": "a__b__c", "parameters": {
|
||||
"type": "object", "properties": {"path": {"type": "string"}}}})
|
||||
self.engine.scripts = [self.tok.encode(
|
||||
"</think>\n\n<tool_call>\n<function=a__b__c>\n<parameter=path>\n123\n</parameter>\n"
|
||||
"</function>\n</tool_call><|im_end|>", parse_special=True)]
|
||||
self.engine.script = self.engine.scripts[0]
|
||||
for stream in (False, True):
|
||||
code, r = self.post({"model": "m", "input": "go", "tools": tools, "stream": stream})
|
||||
self.assertEqual(code, 200, r)
|
||||
output = r[-1]["response"]["output"] if stream else r["output"]
|
||||
call, = output
|
||||
self.assertNotIn("namespace", call)
|
||||
self.assertEqual((call["name"], json.loads(call["arguments"])), ("a__b__c", {"path": "123"}))
|
||||
|
||||
def test_namespace_and_custom_calls_come_back_under_their_own_names(self):
|
||||
self.engine.scripts = [self.tok.encode(
|
||||
"</think>\n\n<tool_call>\n<function=multi_agent_v1.spawn_agent>\n<parameter=message>\nhi\n</parameter>\n"
|
||||
|
||||
+26
-6
@@ -1187,9 +1187,10 @@ class ToolCallRecovery(unittest.TestCase):
|
||||
|
||||
def test_bare_function_without_the_wrapper(self):
|
||||
# corpus 395efd4c / 6e71eed4 (x5 each): <function=...> with no <tool_call> around it
|
||||
self.check(f"Writing it now.\n\n<function=write>\n{self.PARAMS}", [("write", self.CALL)], "Writing it now.")
|
||||
self.check(f"<function=write>\n{self.PARAMS}\n</tool_call>", [("write", self.CALL)], "") # stray closer
|
||||
self.check(f"<function=write>\n{self.PARAMS}\nDone.", [("write", self.CALL)], "Done.")
|
||||
for name in ("write", " write ", "\twrite\t"):
|
||||
self.check(f"Writing it now.\n\n<function={name}>\n{self.PARAMS}", [("write", self.CALL)], "Writing it now.")
|
||||
self.check(f"<function={name}>\n{self.PARAMS}\n</tool_call>", [("write", self.CALL)], "") # stray closer
|
||||
self.check(f"<function={name}>\n{self.PARAMS}\nDone.", [("write", self.CALL)], "Done.")
|
||||
|
||||
def test_two_calls_in_one_wrapper(self):
|
||||
# corpus 0912870b / 51b155c9: a batch inside one <tool_call> - the second call's parameters were merged into
|
||||
@@ -1208,6 +1209,9 @@ class ToolCallRecovery(unittest.TestCase):
|
||||
self.check(inline, [], inline)
|
||||
unknown = "<function=format_disk>\n<parameter=x>\n1\n</parameter>\n</function>"
|
||||
self.check(unknown, [], unknown)
|
||||
for text in (fenced, inline, unknown):
|
||||
padded = text.replace("<function=write>", "<function= write >").replace("<function=format_disk>", "<function= format_disk >")
|
||||
self.check(padded, [], padded)
|
||||
|
||||
|
||||
class ToolCallRecoveryOff(unittest.TestCase):
|
||||
@@ -1329,12 +1333,28 @@ class UnfinishedToolCall(unittest.TestCase):
|
||||
("anthropic", False): ("end_turn", []), # call out: it has no arguments to give
|
||||
("anthropic", True): ("end_turn", cut)})
|
||||
|
||||
def test_quoted_incomplete_wrapper_is_not_a_call(self):
|
||||
text = "Format: " + self.WHOLE[len("</think>\n\n"):].removesuffix("</tool_call>") + "and then I explain it."
|
||||
answers = self.answers("</think>\n\n" + text)
|
||||
for api, stop in (("openai", "stop"), ("anthropic", "end_turn")):
|
||||
self.assertEqual(answers[api, False], (stop, []))
|
||||
self.assertEqual(answers[api, True][0], stop)
|
||||
with self.assertRaises(ValueError):
|
||||
json.loads(answers[api, True][1])
|
||||
|
||||
def test_a_call_cut_at_the_token_limit(self):
|
||||
"""The same cut by max_tokens: "length" / "max_tokens", and the whole (non-streamed) answers leave the call
|
||||
out in both APIs."""
|
||||
a = self.answers(self.CUT + "rest of the file, never reached" * 40, max_tokens=len(self.CUT))
|
||||
self.assertEqual((a["openai", False], a["anthropic", False]), (("length", []), ("max_tokens", [])))
|
||||
self.assertEqual((a["openai", True][0], a["anthropic", True][0]), ("length", "max_tokens"))
|
||||
cases = ((self.CUT + "rest of the file, never reached" * 40, len(self.CUT)),
|
||||
(self.WHOLE, len(self.WHOLE) - len("</tool_call>")))
|
||||
for script, limit in cases:
|
||||
with self.subTest(limit=limit):
|
||||
a = self.answers(script, max_tokens=limit)
|
||||
self.assertEqual((a["openai", False], a["anthropic", False]), (("length", []), ("max_tokens", [])))
|
||||
self.assertEqual((a["openai", True][0], a["anthropic", True][0]), ("length", "max_tokens"))
|
||||
for api in ("openai", "anthropic"):
|
||||
with self.assertRaises(ValueError):
|
||||
json.loads(a[api, True][1])
|
||||
|
||||
def test_collect_keeps_calls_whose_arguments_parse(self):
|
||||
from serve.server import openai_collect
|
||||
|
||||
@@ -91,6 +91,14 @@ class OnlyDeclaredTools(unittest.TestCase):
|
||||
self.assertFalse([e for e in evs if e.kind in ("tool_start", "tool_args")])
|
||||
self.assertEqual(text(evs), "I will check.\n" + raw)
|
||||
|
||||
def test_an_unparsed_call_is_counted_and_named_for_the_log(self):
|
||||
"""T1 rail: a call that went back as text is no longer silent; the server prints its count and the names the model wrote."""
|
||||
raw = body("<function=get_indicators>", "symbol", "600076")
|
||||
evs, p = run("I will check."+chr(10) + raw, 4, tools=TOOLS)
|
||||
self.assertEqual((p.unread, p.unread_names), (1, ["get_indicators"]))
|
||||
evs, p = run(body("<function=shell>", "cmd", "ls"), 4, tools=TOOLS) # a declared call is no unread call
|
||||
self.assertEqual((p.unread, p.unread_names), (0, []))
|
||||
|
||||
def test_a_declared_call_after_an_undeclared_one_is_still_a_call(self):
|
||||
raw = body("<function=get_indicators>") + "\n" + body("<function=shell>", "cmd", "ls")
|
||||
evs, p = run(raw, 5, tools=TOOLS)
|
||||
@@ -99,7 +107,8 @@ class OnlyDeclaredTools(unittest.TestCase):
|
||||
|
||||
def test_the_responses_spelling_of_a_namespace_tool_counts(self):
|
||||
evs, p = run(body("<function=ns__web_search>", "q", "x"), 4, tools=TOOLS)
|
||||
self.assertEqual(calls(evs), [("ns__web_search", {"q": "x"})])
|
||||
# PR #3556: the call carries the declared spelling (ns.web_search), the one the client offered, not the model's flat one
|
||||
self.assertEqual(calls(evs), [("ns.web_search", {"q": "x"})])
|
||||
|
||||
def test_no_tools_declared(self):
|
||||
raw = body("<function=search>", "q", "2+2")
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
"""A missing wrapper closer ends before a following call, but prose alone does not confirm a call.
|
||||
|
||||
A call's own </function> separates it from the next wrapper without merging their arguments.
|
||||
Parameter values may still contain call-format tags; recovery batches stay opt-in.
|
||||
|
||||
python -m unittest serve.test_tool_call_unclosed -v
|
||||
"""
|
||||
import json
|
||||
import sys
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
|
||||
|
||||
from serve.frontend import OutputParser # noqa: E402
|
||||
|
||||
TOOLS = [{"name": "read", "parameters": {"properties": {"path": {"type": "string"}}}},
|
||||
{"name": "write", "parameters": {"properties": {"path": {"type": "string"}, "content": {"type": "string"}}}}]
|
||||
READ = "<function=read>\n<parameter=path>\na.py\n</parameter>\n</function>"
|
||||
WRITE = "<function=write>\n<parameter=path>\nb.py\n</parameter>\n<parameter=content>\nx = 1\n</parameter>\n</function>"
|
||||
|
||||
|
||||
def run(text, stream_tools, recover, step):
|
||||
p = OutputParser(thinking=False, tools=TOOLS, stream_tools=stream_tools, recover=recover)
|
||||
evs = []
|
||||
for i in range(0, len(text), step): # fed in small pieces, as a stream arrives
|
||||
evs += p.feed(text[i:i + step])
|
||||
evs += p.finish("stop")
|
||||
calls = [(e.call.name, e.call.arguments) for e in evs if e.kind == "tool_call"]
|
||||
if stream_tools and not calls:
|
||||
arguments = "".join(e.text for e in evs if e.kind == "tool_args")
|
||||
if arguments:
|
||||
try:
|
||||
json.loads(arguments)
|
||||
except ValueError:
|
||||
pass
|
||||
else:
|
||||
raise AssertionError("an unconfirmed call has complete streamed arguments")
|
||||
return calls, "".join(e.text for e in evs if e.kind == "content")
|
||||
|
||||
|
||||
class UnclosedCall(unittest.TestCase):
|
||||
def check(self, text, want_calls, want_content=""):
|
||||
for stream_tools in (False, True):
|
||||
for recover in (False, True):
|
||||
for step in (1, 3, 7, 4096):
|
||||
with self.subTest(stream_tools=stream_tools, recover=recover, step=step):
|
||||
calls, content = run(text, stream_tools, recover, step)
|
||||
self.assertEqual(calls, want_calls)
|
||||
self.assertEqual(content.strip(), want_content)
|
||||
|
||||
def test_closed_calls_are_unchanged(self):
|
||||
self.check("<tool_call>\n" + READ + "\n</tool_call>\n<tool_call>\n" + WRITE + "\n</tool_call>",
|
||||
[("read", {"path": "a.py"}), ("write", {"path": "b.py", "content": "x = 1"})])
|
||||
|
||||
def test_a_second_call_after_an_unclosed_one_is_its_own_call(self):
|
||||
self.check("<tool_call>\n" + READ + "\n<tool_call>\n" + WRITE + "\n</tool_call>",
|
||||
[("read", {"path": "a.py"}), ("write", {"path": "b.py", "content": "x = 1"})])
|
||||
|
||||
def test_quoted_incomplete_wrapper_stays_literal(self):
|
||||
text = "Format: <tool_call>\n" + READ + "\nand then I explain it."
|
||||
self.check(text, [], text)
|
||||
|
||||
def test_following_literal_marker_does_not_complete_an_unclosed_call(self):
|
||||
text = "<tool_call>\n" + READ + "\n<tool_call> is a format marker."
|
||||
self.check(text, [], text)
|
||||
|
||||
def test_omitted_wrapper_recovers_only_on_a_natural_stop(self):
|
||||
for closed in (True, False):
|
||||
text = "<tool_call>\n" + READ + ("\n</tool_call>" if closed else "")
|
||||
for reason in (None, "stop", "length", "cancel", "disconnect"):
|
||||
for step in (1, 7, 4096):
|
||||
with self.subTest(closed=closed, reason=reason, step=step):
|
||||
p = OutputParser(thinking=False, tools=TOOLS, stream_tools=True)
|
||||
events = []
|
||||
for offset in range(0, len(text), step):
|
||||
events += p.feed(text[offset:offset + step])
|
||||
events += p.finish(reason)
|
||||
calls = [e for e in events if e.kind == "tool_call"]
|
||||
complete = closed or reason in (None, "stop")
|
||||
self.assertEqual(len(calls), int(complete))
|
||||
if not complete:
|
||||
with self.assertRaises(ValueError):
|
||||
json.loads("".join(e.text for e in events if e.kind == "tool_args"))
|
||||
|
||||
def test_a_value_that_holds_the_tags_still_ends_at_the_calls_own_closer(self):
|
||||
value = "a </function> and a <tool_call> in a string"
|
||||
call = ("<tool_call>\n<function=write>\n<parameter=path>\nc.py\n</parameter>\n<parameter=content>\n" + value +
|
||||
"\n</parameter>\n</function>\n</tool_call>")
|
||||
self.check(call, [("write", {"path": "c.py", "content": value})])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user