mirror of
https://github.com/Niko1221/Strata.git
synced 2026-11-07 06:15:58 -10:00
serve: a call returned as text for an undeclared name is named in the reply; the collapsed mcp__a__b spelling is the declared tool (#3796)
Problem: a request declares tools; the model writes a complete call for a name that is not among them (bash for shell, a dropped prefix, an MCP name with every "__" written as "_"). Since #3075 such a call stays text, which is the rule we keep, but the client saw an ordinary answer with finish_reason stop and nothing else, so the agent turn ended with raw XML and the model never learned of its mistake. Reporters on OpenCode, Pithagoras, OpenClaw, LibreChat, Open WebUI and Pi hit it. What changed (declared-only stays: nothing undeclared is ever returned as a call, finish_reason is unchanged): - serve/frontend.py OutputParser: a declared name that contains "__" is also read when written with every "__" as "_" (mcp_playwright_browser_navigate for mcp__playwright__browser_navigate); the call goes to the client under the declared name. Same rules as the other aliases: only when unambiguous, and a tool declared under that spelling wins. A reordered or renamed tool (server1_label_values for label_values_mcp_server1, bash for shell) is not guessed. - serve/server.py: when a block went back as text for an undeclared name, the reply carries "strata_rejected_tool_calls": [{"name": ..., "reason": "undeclared"}] (the names the parser already collected, up to five): at the top level of a Chat Completions reply and in its last streamed chunk, and in the message_delta event and the message of /v1/messages. It is absent when nothing was rejected, so every other reply is unchanged. The text itself is untouched. - The server window line says the response carries the names; the count goes to totals["tool_calls_unread"] and to /metrics/prometheus as strata:tool_calls_unread_total. - docs/DETAILS.md: the alias, the marker, the counter. Tests: serve/test_tool_name_alias.py (collapsed spelling at several chunk widths, a declared spelling and a plain tool unchanged, a tool declared in the collapsed spelling wins, two tools with one collapsed spelling are not guessed, reordered/renamed names stay text and are named), serve/test_server.py ForcedToolChoice (the field in Chat Completions whole and streamed and in /v1/messages, absent for an ordinary answer and a declared call, the collapsed name is a call; the counter). The whole serve/ suite: 1055 tests, two failures that also fail on the base tree (test_fatal_recovery on this PC, RestartWindow.test_not_alive_until_ready under load). Not covered: returning an undeclared call as a real tool call (the owner keeps declared-only); a block for a DECLARED name that came back as text because it was malformed (counted, no name to list); the /v1/messages field with a real client; a stderr or log-file copy of the warning.
This commit is contained in:
1 parent
4a759f82d4
commit
805ebe17be
5 files changed
+174
-8
No files matched your search
+13
-2
@@ -483,9 +483,20 @@ applies at the next start. Network, key, MCP and program keys are not editable t
|
||||
every 15 s and `GET /status` says what it is doing. Closing the connection or pressing stop in your app stops the model.
|
||||
- A tool call counts only when it names a tool the request declared. A namespaced tool (`mcp__filesystem.write_file`) is
|
||||
that call when the model writes `mcp__filesystem__write_file`, `mcp__filesystem=write_file`, `mcp__filesystem_write_file`,
|
||||
or the bare member (`write_file`) and exactly one declared tool has that member. Any other call, or any call in a request
|
||||
that declared no tools, comes back as the text it was written as. A call whose envelope drifted by a token (`<=NAME>`,
|
||||
or the bare member (`write_file`) and exactly one declared tool has that member. A name with `__` in it (`mcp__playwright__browser_navigate`)
|
||||
is also that call when the model writes every `__` as `_` (`mcp_playwright_browser_navigate`), unless two declared tools collapse to the same
|
||||
spelling. Any other call, or any call in a request that declared no tools, comes back as the text it was written as. Names are never
|
||||
guessed: `bash` is not `shell`, and a reordered name (`server1_label_values` for `label_values_mcp_server1`) is not the declared one.
|
||||
A call whose envelope drifted by a token (`<=NAME>`,
|
||||
`<function:NAME>` or `<NAME>` instead of `<function=NAME>`) is read as the call when NAME is a declared tool.
|
||||
- **A call that came back as text because its name was not declared (#3796).** The text stays exactly as the model wrote it and
|
||||
`finish_reason` stays `stop` (or `end_turn` on `/v1/messages`). Next to it the reply carries a list, only when there is something in it:
|
||||
`"strata_rejected_tool_calls": [{"name": "bash", "reason": "undeclared"}]` (at the top level of a Chat Completions reply, in
|
||||
its last streamed chunk, and in the `message_delta` event and the message of `/v1/messages`; up to five names). A client that
|
||||
wants to recover can answer the model with the tool names it does have and ask again; Strata never runs or forwards a call for a
|
||||
tool the request did not declare. The server window prints the same names (`N <tool_call> block(s) ... went back as text`), and
|
||||
`/metrics/prometheus` counts the blocks in `strata:tool_calls_unread_total`. A block that is malformed (not an undeclared name) is counted
|
||||
there and in the window, but has no name to list.
|
||||
- JSON output: `response_format: {"type":"json_object"}` or `{"type":"json_schema", ...}` on `/v1/chat/completions`. The schema
|
||||
must accept only JSON objects at its root (`"type":"object"`, or `anyOf`/`oneOf` of object schemas). Local `#` references
|
||||
work and remote ones are refused. This is schema prompting followed by server validation, not grammar-constrained
|
||||
|
||||
@@ -935,6 +935,12 @@ class OutputParser:
|
||||
for alias in (f"{namespace}__{member}", f"{namespace}={member}",
|
||||
f"{namespace}_{member}", member):
|
||||
aliases.setdefault(alias, set()).add(name)
|
||||
# #3796: the same tool written with every `__` collapsed to `_` (`mcp__playwright__browser_navigate` as
|
||||
# `mcp_playwright_browser_navigate`, the way Qwen often flattens a name) is that tool. Only a spelling of a declared
|
||||
# name: a reordered or renamed tool (`server1_label_values` for `label_values_mcp_server1`) is not guessed.
|
||||
for name in self.schemas:
|
||||
if isinstance(name, str) and "__" in name:
|
||||
aliases.setdefault(name.replace("__", "_"), 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
|
||||
|
||||
+23
-6
@@ -4680,6 +4680,8 @@ class Service:
|
||||
("strata:requests_total", "counter", "Requests finished", t["requests"]),
|
||||
("strata:drafts_offered_total", "counter", "Speculative draft tokens offered", t["drafts_offered"]),
|
||||
("strata:drafts_accepted_total", "counter", "Speculative draft tokens accepted", t["drafts_accepted"]),
|
||||
("strata:tool_calls_unread_total", "counter", "<tool_call> blocks returned as text (an undeclared name or a form the template does not use)",
|
||||
t.get("tool_calls_unread", 0)),
|
||||
("strata:model_loaded", "gauge", "1 while the model is loaded", int(self.loaded())),
|
||||
]
|
||||
return "".join(f"# HELP {name} {help_}\n# TYPE {name} {kind}\n{name} {value}\n"
|
||||
@@ -5596,11 +5598,17 @@ class Service:
|
||||
self.totals["tool_calls_from_reasoning"] = self.totals.get("tool_calls_from_reasoning", 0) + parser.rescued
|
||||
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)
|
||||
rejected = []
|
||||
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)" + (f"; the names the model wrote: {', '.join(named)}" if named else ""), flush=True)
|
||||
"not use)" + (f"; the names the model wrote: {', '.join(named)}" if named else "")
|
||||
+ "; the response carries them as strata_rejected_tool_calls, /metrics counts them", flush=True)
|
||||
with self.status_lock:
|
||||
self.totals["tool_calls_unread"] = self.totals.get("tool_calls_unread", 0) + parser.unread
|
||||
# #3796: the same names for the client (a machine-readable marker next to the text, which stays as the model wrote it)
|
||||
rejected = [{"name": nm, "reason": "undeclared"} for nm in named or []]
|
||||
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),
|
||||
@@ -5608,6 +5616,8 @@ class Service:
|
||||
"reasoning_recoveries": recovery_count}
|
||||
if stops is not None and stops.hit is not None:
|
||||
done["stop_sequence"] = stops.hit
|
||||
if rejected:
|
||||
done["rejected_tool_calls"] = rejected
|
||||
yield "done", done
|
||||
|
||||
|
||||
@@ -5892,6 +5902,8 @@ def openai_chunks(svc: Service, req: dict, ids, thinking, tools, max_new, cancel
|
||||
"prompt_tokens_details": {"cached_tokens": x.get("reused") or 0}}
|
||||
if x.get("timings"):
|
||||
last["timings"] = x["timings"] # llama.cpp's field: the speed its clients show
|
||||
if x.get("rejected_tool_calls"):
|
||||
last["strata_rejected_tool_calls"] = x["rejected_tool_calls"] # #3796: only when a call was refused as undeclared
|
||||
yield last
|
||||
|
||||
|
||||
@@ -5938,6 +5950,8 @@ def openai_collect(chunks) -> dict:
|
||||
"usage": last["usage"]}
|
||||
if last.get("timings"):
|
||||
out["timings"] = last["timings"]
|
||||
if last.get("strata_rejected_tool_calls"):
|
||||
out["strata_rejected_tool_calls"] = last["strata_rejected_tool_calls"]
|
||||
return out
|
||||
|
||||
|
||||
@@ -6029,11 +6043,12 @@ def anthropic_events(svc: Service, req: dict, ids, thinking, tools, max_new, can
|
||||
# request, a cache write; input_tokens keeps the last token, as 0 makes Claude Code count the prompt twice (#1013)
|
||||
reused = max(0, min(x.get("reused") or 0, len(ids) - 1))
|
||||
read_now = len(ids) - reused
|
||||
yield "message_delta", {"type": "message_delta", "delta": {"stop_reason": stop,
|
||||
"stop_sequence": x.get("stop_sequence")},
|
||||
"usage": {"input_tokens": 1, "cache_creation_input_tokens": read_now - 1,
|
||||
"cache_read_input_tokens": reused,
|
||||
"output_tokens": x["completion_tokens"]}}
|
||||
delta_event = {"type": "message_delta", "delta": {"stop_reason": stop, "stop_sequence": x.get("stop_sequence")},
|
||||
"usage": {"input_tokens": 1, "cache_creation_input_tokens": read_now - 1,
|
||||
"cache_read_input_tokens": reused, "output_tokens": x["completion_tokens"]}}
|
||||
if x.get("rejected_tool_calls"):
|
||||
delta_event["strata_rejected_tool_calls"] = x["rejected_tool_calls"] # #3796
|
||||
yield "message_delta", delta_event
|
||||
yield "message_stop", {"type": "message_stop"}
|
||||
|
||||
|
||||
@@ -6065,6 +6080,8 @@ def anthropic_collect(events) -> dict:
|
||||
msg["stop_reason"] = e["delta"]["stop_reason"]
|
||||
msg["stop_sequence"] = e["delta"].get("stop_sequence")
|
||||
msg["usage"].update(e["usage"])
|
||||
if e.get("strata_rejected_tool_calls"):
|
||||
msg["strata_rejected_tool_calls"] = e["strata_rejected_tool_calls"]
|
||||
for i in range(len(blocks) - 1): # a turn with no </think> (serve/dsml.py): the text is the answer; it is not also a thinking block
|
||||
a, b = blocks[i], blocks[i + 1]
|
||||
if a.get("type") == "thinking" and b.get("type") == "text" and a.get("thinking", "").strip() == b.get("text", "").strip():
|
||||
|
||||
@@ -4521,6 +4521,74 @@ class ForcedToolChoice(unittest.TestCase):
|
||||
self.assertIn("Here is the report.", content)
|
||||
self.assertIn("<function=lookup>", content)
|
||||
|
||||
def _undeclared_call_answer(self, name="lookup"):
|
||||
from serve.frontend import CALL_END, CALL_START
|
||||
return ("I will check that.\n" + CALL_START + f"\n<function={name}>\n<parameter=q>\n2+2\n</parameter>\n"
|
||||
"</function>\n" + CALL_END)
|
||||
|
||||
def _rejected_of(self, b, stream):
|
||||
"""The strata_rejected_tool_calls of a whole reply or of a streamed one (None when the reply has none)."""
|
||||
if not stream:
|
||||
return b.get("strata_rejected_tool_calls")
|
||||
found = [json.loads(line[6:]).get("strata_rejected_tool_calls") for line in b.splitlines() if line.startswith("data: {")]
|
||||
found = [f for f in found if f]
|
||||
self.assertLessEqual(len(found), 1, "sent once, with the last chunk")
|
||||
return found[0] if found else None
|
||||
|
||||
def test_an_undeclared_call_is_named_in_the_response(self):
|
||||
"""#3796: the call stays text and finish_reason stays "stop"; the response also lists the names the model wrote
|
||||
(strata_rejected_tool_calls), so a client can tell this from an ordinary answer and answer the model."""
|
||||
self.says(self._undeclared_call_answer("execute_gdscript"))
|
||||
for stream in (False, True):
|
||||
with self.subTest(stream=stream):
|
||||
before = self.svc.totals.get("tool_calls_unread", 0)
|
||||
code, b = self.openai(stream=stream)
|
||||
finish, calls, _ = self.call_of(code, b, stream)
|
||||
self.assertEqual((finish, calls), ("stop", []), b)
|
||||
self.assertEqual(self._rejected_of(b, stream), [{"name": "execute_gdscript", "reason": "undeclared"}])
|
||||
self.assertEqual(self.svc.totals["tool_calls_unread"], before + 1) # /metrics counts it
|
||||
if not stream:
|
||||
self.assertIn("<function=execute_gdscript>", b["choices"][0]["message"]["content"]) # the text is untouched
|
||||
|
||||
def test_no_marker_when_nothing_was_rejected(self):
|
||||
"""An ordinary answer and a declared call carry no extra field: the API is what it was."""
|
||||
from serve.frontend import CALL_END, CALL_START
|
||||
for label, answer in (("text", "The answer is 4."),
|
||||
("declared call", CALL_START + "\n<function=search>\n<parameter=q>\n2+2\n</parameter>\n</function>\n" + CALL_END)):
|
||||
self.says(answer)
|
||||
for stream in (False, True):
|
||||
with self.subTest(case=label, stream=stream):
|
||||
code, b = self.openai(stream=stream)
|
||||
self.call_of(code, b, stream)
|
||||
self.assertIsNone(self._rejected_of(b, stream))
|
||||
if stream:
|
||||
self.assertNotIn("strata_rejected_tool_calls", b)
|
||||
|
||||
def test_a_collapsed_mcp_name_is_a_call_not_a_rejection(self):
|
||||
"""#3796: `mcp_playwright_browser_navigate` for the declared `mcp__playwright__browser_navigate`."""
|
||||
from serve.frontend import CALL_END, CALL_START
|
||||
tools = [{"type": "function", "function": {"name": "mcp__playwright__browser_navigate", "description": "open a page",
|
||||
"parameters": {"type": "object", "properties": {"q": {"type": "string"}}}}}]
|
||||
self.says(CALL_START + "\n<function=mcp_playwright_browser_navigate>\n<parameter=q>\nx\n</parameter>\n</function>\n" + CALL_END)
|
||||
for stream in (False, True):
|
||||
with self.subTest(stream=stream):
|
||||
code, b = self.openai(stream=stream, tools=tools)
|
||||
finish, calls, _ = self.call_of(code, b, stream)
|
||||
self.assertEqual((finish, calls), ("tool_calls", [("mcp__playwright__browser_navigate", {"q": "x"})]))
|
||||
self.assertIsNone(self._rejected_of(b, stream))
|
||||
|
||||
def test_the_anthropic_reply_names_the_rejected_call_too(self):
|
||||
self.says(self._undeclared_call_answer("bash"))
|
||||
body = {"model": "m", "max_tokens": 400, "messages": [{"role": "user", "content": "2+2?"}],
|
||||
"tools": [{"name": "search", "description": "search", "input_schema": {"type": "object", "properties": {}}}]}
|
||||
req = urllib.request.Request(self.base + "/v1/messages", data=json.dumps(body).encode(),
|
||||
headers={"Content-Type": "application/json"})
|
||||
with urllib.request.urlopen(req, timeout=30) as r:
|
||||
msg = json.loads(r.read().decode())
|
||||
self.assertEqual(msg["stop_reason"], "end_turn")
|
||||
self.assertEqual(msg["strata_rejected_tool_calls"], [{"name": "bash", "reason": "undeclared"}])
|
||||
self.assertFalse([c for c in msg["content"] if c["type"] == "tool_use"])
|
||||
|
||||
def test_a_call_with_a_dropped_function_keyword_is_still_the_call(self):
|
||||
"""#3441: `<=search>` for `<function=search>`, as a model under a large tool surface writes it."""
|
||||
from serve.frontend import CALL_END, CALL_START
|
||||
|
||||
@@ -193,5 +193,69 @@ class DriftedMcpNames(unittest.TestCase):
|
||||
self.assertNotIn("namespace", calls[0])
|
||||
|
||||
|
||||
def plain_tool(name):
|
||||
return {"name": name, "parameters": {"type": "object", "properties": {"url": {"type": "string"}}}}
|
||||
|
||||
|
||||
def call_text(name, param="url", value="https://example.com"):
|
||||
return f"<tool_call>\n<function={name}>\n<parameter={param}>\n{value}\n</parameter>\n</function>\n</tool_call>"
|
||||
|
||||
|
||||
def run_parser(text, tools, width=0):
|
||||
parser = OutputParser(thinking=False, tools=tools, stream_tools=True, declared_only=True)
|
||||
evs = []
|
||||
step = width or len(text) or 1
|
||||
for k in range(0, len(text), step):
|
||||
evs += parser.feed(text[k:k + step])
|
||||
evs += parser.finish("stop")
|
||||
return evs, parser
|
||||
|
||||
|
||||
class CollapsedUnderscoreNames(unittest.TestCase):
|
||||
"""#3796: `mcp__server__tool` written as `mcp_server_tool` is a spelling of the declared tool (the same class of
|
||||
difference as namespace__member). A name that is only similar stays text, and is named for the client."""
|
||||
MCP = [plain_tool("mcp__playwright__browser_navigate"), plain_tool("mcp__playwright__browser_snapshot"),
|
||||
plain_tool("mcp__playwright__browser_tabs"), plain_tool("todo_write")]
|
||||
|
||||
def test_the_collapsed_spelling_is_the_declared_tool(self):
|
||||
for name in ("browser_navigate", "browser_snapshot", "browser_tabs"):
|
||||
for width in (0, 1, 5):
|
||||
with self.subTest(name=name, width=width):
|
||||
evs, parser = run_parser("Opening it.\n" + call_text("mcp_playwright_" + name), self.MCP, width)
|
||||
found = [e.call for e in evs if e.kind == "tool_call"]
|
||||
self.assertEqual([c.name for c in found], ["mcp__playwright__" + name]) # the client gets the name it declared
|
||||
self.assertEqual(found[0].arguments, {"url": "https://example.com"})
|
||||
self.assertEqual(text_of(evs).strip(), "Opening it.")
|
||||
self.assertEqual((parser.unread, parser.unread_names), (0, []))
|
||||
|
||||
def test_the_declared_spelling_and_a_plain_tool_are_unchanged(self):
|
||||
evs, _ = run_parser(call_text("mcp__playwright__browser_tabs") + "\n" + call_text("todo_write"), self.MCP)
|
||||
self.assertEqual([e.call.name for e in evs if e.kind == "tool_call"], ["mcp__playwright__browser_tabs", "todo_write"])
|
||||
|
||||
def test_a_tool_declared_in_the_collapsed_spelling_wins(self):
|
||||
tools = self.MCP + [plain_tool("mcp_playwright_browser_tabs")]
|
||||
evs, _ = run_parser(call_text("mcp_playwright_browser_tabs"), tools)
|
||||
self.assertEqual([e.call.name for e in evs if e.kind == "tool_call"], ["mcp_playwright_browser_tabs"])
|
||||
|
||||
def test_two_tools_with_the_same_collapsed_spelling_are_not_guessed(self):
|
||||
tools = [plain_tool("a__b_c"), plain_tool("a_b__c")]
|
||||
evs, parser = run_parser(call_text("a_b_c"), tools)
|
||||
self.assertEqual([e for e in evs if e.kind == "tool_call"], [])
|
||||
self.assertIn("a_b_c", text_of(evs))
|
||||
self.assertEqual(parser.unread_names, ["a_b_c"])
|
||||
|
||||
def test_a_reordered_or_renamed_tool_stays_text_and_is_named(self):
|
||||
"""The LibreChat case (`server1_label_values` for `label_values_mcp_server1`) and the bash/shell case: no guessing."""
|
||||
for declared, written in (("label_values_mcp_server1", "server1_label_values"), ("shell", "bash"),
|
||||
("godot_execute_gdscript", "execute_gdscript")):
|
||||
for width in (0, 3):
|
||||
with self.subTest(written=written, width=width):
|
||||
raw = "Done.\n" + call_text(written)
|
||||
evs, parser = run_parser(raw, [plain_tool(declared)], width)
|
||||
self.assertEqual([e for e in evs if e.kind == "tool_call"], [])
|
||||
self.assertEqual(text_of(evs), raw) # the text is exactly what the model wrote
|
||||
self.assertEqual((parser.unread, parser.unread_names), (1, [written]))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user