From 1ba1830c493cec21842c2872d3fe8bafbe4d5789 Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 17 Aug 2026 21:36:22 -0500 Subject: [PATCH 1/8] Support OpenAI tool calling (function calling) The server accepted `tools` in a request and silently discarded them: the field wasn't on ChatCompletionRequest, so pydantic dropped it and DeepSeek never learned the tools existed. Clients driving a tool loop got prose narration instead of tool calls -- typically ReAct-style 'Action: some_tool / Action Input: {...}' -- along with invented tool names and imagined results. DeepSeek's web chat has no native function-calling channel, so this emulates it: - schemas: accept `tools`/`tool_choice`, and `tool_calls`/`tool_call_id`/`name` on messages - prompt: render the tool schemas with a strict JSON output contract, and replay past assistant calls and tool results so multi-step loops keep their shape - parse replies back into OpenAI `tool_calls` (fenced JSON, bare JSON, OpenAI-shaped, or ReAct prose), with `finish_reason: tool_calls` - reject calls naming a tool that was never offered -- those are hallucinations, and returning them as text is the honest outcome - streaming buffers tool-enabled requests, since a call is only recognisable once its JSON object is complete; requests without `tools` stream unchanged - `DEBUG_REQUESTS=1` logs each request's roles and offered tool names Tests cover each reply shape, hallucinated-tool rejection, prompt replay, and the endpoint in both streaming and non-streaming modes; they fake the upstream client, so no DeepSeek session is required. --- README.md | 56 ++++++ server/api.py | 40 +++- server/openai_format.py | 361 ++++++++++++++++++++++++++++++++++--- server/schemas.py | 13 +- tests/test_tool_calling.py | 191 ++++++++++++++++++++ 5 files changed, 631 insertions(+), 30 deletions(-) create mode 100644 tests/test_tool_calling.py diff --git a/README.md b/README.md index 9e577a0..339d17b 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ You sign in once in a browser with your DeepSeek account; your session is saved - [Command line](#command-line) - [Human-check & proof-of-work (automatic)](#human-check--proof-of-work-automatic) - [Models, DeepThink & web search](#models-deepthink--web-search) +- [Tool calling (function calling)](#tool-calling-function-calling) - [Concurrency](#concurrency) - [Rate limiting](#rate-limiting) - [Project layout](#project-layout) @@ -220,6 +221,61 @@ on resume. Unknown model names return a `404` (no silent fallback). See --- +## Tool calling (function calling) + +The server accepts OpenAI-style `tools` and returns `tool_calls`, so agent +frameworks that drive a tool loop work against it: + +```python +resp = client.chat.completions.create( + model="deepseek-chat", + messages=[{"role": "user", "content": "What's the weather in Paris?"}], + tools=[{ + "type": "function", + "function": { + "name": "get_weather", + "description": "Current weather for a city", + "parameters": { + "type": "object", + "properties": {"city": {"type": "string"}}, + "required": ["city"], + }, + }, + }], +) +call = resp.choices[0].message.tool_calls[0] # finish_reason == "tool_calls" +print(call.function.name, call.function.arguments) +``` + +Send the result back as a `tool` message (with the matching `tool_call_id`) and +the conversation continues as it would against OpenAI. + +**This is emulated, not native.** DeepSeek's web chat has no function-calling +channel, so the bridge puts the tool schemas into the prompt with a strict +output contract and parses the model's JSON reply back into `tool_calls`. That +has consequences worth knowing: + +- **It's best-effort.** A prompt is not a decoder constraint. Complex or deeply + nested parameter schemas are where it frays first. +- **Hallucinated tools are dropped.** A call naming a tool you didn't offer is + returned as ordinary text, never as a `tool_call`. +- **Prose calls are salvaged.** Replies that narrate the call ReAct-style + (`Action: some_tool` / `Action Input: {...}`) are parsed into real calls + anyway, including when the model drops a namespace prefix. +- **`tool_choice`** supports `"none"`, `"required"`, and pinning a named + function. +- **Streaming buffers.** A tool call is only recognisable once its JSON is + complete, so tool-enabled streaming requests emit one delta instead of + token-by-token output. Requests without `tools` stream as before. + +Set `DEBUG_REQUESTS=1` to log each request's message roles and offered tool +names — the quickest way to tell whether a client actually sent `tools`. + +Tests: `python -m pytest tests/test_tool_calling.py` (no DeepSeek session +needed; the upstream client is faked). + +--- + ## Concurrency The server bridges a **single** signed-in DeepSeek account behind one shared diff --git a/server/api.py b/server/api.py index 471b258..7e47de5 100644 --- a/server/api.py +++ b/server/api.py @@ -21,6 +21,9 @@ from __future__ import annotations +import json +import logging +import os import threading import time @@ -39,12 +42,27 @@ is_known_model, resolve_model_type, ) -from .openai_format import completion_response, messages_to_prompt, stream_chunks +from .openai_format import ( + completion_response, + extract_tool_calls, + messages_to_prompt, + stream_chunks, + stream_chunks_with_tools, +) from .ratelimit import RateLimiter, install_rate_limit from .schemas import ChatCompletionRequest load_dotenv() +# uvicorn configures logging with disable_existing_loggers, so hang our +# debug output off its own logger to be sure it reaches the console. +log = logging.getLogger("uvicorn.error") +# Set DEBUG_REQUESTS=1 to log each request's shape (roles + tool names). Useful +# when a client's tool calls aren't arriving: it shows whether `tools` was sent +# at all, which is the difference between "bridge ignored them" and "client +# never offered them". +DEBUG_REQUESTS = os.getenv("DEBUG_REQUESTS", "").lower() in ("1", "true", "yes", "on") + app = FastAPI(title="DeepSeek OpenAI-compatible API", version="0.1.0") install_rate_limit(app, RateLimiter(limit=RATE_LIMIT_PER_MINUTE, window=60.0)) @@ -112,7 +130,16 @@ async def chat_completions(req: ChatCompletionRequest): # A thread's model is fixed when it's created, so on resume we ignore `model` # (the OpenAI SDK always sends one) and let the existing thread's model stand. model_type = None if req.conversation_id else resolve_model_type(req.model) - prompt = messages_to_prompt(req.messages) + if DEBUG_REQUESTS: + log.warning("request: model=%s stream=%s roles=%s tools=%s tool_choice=%s", + req.model, req.stream, [m.role for m in req.messages], + [ (t.get("function") or t).get("name") for t in (req.tools or []) ], + req.tool_choice) + + prompt = messages_to_prompt(req.messages, req.tools, req.tool_choice) + # DeepSeek has no native tool channel; `tools` are emulated in the prompt + # and parsed back out of the reply. `tool_choice: none` means don't offer. + tools = None if req.tool_choice == "none" else req.tools try: # Off the event loop: get_client() uses Playwright's sync API, which @@ -129,7 +156,10 @@ def gen(): prompt, conversation_id=req.conversation_id, model=model_type, thinking=req.thinking, search=req.search, ) - yield from stream_chunks(req.model, stream) + if tools: + yield from stream_chunks_with_tools(req.model, stream, tools) + else: + yield from stream_chunks(req.model, stream) return StreamingResponse(gen(), media_type="text/event-stream") @@ -141,4 +171,6 @@ def gen(): except Exception as e: return _error(f"DeepSeek request failed: {e}") - return completion_response(req.model, reply.text, prompt, reply.conversation_id) + calls, text = extract_tool_calls(reply.text, tools) + return completion_response(req.model, text, prompt, reply.conversation_id, + tool_calls=calls) diff --git a/server/openai_format.py b/server/openai_format.py index f0985cb..4dc5c1e 100644 --- a/server/openai_format.py +++ b/server/openai_format.py @@ -3,19 +3,55 @@ DeepSeek's protocol has no system/role channel — just a single `prompt` string. So we flatten the OpenAI `messages` array into one prompt, and wrap DeepSeek's text output back into OpenAI response/stream objects. + +Tool calling is EMULATED. The web chat has no native function-calling channel, +so when a request carries `tools` we (1) describe them in the prompt with a +strict output contract, and (2) parse the model's JSON reply back into OpenAI +`tool_calls`. Without this the model narrates its intent in prose ("Action: +some_tool ...") and the caller sees text where it expected a tool call. """ from __future__ import annotations import json +import re import time import uuid -from typing import Iterable, List +from typing import Iterable, List, Optional, Tuple from .schemas import ChatMessage _ROLE_LABELS = {"system": "System", "user": "User", "assistant": "Assistant"} +# The contract we ask the model to follow when tools are available. Kept blunt +# and example-led: the failure mode we're fixing is the model *describing* a +# call instead of emitting one. +_TOOL_PROTOCOL = """\ +# Tool calling + +You have access to the tools listed below. They are the ONLY tools that exist. + +To call one or more tools, reply with a single JSON object and NOTHING else — +no prose before it, no explanation after it, in exactly this shape: + +```json +{"tool_calls": [{"name": "", "arguments": {"": ""}}]} +``` + +Rules, all of them mandatory: +- Use ONLY tool names from the list. Never invent a tool that is not listed. +- `arguments` must be a JSON object matching that tool's parameter schema. +- NEVER write a call as prose. Lines like "Action: some_tool" or + "Action Input: {...}" are not tool calls and do nothing. +- NEVER write out, guess, or imagine a tool's result. Stop after the JSON; the + real result comes back to you in the next turn. +- When you are done using tools and want to answer the user, reply with normal + text and no JSON block. + +## Available tools + +""" + def _text_of(content) -> str: """Extract plain text from a message's content (string or list-of-parts).""" @@ -30,24 +66,251 @@ def _text_of(content) -> str: return "\n".join(parts) -def messages_to_prompt(messages: List[ChatMessage]) -> str: +def _function_of(tool: dict) -> dict: + """The function spec of a tool entry, tolerating the flat (pre-`tools`) shape.""" + if isinstance(tool, dict) and isinstance(tool.get("function"), dict): + return tool["function"] + return tool if isinstance(tool, dict) else {} + + +def tool_names(tools) -> List[str]: + names = [] + for t in tools or []: + n = _function_of(t).get("name") + if n: + names.append(n) + return names + + +def tools_preamble(tools, tool_choice=None) -> str: + """Render the tool schemas plus the output contract as prompt text.""" + blocks = [] + for t in tools or []: + fn = _function_of(t) + name = fn.get("name") + if not name: + continue + spec = { + "name": name, + "description": fn.get("description", ""), + "parameters": fn.get("parameters", {"type": "object", "properties": {}}), + } + blocks.append(json.dumps(spec, ensure_ascii=False)) + if not blocks: + return "" + + text = _TOOL_PROTOCOL + "\n\n".join(blocks) + + # tool_choice steers whether a call is optional, mandatory, or pinned. + if isinstance(tool_choice, dict): + forced = _function_of(tool_choice).get("name") + if forced: + text += (f"\n\nFor this turn you MUST call `{forced}` and no other tool. " + "Reply with the JSON object only.") + elif tool_choice == "required": + text += ("\n\nFor this turn you MUST call one of the tools above. " + "Reply with the JSON object only.") + elif tool_choice == "none": + text += ("\n\nFor this turn do NOT call any tool. Answer the user in " + "plain text.") + return text + + +def _render_assistant_tool_calls(calls) -> str: + """Replay a past assistant tool call in the same format we ask the model for.""" + rendered = [] + for c in calls or []: + fn = c.get("function", {}) if isinstance(c, dict) else {} + args = fn.get("arguments", "{}") + if isinstance(args, str): + try: + args = json.loads(args or "{}") + except (ValueError, TypeError): + args = {"_raw": args} + rendered.append({"name": fn.get("name") or c.get("name"), "arguments": args}) + body = json.dumps({"tool_calls": rendered}, ensure_ascii=False) + return f"```json\n{body}\n```" + + +def messages_to_prompt(messages: List[ChatMessage], tools=None, + tool_choice=None) -> str: """Flatten a chat history into a single prompt DeepSeek can answer. - A lone user message is sent verbatim. Multi-turn / system-prompted - conversations are serialised with role labels and a trailing 'Assistant:' - cue so the model continues in the right voice. + A lone user message with no tools is sent verbatim. Anything else — system + prompts, multi-turn, tool results — is serialised with role labels and a + trailing 'Assistant:' cue so the model continues in the right voice. """ - if len(messages) == 1 and messages[0].role == "user": + preamble = tools_preamble(tools, tool_choice) + + if len(messages) == 1 and messages[0].role == "user" and not preamble: return _text_of(messages[0].content) lines = [] + if preamble: + lines.append(preamble) + for m in messages: + if m.role == "tool": + # The answer to a call we made. Label it with the tool's name when + # the caller gave us one, so the model can match it to its request. + who = m.name or m.tool_call_id or "tool" + lines.append(f"Tool result ({who}): {_text_of(m.content)}") + continue + label = _ROLE_LABELS.get(m.role, m.role.capitalize()) - lines.append(f"{label}: {_text_of(m.content)}") + body = _text_of(m.content) + if m.role == "assistant" and m.tool_calls: + calls = _render_assistant_tool_calls(m.tool_calls) + body = f"{body}\n{calls}".strip() if body else calls + lines.append(f"{label}: {body}") + lines.append("Assistant:") return "\n\n".join(lines) +# --- parsing the model's reply back into tool calls ------------------------- + +_FENCE_RE = re.compile(r"```(?:json)?\s*(.+?)\s*```", re.DOTALL) + +# Salvage path: some replies narrate the call in ReAct prose instead of emitting +# JSON ("Action: some_tool Action Input: {...}"). The preamble tells the model +# not to, but a prompt is not a decoder constraint, so we parse it anyway rather +# than hand the caller a wall of text where a tool call belonged. +_REACT_RE = re.compile( + r"Action\s*:\s*[`\"']?([\w.\-]+)[`\"']?\s*[\r\n]*" + r"Action\s*Input\s*:\s*(\{.*?\})\s*(?:$|[\r\n])", + re.DOTALL | re.IGNORECASE, +) + + +def _resolve_name(name: str, allowed: List[str]) -> Optional[str]: + """Map a model-written tool name onto an offered one, or None. + + Exact match first, then a unique suffix match — models routinely drop a + namespace prefix (`mcp__server__do_thing` -> `do_thing`) or keep it when the + caller offered the short form. + """ + if name in allowed: + return name + hits = [a for a in allowed if a.endswith(name) or name.endswith(a)] + return hits[0] if len(hits) == 1 else None + + +def _extract_react_calls(text: str, allowed: List[str]) -> Tuple[Optional[List[dict]], str]: + """Parse ReAct-style `Action:` / `Action Input:` prose into tool calls.""" + calls, spans = [], [] + for m in _REACT_RE.finditer(text): + name = _resolve_name(m.group(1), allowed) + if not name: + continue + try: + args = json.loads(m.group(2)) + except (ValueError, TypeError): + continue + if not isinstance(args, dict): + continue + calls.append({ + "id": "call_" + uuid.uuid4().hex[:24], + "type": "function", + "function": {"name": name, "arguments": json.dumps(args, ensure_ascii=False)}, + }) + spans.append(m.span()) + if not calls: + return None, text + leftover = text + for start, end in reversed(spans): + leftover = leftover[:start] + leftover[end:] + return calls, leftover.strip() + + +def _iter_json_candidates(text: str) -> Iterable[str]: + """Yield substrings of `text` that might be the tool-call JSON object. + + Fenced blocks first (what we asked for), then any brace-balanced object in + the raw text (what we sometimes get). + """ + for m in _FENCE_RE.finditer(text): + yield m.group(1) + depth, start = 0, None + for i, ch in enumerate(text): + if ch == "{": + if depth == 0: + start = i + depth += 1 + elif ch == "}" and depth: + depth -= 1 + if depth == 0 and start is not None: + yield text[start:i + 1] + + +def _normalise_calls(obj, allowed: List[str]) -> Optional[List[dict]]: + """Pull a list of {name, arguments} out of a parsed JSON object, or None. + + Lenient about the wrapper (`tool_calls`, `tool_call`, or a bare call) because + the model is not a schema-constrained decoder. Strict about the names: a call + to a tool that wasn't offered is a hallucination, not a call. + """ + if isinstance(obj, dict): + raw = obj.get("tool_calls") or obj.get("tool_call") or obj + else: + raw = obj + if isinstance(raw, dict): + raw = [raw] + if not isinstance(raw, list): + return None + + calls = [] + for item in raw: + if not isinstance(item, dict): + return None + fn = item.get("function") if isinstance(item.get("function"), dict) else item + name = fn.get("name") + if not isinstance(name, str): + return None + name = _resolve_name(name, allowed) if allowed else name + if not name: + return None + args = fn.get("arguments", fn.get("parameters", {})) + if isinstance(args, str): + try: + args = json.loads(args or "{}") + except (ValueError, TypeError): + args = {} + if not isinstance(args, dict): + return None + calls.append({ + "id": "call_" + uuid.uuid4().hex[:24], + "type": "function", + "function": { + "name": name, + "arguments": json.dumps(args, ensure_ascii=False), + }, + }) + return calls or None + + +def extract_tool_calls(text: str, tools) -> Tuple[Optional[List[dict]], str]: + """Split a reply into (tool_calls, leftover_text). + + Returns (None, text) when the model answered in prose — the normal case for + a final answer. + """ + allowed = tool_names(tools) + if not allowed or not text: + return None, text + for candidate in _iter_json_candidates(text): + try: + obj = json.loads(candidate) + except (ValueError, TypeError): + continue + calls = _normalise_calls(obj, allowed) + if calls: + leftover = text.replace(candidate, "", 1) + leftover = _FENCE_RE.sub("", leftover).strip() + return calls, leftover + return _extract_react_calls(text, allowed) + + def _now() -> int: return int(time.time()) @@ -62,13 +325,19 @@ def _est_tokens(text: str) -> int: def completion_response(model: str, content: str, prompt: str, - conversation_id: str = None) -> dict: + conversation_id: str = None, + tool_calls: Optional[List[dict]] = None) -> dict: """A full (non-streaming) OpenAI chat.completion object. `conversation_id` is an extra top-level field (outside OpenAI's schema) you send back to resume the conversation. """ pt, ct = _est_tokens(prompt), _est_tokens(content) + message = {"role": "assistant", "content": content or None} + finish = "stop" + if tool_calls: + message["tool_calls"] = tool_calls + finish = "tool_calls" return { "id": _id(), "object": "chat.completion", @@ -78,8 +347,8 @@ def completion_response(model: str, content: str, prompt: str, "choices": [ { "index": 0, - "message": {"role": "assistant", "content": content}, - "finish_reason": "stop", + "message": message, + "finish_reason": finish, } ], "usage": { @@ -90,6 +359,20 @@ def completion_response(model: str, content: str, prompt: str, } +def _frame(cid: str, created: int, model: str, delta: dict, + finish=None, extra: dict = None) -> str: + obj = { + "id": cid, + "object": "chat.completion.chunk", + "created": created, + "model": model, + "choices": [{"index": 0, "delta": delta, "finish_reason": finish}], + } + if extra: + obj.update(extra) + return f"data: {json.dumps(obj, ensure_ascii=False)}\n\n" + + def stream_chunks(model: str, stream: Iterable[str]) -> Iterable[str]: """Yield OpenAI SSE lines (`data: {...}\\n\\n`) for a streamed completion. @@ -98,23 +381,51 @@ def stream_chunks(model: str, stream: Iterable[str]) -> Iterable[str]: """ cid, created = _id(), _now() - def frame(delta: dict, finish=None, extra: dict = None) -> str: - obj = { - "id": cid, - "object": "chat.completion.chunk", - "created": created, - "model": model, - "choices": [{"index": 0, "delta": delta, "finish_reason": finish}], - } - if extra: - obj.update(extra) - return f"data: {json.dumps(obj, ensure_ascii=False)}\n\n" - # First frame announces the assistant role. - yield frame({"role": "assistant", "content": ""}) + yield _frame(cid, created, model, {"role": "assistant", "content": ""}) for d in stream: if d: - yield frame({"content": d}) + yield _frame(cid, created, model, {"content": d}) + conversation_id = getattr(stream, "conversation_id", None) + yield _frame(cid, created, model, {}, finish="stop", + extra={"conversation_id": conversation_id}) + yield "data: [DONE]\n\n" + + +def stream_chunks_with_tools(model: str, stream: Iterable[str], tools) -> Iterable[str]: + """Streaming variant for tool-enabled requests. + + A tool call is only recognisable once its JSON object is complete, so the + upstream text is buffered and emitted as one delta: either the prose answer + or a `tool_calls` delta. Token-by-token output is lost for these requests — + correctness over cosmetics, since a half-parsed call is worse than useless. + """ + cid, created = _id(), _now() + yield _frame(cid, created, model, {"role": "assistant", "content": ""}) + + buf = "".join(d for d in stream if d) conversation_id = getattr(stream, "conversation_id", None) - yield frame({}, finish="stop", extra={"conversation_id": conversation_id}) + calls, leftover = extract_tool_calls(buf, tools) + + if calls: + delta = {"tool_calls": [ + { + "index": i, + "id": c["id"], + "type": "function", + "function": c["function"], + } + for i, c in enumerate(calls) + ]} + if leftover: + delta["content"] = leftover + yield _frame(cid, created, model, delta) + finish = "tool_calls" + else: + if buf: + yield _frame(cid, created, model, {"content": buf}) + finish = "stop" + + yield _frame(cid, created, model, {}, finish=finish, + extra={"conversation_id": conversation_id}) yield "data: [DONE]\n\n" diff --git a/server/schemas.py b/server/schemas.py index fba6a94..ab7042f 100644 --- a/server/schemas.py +++ b/server/schemas.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import List, Optional, Union +from typing import Any, Dict, List, Optional, Union from pydantic import BaseModel @@ -14,6 +14,12 @@ class ChatMessage(BaseModel): # content is a plain string, or a list of parts (OpenAI vision-style). We only # read text parts; non-text parts are ignored. content: Union[str, List[dict], None] = None + # Set on assistant turns that called tools, and on the `tool` turns that + # answer them. We replay both back into the prompt so a multi-step tool + # conversation keeps its shape (see openai_format.messages_to_prompt). + tool_calls: Optional[List[dict]] = None + tool_call_id: Optional[str] = None + name: Optional[str] = None class ChatCompletionRequest(BaseModel): @@ -26,6 +32,11 @@ class ChatCompletionRequest(BaseModel): # pass these via extra_body: `thinking` (DeepThink), `search` (web). thinking: bool = False search: bool = False + # OpenAI function calling. DeepSeek's web chat has no native tool channel, so + # these are emulated: the schemas go into the prompt and the model's JSON + # reply is parsed back into `tool_calls`. + tools: Optional[List[Dict[str, Any]]] = None + tool_choice: Optional[Union[str, Dict[str, Any]]] = None # Accepted for compatibility but not all are forwarded to DeepSeek. temperature: Optional[float] = None top_p: Optional[float] = None diff --git a/tests/test_tool_calling.py b/tests/test_tool_calling.py new file mode 100644 index 0000000..ae8da19 --- /dev/null +++ b/tests/test_tool_calling.py @@ -0,0 +1,191 @@ +"""Emulated tool calling: prompt contract in, OpenAI tool_calls out. + +Runs without a DeepSeek session — the upstream client is faked. + + ./venv/bin/python -m pytest tests/test_tool_calling.py +""" + +import json + +from fastapi.testclient import TestClient + +import server.api as api +from server.openai_format import extract_tool_calls, messages_to_prompt +from server.schemas import ChatMessage + +TOOLS = [{"type": "function", "function": { + "name": "browser_navigate", + "description": "Navigate to a URL", + "parameters": {"type": "object", "properties": {"url": {"type": "string"}}, + "required": ["url"]}}}] + +MCP_TOOLS = [{"type": "function", "function": { + "name": "mcp__openbrowser__browser_navigate", + "parameters": {"type": "object", "properties": {"url": {"type": "string"}}}}}] + + +def _names(calls): + return [c["function"]["name"] for c in calls or []] + + +def _args(calls, i=0): + return json.loads(calls[i]["function"]["arguments"]) + + +def test_fenced_json_is_a_tool_call(): + calls, left = extract_tool_calls( + '```json\n{"tool_calls":[{"name":"browser_navigate",' + '"arguments":{"url":"https://youtube.com"}}]}\n```', TOOLS) + assert _names(calls) == ["browser_navigate"] + assert _args(calls)["url"] == "https://youtube.com" + assert left == "" + + +def test_bare_json_and_prose_wrapper(): + for text in ( + '{"tool_calls":[{"name":"browser_navigate","arguments":{"url":"https://a.com"}}]}', + 'Sure.\n```json\n{"tool_calls":[{"name":"browser_navigate",' + '"arguments":{"url":"https://a.com"}}]}\n```', + ): + calls, _ = extract_tool_calls(text, TOOLS) + assert _args(calls)["url"] == "https://a.com" + + +def test_singular_and_openai_shaped_wrappers(): + calls, _ = extract_tool_calls( + '{"tool_call":{"name":"browser_navigate","arguments":{"url":"https://c.com"}}}', TOOLS) + assert _names(calls) == ["browser_navigate"] + calls, _ = extract_tool_calls( + '{"tool_calls":[{"type":"function","function":{"name":"browser_navigate",' + '"arguments":"{\\"url\\":\\"https://d.com\\"}"}}]}', TOOLS) + assert _args(calls)["url"] == "https://d.com" + + +def test_plain_answer_is_not_a_tool_call(): + calls, left = extract_tool_calls("YouTube is a video site.", TOOLS) + assert calls is None and left == "YouTube is a video site." + + +def test_unoffered_tool_is_rejected(): + # The regression that started this: models invent tools that don't exist. + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"browser_get_all_pages","arguments":{}}]}', TOOLS) + assert calls is None + + +def test_react_prose_is_salvaged(): + calls, left = extract_tool_calls( + "I'll navigate to YouTube.com using OpenBrowser.\n\n" + 'Action: mcp__openbrowser__browser_navigate ' + 'Action Input: {"url": "https://www.youtube.com"}', MCP_TOOLS) + assert _names(calls) == ["mcp__openbrowser__browser_navigate"] + assert _args(calls)["url"] == "https://www.youtube.com" + assert "Action:" not in left + + +def test_react_prose_resolves_a_dropped_namespace_prefix(): + calls, _ = extract_tool_calls( + 'Action: browser_navigate\nAction Input: {"url": "https://x.com"}', MCP_TOOLS) + assert _names(calls) == ["mcp__openbrowser__browser_navigate"] + + +def test_react_prose_for_an_unoffered_tool_stays_text(): + calls, _ = extract_tool_calls("Action: browser_get_all_pages Action Input: {}", MCP_TOOLS) + assert calls is None + + +def test_prompt_carries_schemas_and_forbids_prose_calls(): + prompt = messages_to_prompt( + [ChatMessage(role="user", content="go to youtube")], TOOLS) + assert "browser_navigate" in prompt + assert "tool_calls" in prompt + assert "Action:" in prompt # the explicit "don't do this" rule + + +def test_prompt_replays_past_calls_and_results(): + prompt = messages_to_prompt([ + ChatMessage(role="user", content="go to youtube"), + ChatMessage(role="assistant", tool_calls=[{"id": "call_1", "type": "function", + "function": {"name": "browser_navigate", + "arguments": '{"url":"https://youtube.com"}'}}]), + ChatMessage(role="tool", tool_call_id="call_1", name="browser_navigate", + content="navigated ok"), + ], TOOLS) + assert "Tool result (browser_navigate): navigated ok" in prompt + assert "https://youtube.com" in prompt + + +# --- endpoint level, with a faked upstream --------------------------------- + +class _FakeReply: + def __init__(self, text): + self.text, self.conversation_id = text, "conv-1" + + +class _FakeStream: + def __init__(self, text): + self.parts = [text[i:i + 7] for i in range(0, len(text), 7)] + self.conversation_id = "conv-1" + + def __iter__(self): + return iter(self.parts) + + +class _FakeClient: + canned = "" + + def chat(self, *a, **k): + return _FakeReply(self.canned) + + def stream(self, *a, **k): + return _FakeStream(self.canned) + + +def _client(canned): + fake = _FakeClient() + fake.canned = canned + api.get_client = lambda: fake + return TestClient(api.app) + + +def test_endpoint_returns_tool_calls_with_finish_reason(): + c = _client('```json\n{"tool_calls":[{"name":"browser_navigate",' + '"arguments":{"url":"https://www.youtube.com"}}]}\n```') + choice = c.post("/v1/chat/completions", json={ + "model": "deepseek-chat", "tools": TOOLS, + "messages": [{"role": "user", "content": "go to youtube"}], + }).json()["choices"][0] + assert choice["finish_reason"] == "tool_calls" + assert choice["message"]["content"] is None + assert _names(choice["message"]["tool_calls"]) == ["browser_navigate"] + + +def test_endpoint_final_answer_still_stops_normally(): + c = _client("YouTube loaded fine.") + choice = c.post("/v1/chat/completions", json={ + "model": "deepseek-chat", "tools": TOOLS, + "messages": [{"role": "user", "content": "what happened"}], + }).json()["choices"][0] + assert choice["finish_reason"] == "stop" + assert choice["message"]["content"] == "YouTube loaded fine." + assert "tool_calls" not in choice["message"] + + +def test_streaming_emits_a_tool_calls_delta(): + c = _client('{"tool_calls":[{"name":"browser_navigate","arguments":{"url":"https://x.com"}}]}') + body = c.post("/v1/chat/completions", json={ + "model": "deepseek-chat", "tools": TOOLS, "stream": True, + "messages": [{"role": "user", "content": "go"}], + }).text + assert '"tool_calls"' in body + assert '"finish_reason": "tool_calls"' in body + assert body.rstrip().endswith("data: [DONE]") + + +def test_tool_choice_none_suppresses_tools(): + c = _client("Just answering in text.") + choice = c.post("/v1/chat/completions", json={ + "model": "deepseek-chat", "tools": TOOLS, "tool_choice": "none", + "messages": [{"role": "user", "content": "hi"}], + }).json()["choices"][0] + assert choice["finish_reason"] == "stop" From 2b5eb7d8710d779bedc56eab27f746e9f7b289e2 Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 17 Aug 2026 21:53:55 -0500 Subject: [PATCH 2/8] Handle the reply shapes that leaked past the parser Three failures found driving a real agent loop against both models: - The brace scanner was quote-blind, so a brace inside a string value -- `{"text": "hi {name}"}`, a templated URL -- mis-balanced the count and the call leaked to the caller as raw JSON text. Scanning is now string- and escape-aware. - Replies cut off mid-object leaked the same way. A truncated object is now closed structurally so it still parses, but a half-written value is dropped along with its key rather than completed: `"tabId": 15` truncated from `1514652929` parses fine and selects a different tab, and a tool erroring on a missing argument beats one acting on a wrong one. - `deepseek-expert` writes calls as code -- `read_file({"path": "a.txt"})` -- which matched neither the JSON contract nor the ReAct salvage, so no tool ever ran for that model. Parsed now, with the same offered-tools-only check. The expert model also announced calls instead of making them ("I'll read the file right now.") and ended its turn. The tool contract was being rendered at the top of the prompt, ahead of the whole conversation; moving it to the end, next to the generation point, and naming the announce-instead-of-call failure explicitly fixes it. Verified end to end on both models, including a two-step loop that writes a file and reads it back. 11 new tests cover each shape. --- README.md | 13 ++- server/openai_format.py | 186 +++++++++++++++++++++++++++++++++---- tests/test_tool_calling.py | 87 +++++++++++++++++ 3 files changed, 265 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 339d17b..aaf105e 100644 --- a/README.md +++ b/README.md @@ -259,9 +259,16 @@ has consequences worth knowing: nested parameter schemas are where it frays first. - **Hallucinated tools are dropped.** A call naming a tool you didn't offer is returned as ordinary text, never as a `tool_call`. -- **Prose calls are salvaged.** Replies that narrate the call ReAct-style - (`Action: some_tool` / `Action Input: {...}`) are parsed into real calls - anyway, including when the model drops a namespace prefix. +- **Other call dialects are salvaged.** Models don't reliably follow the + contract, so replies that narrate the call ReAct-style (`Action: some_tool` / + `Action Input: {...}`) or write it as code (`some_tool({"arg": 1})`) are + parsed into real calls anyway, including when the model drops a namespace + prefix. `deepseek-expert` prefers the code form. +- **Truncated replies are repaired structurally, never by guessing.** A cut-off + object is closed so it can be parsed, but a half-written *value* is dropped + with its key: `"tabId": 15` truncated from `1514652929` parses cleanly and + points at the wrong thing, and a missing argument that errors is better than a + wrong one that gets acted on. - **`tool_choice`** supports `"none"`, `"required"`, and pinning a named function. - **Streaming buffers.** A tool call is only recognisable once its JSON is diff --git a/server/openai_format.py b/server/openai_format.py index 4dc5c1e..c590a4c 100644 --- a/server/openai_format.py +++ b/server/openai_format.py @@ -41,10 +41,14 @@ Rules, all of them mandatory: - Use ONLY tool names from the list. Never invent a tool that is not listed. - `arguments` must be a JSON object matching that tool's parameter schema. -- NEVER write a call as prose. Lines like "Action: some_tool" or - "Action Input: {...}" are not tool calls and do nothing. +- NEVER write a call as prose or as code. Lines like "Action: some_tool", + "Action Input: {...}", or `some_tool({"arg": "value"})` are not tool calls + and do nothing. Only the JSON object above is a tool call. - NEVER write out, guess, or imagine a tool's result. Stop after the JSON; the real result comes back to you in the next turn. +- NEVER announce a call in words. "I'll read the file now", "Let me check + that", "First I need to open it" — these do nothing, and the turn ends there. + If you intend to use a tool, the JSON object IS your entire reply, right now. - When you are done using tools and want to answer the user, reply with normal text and no JSON block. @@ -146,9 +150,6 @@ def messages_to_prompt(messages: List[ChatMessage], tools=None, return _text_of(messages[0].content) lines = [] - if preamble: - lines.append(preamble) - for m in messages: if m.role == "tool": # The answer to a call we made. Label it with the tool's name when @@ -164,6 +165,13 @@ def messages_to_prompt(messages: List[ChatMessage], tools=None, body = f"{body}\n{calls}".strip() if body else calls lines.append(f"{label}: {body}") + # The contract goes LAST, immediately before the generation point. Leading + # with it loses to the conversation that follows: the expert model in + # particular would answer "I'll read the file now" and stop, announcing a + # call instead of emitting one. + if preamble: + lines.append(preamble) + lines.append("Assistant:") return "\n\n".join(lines) @@ -196,6 +204,51 @@ def _resolve_name(name: str, allowed: List[str]) -> Optional[str]: return hits[0] if len(hits) == 1 else None +# Second salvage path: `tool_name({"arg": "value"})` call syntax, which the +# expert model reaches for in preference to a JSON object. The name is only +# accepted if it resolves to a tool that was actually offered, so this can't +# turn ordinary prose containing parentheses into a call. +_CALL_SYNTAX_RE = re.compile(r"([A-Za-z_][\w.\-]*)\s*\(\s*(?=[{)])") + + +def _extract_call_syntax(text: str, allowed: List[str]) -> Tuple[Optional[List[dict]], str]: + """Parse `tool_name({...})` / `tool_name()` invocations into tool calls.""" + calls, spans = [], [] + for m in _CALL_SYNTAX_RE.finditer(text): + name = _resolve_name(m.group(1), allowed) + if not name: + continue + end = m.end() + if text[end:end + 1] == ")": # no-argument call + args = {} + end += 1 + else: + obj, partial = _scan_object(text, end) + if not obj: + obj = _repair_truncated(partial or "") or "" + try: + args = json.loads(obj) + except (ValueError, TypeError): + continue + if not isinstance(args, dict): + continue + end += len(obj) + if text[end:end + 1] == ")": + end += 1 + calls.append({ + "id": "call_" + uuid.uuid4().hex[:24], + "type": "function", + "function": {"name": name, "arguments": json.dumps(args, ensure_ascii=False)}, + }) + spans.append((m.start(), end)) + if not calls: + return None, text + leftover = text + for start, stop in reversed(spans): + leftover = leftover[:start] + leftover[stop:] + return calls, leftover.strip() + + def _extract_react_calls(text: str, allowed: List[str]) -> Tuple[Optional[List[dict]], str]: """Parse ReAct-style `Action:` / `Action Input:` prose into tool calls.""" calls, spans = [], [] @@ -223,24 +276,118 @@ def _extract_react_calls(text: str, allowed: List[str]) -> Tuple[Optional[List[d return calls, leftover.strip() +def _scan_object(text: str, start: int) -> Tuple[Optional[str], Optional[str]]: + """Scan one JSON object starting at `text[start]` == '{'. + + Returns (complete, partial): `complete` is the balanced object if one closes, + otherwise `partial` is the truncated remainder (for the repair path below). + + Brace counting MUST ignore braces inside string literals. A quote-blind + scanner mis-balances on ordinary payloads — `{"text": "hi {name}"}` — and + the tool call leaks to the caller as raw JSON text. + """ + depth = 0 + in_string = False + escaped = False + for i in range(start, len(text)): + ch = text[i] + if in_string: + if escaped: + escaped = False + elif ch == "\\": + escaped = True + elif ch == '"': + in_string = False + continue + if ch == '"': + in_string = True + elif ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return text[start:i + 1], None + return None, text[start:] + + +def _repair_truncated(fragment: str) -> Optional[str]: + """Close a cut-off JSON object so it can be parsed, or None. + + A reply can end mid-object (upstream cut, length cap). The fragment is still + a real tool call the caller should get, so we balance the open brackets. + + We never complete a truncated VALUE. `"tabId": 15` cut from `1514652929` + parses fine and points at a different tab — a wrong argument is worse than a + missing one, because the caller acts on it instead of erroring. So a + half-written value is dropped along with its key, and a tool left missing a + required argument fails loudly where it belongs. + """ + if '"tool_calls"' not in fragment and '"name"' not in fragment: + return None + + stack = [] + in_string = False + escaped = False + for ch in fragment: + if in_string: + if escaped: + escaped = False + elif ch == "\\": + escaped = True + elif ch == '"': + in_string = False + continue + if ch == '"': + in_string = True + elif ch in "{[": + stack.append(ch) + elif ch in "}]" and stack: + stack.pop() + if not stack: + return None + + repaired = fragment + if in_string: + # Cut inside a string: drop the whole "key": "half-written pair. + opening = repaired.rfind('"') + repaired = repaired[:opening] + repaired = re.sub(r',?\s*"[^"]*"\s*:\s*$', "", repaired) + else: + # Cut inside a bare literal (number/true/null) or right after a key. + repaired = re.sub(r',?\s*"[^"]*"\s*:\s*(?:-?\d+(?:\.\d*)?(?:[eE][-+]?\d*)?' + r'|t(?:r(?:u(?:e)?)?)?|f(?:a(?:l(?:s(?:e)?)?)?)?' + r'|n(?:u(?:l(?:l)?)?)?)?$', "", repaired) + repaired = re.sub(r",\s*$", "", repaired) + for opener in reversed(stack): + repaired += "}" if opener == "{" else "]" + return repaired + + def _iter_json_candidates(text: str) -> Iterable[str]: """Yield substrings of `text` that might be the tool-call JSON object. - Fenced blocks first (what we asked for), then any brace-balanced object in - the raw text (what we sometimes get). + Fenced blocks first (what we asked for), then any string-aware balanced + object in the raw text, then a repaired tail if the reply was cut off. """ for m in _FENCE_RE.finditer(text): yield m.group(1) - depth, start = 0, None - for i, ch in enumerate(text): - if ch == "{": - if depth == 0: - start = i - depth += 1 - elif ch == "}" and depth: - depth -= 1 - if depth == 0 and start is not None: - yield text[start:i + 1] + partials = [] + i = 0 + while i < len(text): + if text[i] == "{": + complete, partial = _scan_object(text, i) + if complete: + yield complete + i += len(complete) + continue + if partial: + partials.append(partial) + break + i += 1 + for partial in partials: + repaired = _repair_truncated(partial) + if repaired: + yield repaired def _normalise_calls(obj, allowed: List[str]) -> Optional[List[dict]]: @@ -308,7 +455,10 @@ def extract_tool_calls(text: str, tools) -> Tuple[Optional[List[dict]], str]: leftover = text.replace(candidate, "", 1) leftover = _FENCE_RE.sub("", leftover).strip() return calls, leftover - return _extract_react_calls(text, allowed) + calls, leftover = _extract_react_calls(text, allowed) + if calls: + return calls, leftover + return _extract_call_syntax(text, allowed) def _now() -> int: diff --git a/tests/test_tool_calling.py b/tests/test_tool_calling.py index ae8da19..d3cf037 100644 --- a/tests/test_tool_calling.py +++ b/tests/test_tool_calling.py @@ -189,3 +189,90 @@ def test_tool_choice_none_suppresses_tools(): "messages": [{"role": "user", "content": "hi"}], }).json()["choices"][0] assert choice["finish_reason"] == "stop" + + +# --- parser robustness: the shapes that leaked in real use ----------------- + +def test_braces_inside_a_string_value_do_not_break_balance(): + # A quote-blind brace scanner mis-counts here and leaks the call as text. + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"browser_navigate",' + '"arguments":{"url":"https://x.com/?q={a}","note":"hi {name}, bye}"}}]}', TOOLS) + assert _args(calls)["url"] == "https://x.com/?q={a}" + assert _args(calls)["note"] == "hi {name}, bye}" + + +def test_escapes_inside_string_values(): + # Build it with json.dumps so the escaping is the real thing, not a + # hand-written literal: quotes and backslashes inside a value must not be + # mistaken for the end of the string while scanning for the closing brace. + tricky = 'say "hi" \\ then {x}' + payload = json.dumps({"tool_calls": [ + {"name": "browser_navigate", "arguments": {"url": tricky}}]}) + calls, _ = extract_tool_calls(payload, TOOLS) + assert _args(calls)["url"] == tricky + + +def test_unclosed_fence_still_parses(): + calls, _ = extract_tool_calls( + '```json\n{"tool_calls":[{"name":"browser_navigate",' + '"arguments":{"url":"https://a.com"}}]}', TOOLS) + assert _args(calls)["url"] == "https://a.com" + + +def test_truncated_reply_is_repaired_structurally(): + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"browser_navigate","arguments":{"url":"https://a.com","depth":', TOOLS) + assert _args(calls)["url"] == "https://a.com" + + +def test_truncated_value_is_dropped_not_guessed(): + # `1514652929` cut to `15` would parse and point somewhere else entirely. + # A missing argument fails loudly; a wrong one gets acted on. + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"browser_navigate","arguments":{"url":"https://a.com","tabId":15', TOOLS) + assert "tabId" not in _args(calls) + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"browser_navigate","arguments":{"url":"https://a.com","note":"half wri', TOOLS) + assert "note" not in _args(calls) + + +# --- call-syntax dialect (what the expert model emits) --------------------- + +def test_function_call_syntax_is_parsed(): + calls, left = extract_tool_calls('browser_navigate({"url": "https://a.com"})', TOOLS) + assert _names(calls) == ["browser_navigate"] + assert _args(calls)["url"] == "https://a.com" + assert left == "" + + +def test_call_syntax_with_surrounding_prose(): + calls, _ = extract_tool_calls( + 'Let me check.\nbrowser_navigate({"url": "https://a.com"})\nOne moment.', TOOLS) + assert _args(calls)["url"] == "https://a.com" + + +def test_call_syntax_no_arguments(): + calls, _ = extract_tool_calls("browser_navigate()", TOOLS) + assert _args(calls) == {} + + +def test_call_syntax_resolves_dropped_namespace(): + calls, _ = extract_tool_calls('browser_navigate({"url": "https://a.com"})', MCP_TOOLS) + assert _names(calls) == ["mcp__openbrowser__browser_navigate"] + + +def test_call_syntax_ignores_unoffered_tools_and_plain_prose(): + assert extract_tool_calls('delete_everything({"path": "/"})', TOOLS)[0] is None + assert extract_tool_calls("I read the file (the one you named) and it says hi.", TOOLS)[0] is None + + +def test_prompt_forbids_announcing_and_puts_contract_last(): + prompt = messages_to_prompt( + [ChatMessage(role="system", content="You are helpful."), + ChatMessage(role="user", content="read a file")], TOOLS) + assert "NEVER announce" in prompt + # The contract must sit next to the generation point, not above the chat: + # leading with it is what made the expert model announce instead of call. + assert prompt.index("browser_navigate") > prompt.index("You are helpful.") + assert prompt.rstrip().endswith("Assistant:") From 843ec5e70142fd8e9e0018900773a8ea2ac0105b Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 17 Aug 2026 22:00:04 -0500 Subject: [PATCH 3/8] Don't let one bad entry sink a whole batch of tool calls Validating a multi-call reply all-or-nothing meant a single invented tool name -- or a batch whose tail was truncated -- discarded every valid call beside it, and the entire array surfaced to the caller as raw JSON text. Entries are now validated individually: bad ones are skipped, the rest come through, and only a batch with nothing valid in it falls back to text. Also parse the XML dialects, which leak in when a client's own prompt format reaches the model: attribute form (``, the shape several agent frameworks train on) and `` wrappers around either JSON or a bare tool name. Same offered-tools-only rule, so prose and HTML containing angle brackets stay text. Verified against a live agent driving MCP browser tools over three rounds of assistant/tool turns, on both deepseek-chat and deepseek-expert. --- README.md | 9 +++-- server/openai_format.py | 75 ++++++++++++++++++++++++++++++++++---- tests/test_tool_calling.py | 66 +++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index aaf105e..ddf1b72 100644 --- a/README.md +++ b/README.md @@ -257,13 +257,16 @@ has consequences worth knowing: - **It's best-effort.** A prompt is not a decoder constraint. Complex or deeply nested parameter schemas are where it frays first. -- **Hallucinated tools are dropped.** A call naming a tool you didn't offer is - returned as ordinary text, never as a `tool_call`. +- **Hallucinated tools are dropped, individually.** A call naming a tool you + didn't offer is returned as ordinary text, never as a `tool_call` — but only + that call. In a batch, the valid calls beside it still come through. - **Other call dialects are salvaged.** Models don't reliably follow the contract, so replies that narrate the call ReAct-style (`Action: some_tool` / `Action Input: {...}`) or write it as code (`some_tool({"arg": 1})`) are parsed into real calls anyway, including when the model drops a namespace - prefix. `deepseek-expert` prefers the code form. + prefix. XML forms (``, `...`) + are handled too — they turn up when a client's own prompt format leaks into + the reply. `deepseek-expert` prefers the code form. - **Truncated replies are repaired structurally, never by guessing.** A cut-off object is closed so it can be parsed, but a half-written *value* is dropped with its key: `"tabId": 15` truncated from `1514652929` parses cleanly and diff --git a/server/openai_format.py b/server/openai_format.py index c590a4c..1946a87 100644 --- a/server/openai_format.py +++ b/server/openai_format.py @@ -42,8 +42,9 @@ - Use ONLY tool names from the list. Never invent a tool that is not listed. - `arguments` must be a JSON object matching that tool's parameter schema. - NEVER write a call as prose or as code. Lines like "Action: some_tool", - "Action Input: {...}", or `some_tool({"arg": "value"})` are not tool calls - and do nothing. Only the JSON object above is a tool call. + "Action Input: {...}", `some_tool({"arg": "value"})`, or + `` are not tool calls and do nothing. Only the JSON + object above is a tool call. - NEVER write out, guess, or imagine a tool's result. Stop after the JSON; the real result comes back to you in the next turn. - NEVER announce a call in words. "I'll read the file now", "Let me check @@ -249,6 +250,60 @@ def _extract_call_syntax(text: str, allowed: List[str]) -> Tuple[Optional[List[d return calls, leftover.strip() +# Third salvage path: XML-ish dialects. `` wrappers appear when a +# client's own prompt format leaks into the reply, and attribute form +# (``) is what several agent frameworks train models to +# emit. Both are only honoured for tools that were actually offered. +_XML_TAG_RE = re.compile(r"<\s*([A-Za-z_][\w.\-]*)\s*((?:[\w.\-]+\s*=\s*\"[^\"]*\"\s*)*)/?>") +_XML_ATTR_RE = re.compile(r"([\w.\-]+)\s*=\s*\"([^\"]*)\"") + + +def _coerce(value: str): + """Turn an XML attribute string into a JSON scalar where it clearly is one.""" + try: + parsed = json.loads(value) + except (ValueError, TypeError): + return value + return parsed if isinstance(parsed, (int, float, bool, list, dict)) else value + + +def _extract_xml_calls(text: str, allowed: List[str]) -> Tuple[Optional[List[dict]], str]: + """Parse `` and bare `name`.""" + calls, spans = [], [] + for m in _XML_TAG_RE.finditer(text): + tag, attrs = m.group(1), m.group(2) + name = _resolve_name(tag, allowed) + end = m.end() + if not name: + # A generic wrapper: the tool name is the tag's content instead. + if tag.lower() not in ("tool_call", "tool", "function_call", "invoke"): + continue + rest = text[m.end():] + inner = re.match(r"\s*([\w.\-]+)", rest) + if not inner: + continue + name = _resolve_name(inner.group(1), allowed) + if not name: + continue + end = m.end() + inner.end() + attrs = "" + args = {k: _coerce(v) for k, v in _XML_ATTR_RE.findall(attrs)} + calls.append({ + "id": "call_" + uuid.uuid4().hex[:24], + "type": "function", + "function": {"name": name, "arguments": json.dumps(args, ensure_ascii=False)}, + }) + spans.append((m.start(), end)) + if not calls: + return None, text + leftover = text + for start, stop in reversed(spans): + leftover = leftover[:start] + leftover[stop:] + # Tidy the now-orphaned closing tags. + leftover = re.sub(r"", "", leftover).strip() + return calls, leftover + + def _extract_react_calls(text: str, allowed: List[str]) -> Tuple[Optional[List[dict]], str]: """Parse ReAct-style `Action:` / `Action Input:` prose into tool calls.""" calls, spans = [], [] @@ -408,15 +463,18 @@ def _normalise_calls(obj, allowed: List[str]) -> Optional[List[dict]]: calls = [] for item in raw: + # Entries are validated INDIVIDUALLY and bad ones skipped. Failing the + # whole batch on one bad entry is how a valid `browser_tabs` call ends + # up rendered as raw JSON text next to an invented tool name. if not isinstance(item, dict): - return None + continue fn = item.get("function") if isinstance(item.get("function"), dict) else item name = fn.get("name") if not isinstance(name, str): - return None + continue name = _resolve_name(name, allowed) if allowed else name if not name: - return None + continue args = fn.get("arguments", fn.get("parameters", {})) if isinstance(args, str): try: @@ -424,7 +482,7 @@ def _normalise_calls(obj, allowed: List[str]) -> Optional[List[dict]]: except (ValueError, TypeError): args = {} if not isinstance(args, dict): - return None + continue calls.append({ "id": "call_" + uuid.uuid4().hex[:24], "type": "function", @@ -458,7 +516,10 @@ def extract_tool_calls(text: str, tools) -> Tuple[Optional[List[dict]], str]: calls, leftover = _extract_react_calls(text, allowed) if calls: return calls, leftover - return _extract_call_syntax(text, allowed) + calls, leftover = _extract_call_syntax(text, allowed) + if calls: + return calls, leftover + return _extract_xml_calls(text, allowed) def _now() -> int: diff --git a/tests/test_tool_calling.py b/tests/test_tool_calling.py index d3cf037..5f6cf44 100644 --- a/tests/test_tool_calling.py +++ b/tests/test_tool_calling.py @@ -276,3 +276,69 @@ def test_prompt_forbids_announcing_and_puts_contract_last(): # leading with it is what made the expert model announce instead of call. assert prompt.index("browser_navigate") > prompt.index("You are helpful.") assert prompt.rstrip().endswith("Assistant:") + + +# --- batches: one bad entry must not sink the valid ones ------------------- + +def test_batch_keeps_valid_calls_when_one_name_is_invented(): + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"browser_navigate","arguments":{"url":"https://a.com"}},' + '{"name":"browser_get_all_pages","arguments":{}}]}', TOOLS) + assert _names(calls) == ["browser_navigate"] + + +def test_batch_keeps_valid_calls_when_the_tail_is_truncated(): + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"browser_navigate","arguments":{"url":"https://a.com"}},' + '{"name":"browser_nav', TOOLS) + assert _names(calls) == ["browser_navigate"] + + +def test_batch_of_only_invalid_calls_stays_text(): + calls, _ = extract_tool_calls('{"tool_calls":[{"name":"nope","arguments":{}}]}', TOOLS) + assert calls is None + + +def test_multiple_valid_calls_all_survive(): + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"browser_navigate","arguments":{"url":"https://a.com"}},' + '{"name":"browser_navigate","arguments":{"url":"https://b.com"}}]}', TOOLS) + assert len(calls) == 2 + + +# --- XML dialects ---------------------------------------------------------- + +def test_xml_attribute_form(): + calls, left = extract_tool_calls('', TOOLS) + assert _args(calls)["url"] == "https://a.com" + assert left == "" + + +def test_xml_self_closing_coerces_scalars(): + calls, _ = extract_tool_calls('', TOOLS) + assert _args(calls) == {"url": "https://a.com", "depth": 3} + + +def test_xml_tool_call_wrapper_around_json(): + calls, _ = extract_tool_calls( + '{"name":"browser_navigate","arguments":{"url":"https://a.com"}}', + TOOLS) + assert _args(calls)["url"] == "https://a.com" + + +def test_xml_tool_call_wrapper_with_bare_name(): + calls, _ = extract_tool_calls("browser_navigate", TOOLS) + assert _names(calls) == ["browser_navigate"] + assert _args(calls) == {} + + +def test_xml_wrapper_naming_something_that_isnt_a_tool_stays_text(): + calls, _ = extract_tool_calls(" some-skill-name", TOOLS) + assert calls is None + + +def test_angle_brackets_in_prose_are_not_calls(): + for text in ("Use a < b and c > d in your filter.", + 'The page had a
element.', + "Compare and tags."): + assert extract_tool_calls(text, TOOLS)[0] is None From 0dc5cd39e6b79c32614327f04425a7a913221161 Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 17 Aug 2026 22:04:22 -0500 Subject: [PATCH 4/8] Unwrap nested tool_call wrappers and repair truncated fenced blocks Two shapes seen coming out of deepseek-expert while driving MCP browser tools: - The real call nested one level down under a wrapper that reuses the protocol's own vocabulary as the tool name: {"name": "tool_call", "arguments": {"name": "browser_navigate", "arguments": {...}}}. The outer name resolves to nothing, so the entry was dropped and the batch fell through to text. Wrappers are now peeled (bounded depth) and the inner call used, while a wrapper around a tool that was never offered still yields nothing. - Truncation inside a CLOSED fence. The repair path only ran on raw-text partials, so a cut-off object inside a tidy ```json block never reached it -- and the trailing fence markers corrupted the repair when it did. Fenced blocks now get the same structural repair, with fence markers stripped first. Both were reproduced live, then re-run against the fix: the same prompt that leaked raw JSON now completes a multi-step browser loop. --- README.md | 2 ++ server/openai_format.py | 64 +++++++++++++++++++++++++++++--------- tests/test_tool_calling.py | 47 ++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index ddf1b72..74ec5b8 100644 --- a/README.md +++ b/README.md @@ -267,6 +267,8 @@ has consequences worth knowing: prefix. XML forms (``, `...`) are handled too — they turn up when a client's own prompt format leaks into the reply. `deepseek-expert` prefers the code form. +- **Nested wrappers are unwrapped.** A model that names the tool `tool_call` and + nests the real call inside `arguments` still gets a working call. - **Truncated replies are repaired structurally, never by guessing.** A cut-off object is closed so it can be parsed, but a half-written *value* is dropped with its key: `"tabId": 15` truncated from `1514652929` parses cleanly and diff --git a/server/openai_format.py b/server/openai_format.py index 1946a87..3af9563 100644 --- a/server/openai_format.py +++ b/server/openai_format.py @@ -412,6 +412,7 @@ def _repair_truncated(fragment: str) -> Optional[str]: repaired = re.sub(r',?\s*"[^"]*"\s*:\s*(?:-?\d+(?:\.\d*)?(?:[eE][-+]?\d*)?' r'|t(?:r(?:u(?:e)?)?)?|f(?:a(?:l(?:s(?:e)?)?)?)?' r'|n(?:u(?:l(?:l)?)?)?)?$', "", repaired) + repaired = re.sub(r"[\s`]*$", "", repaired) repaired = re.sub(r",\s*$", "", repaired) for opener in reversed(stack): repaired += "}" if opener == "{" else "]" @@ -425,7 +426,14 @@ def _iter_json_candidates(text: str) -> Iterable[str]: object in the raw text, then a repaired tail if the reply was cut off. """ for m in _FENCE_RE.finditer(text): - yield m.group(1) + # A fenced block can itself be truncated — the model closes the fence + # but never finishes the object. Offer the repaired form too, or a cut + # call inside a tidy ```json block never reaches the caller. + body = m.group(1) + yield body + repaired = _repair_truncated(body) + if repaired: + yield repaired partials = [] i = 0 while i < len(text): @@ -445,6 +453,44 @@ def _iter_json_candidates(text: str) -> Iterable[str]: yield repaired +# Generic wrapper names a model reaches for when it nests the real call one +# level down: {"name": "tool_call", "arguments": {"name": , ...}}. +_WRAPPER_NAMES = ("tool_call", "tool", "function", "function_call", "invoke", "call") + + +def _unwrap_call(fn: dict, allowed: List[str], depth: int = 0) -> Optional[dict]: + """Peel generic wrappers off a call entry until the real one is found. + + Models sometimes echo the protocol's own vocabulary as the tool name and + nest the actual call inside `arguments`. The inner call is a perfectly good + one, so dig it out instead of dropping the entry (and, with it, every other + call in the same batch). + """ + if depth > 3 or not isinstance(fn, dict): + return None + name = fn.get("name") + if not isinstance(name, str): + return None + args = fn.get("arguments", fn.get("parameters", {})) + if isinstance(args, str): + try: + args = json.loads(args or "{}") + except (ValueError, TypeError): + args = {} + if not isinstance(args, dict): + return None + + resolved = _resolve_name(name, allowed) if allowed else name + # A nested call wins over a wrapper name, even one that happens to resolve. + if isinstance(args.get("name"), str) and (not resolved or name.lower() in _WRAPPER_NAMES): + inner = _unwrap_call(args, allowed, depth + 1) + if inner: + return inner + if not resolved: + return None + return {"name": resolved, "arguments": args} + + def _normalise_calls(obj, allowed: List[str]) -> Optional[List[dict]]: """Pull a list of {name, arguments} out of a parsed JSON object, or None. @@ -469,20 +515,10 @@ def _normalise_calls(obj, allowed: List[str]) -> Optional[List[dict]]: if not isinstance(item, dict): continue fn = item.get("function") if isinstance(item.get("function"), dict) else item - name = fn.get("name") - if not isinstance(name, str): - continue - name = _resolve_name(name, allowed) if allowed else name - if not name: - continue - args = fn.get("arguments", fn.get("parameters", {})) - if isinstance(args, str): - try: - args = json.loads(args or "{}") - except (ValueError, TypeError): - args = {} - if not isinstance(args, dict): + unwrapped = _unwrap_call(fn, allowed) + if not unwrapped: continue + name, args = unwrapped["name"], unwrapped["arguments"] calls.append({ "id": "call_" + uuid.uuid4().hex[:24], "type": "function", diff --git a/tests/test_tool_calling.py b/tests/test_tool_calling.py index 5f6cf44..413e359 100644 --- a/tests/test_tool_calling.py +++ b/tests/test_tool_calling.py @@ -342,3 +342,50 @@ def test_angle_brackets_in_prose_are_not_calls(): 'The page had a
element.', "Compare and tags."): assert extract_tool_calls(text, TOOLS)[0] is None + + +# --- nested wrappers and truncated fenced blocks --------------------------- + +def test_nested_tool_call_wrapper_is_unwrapped(): + # The model echoes the protocol's own vocabulary as the tool name and nests + # the real call inside `arguments`. + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"tool_call","arguments":' + '{"name":"browser_navigate","arguments":{"url":"https://a.com"}}}]}', TOOLS) + assert _names(calls) == ["browser_navigate"] + assert _args(calls)["url"] == "https://a.com" + + +def test_deeply_nested_wrappers_are_unwrapped(): + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"function","arguments":{"name":"tool_call","arguments":' + '{"name":"browser_navigate","arguments":{"url":"https://a.com"}}}}]}', TOOLS) + assert _names(calls) == ["browser_navigate"] + + +def test_wrapper_around_an_unoffered_tool_stays_text(): + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"tool_call","arguments":' + '{"name":"delete_everything","arguments":{}}}]}', TOOLS) + assert calls is None + + +def test_truncated_fenced_block_is_repaired(): + # The model closes the fence but never finishes the object. + calls, _ = extract_tool_calls( + '```json\n{"tool_calls": [{"name": "browser_navigate", ' + '"arguments": {"url": "https://a.com"}}\n```', TOOLS) + assert _args(calls)["url"] == "https://a.com" + + +def test_truncated_fenced_nested_wrapper_the_real_world_case(): + calls, _ = extract_tool_calls( + '```json\n{"tool_calls": [{"name": "tool_call", "arguments": ' + '{"name": "browser_navigate", "arguments": {"url": "https://mail.google.com"}}}\n```', + TOOLS) + assert _names(calls) == ["browser_navigate"] + assert _args(calls)["url"] == "https://mail.google.com" + + +def test_fenced_non_call_json_stays_text(): + assert extract_tool_calls('```json\n{"note": "nothing here"}\n```', TOOLS)[0] is None From 0a2612fe452167e7c4382e6f72aa6e055b66382e Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 17 Aug 2026 22:14:34 -0500 Subject: [PATCH 5/8] Pass unknown tool names through when the envelope is explicit Requiring every name to appear in the request's `tools` array assumed the array is the whole tool surface. It isn't: clients with lazy tool loading -- a tool-search/describe step, MCP servers resolved on demand -- send a partial array and the model calls tools by name from the system prompt. Every such call was rejected as a hallucination and dumped at the user as raw JSON, which is how a valid `mcp__openbrowser__browser_tabs` call ended up rendered as text. An explicit {"tool_calls": [...]} envelope now accepts names outside the array and lets the client resolve them -- it reports an unknown tool cleanly. The heuristic paths (ReAct prose, call syntax, XML) stay strict, since without an envelope a known name is the only thing separating a call from prose. Also logs emitted calls under DEBUG_REQUESTS, which is what made this diagnosable: the bridge was emitting the right call and the client was rejecting it. --- README.md | 10 ++++++--- server/api.py | 4 ++++ server/openai_format.py | 37 ++++++++++++++++++++++++------- tests/test_tool_calling.py | 45 ++++++++++++++++++++++++++++++-------- 4 files changed, 76 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 74ec5b8..2e1fd0e 100644 --- a/README.md +++ b/README.md @@ -257,9 +257,13 @@ has consequences worth knowing: - **It's best-effort.** A prompt is not a decoder constraint. Complex or deeply nested parameter schemas are where it frays first. -- **Hallucinated tools are dropped, individually.** A call naming a tool you - didn't offer is returned as ordinary text, never as a `tool_call` — but only - that call. In a batch, the valid calls beside it still come through. +- **Unknown tool names in an explicit `{"tool_calls": ...}` envelope are passed + through.** Clients with lazy tool loading (a tool-search step, MCP servers + resolved on demand) legitimately call tools that aren't in the request's + `tools` array, so the call goes to the client to resolve — it reports an + unknown tool cleanly, whereas dumping raw JSON at the user does not. The + heuristic salvage paths below stay strict: with no envelope to signal intent, + a known name is the only thing separating a call from ordinary prose. - **Other call dialects are salvaged.** Models don't reliably follow the contract, so replies that narrate the call ReAct-style (`Action: some_tool` / `Action Input: {...}`) or write it as code (`some_tool({"arg": 1})`) are diff --git a/server/api.py b/server/api.py index 7e47de5..d309988 100644 --- a/server/api.py +++ b/server/api.py @@ -172,5 +172,9 @@ def gen(): return _error(f"DeepSeek request failed: {e}") calls, text = extract_tool_calls(reply.text, tools) + if DEBUG_REQUESTS: + log.warning("reply: calls=%s text=%r", + [(c["function"]["name"], c["function"]["arguments"]) for c in calls or []], + (text or "")[:200]) return completion_response(req.model, text, prompt, reply.conversation_id, tool_calls=calls) diff --git a/server/openai_format.py b/server/openai_format.py index 3af9563..7a2f9f2 100644 --- a/server/openai_format.py +++ b/server/openai_format.py @@ -14,6 +14,8 @@ from __future__ import annotations import json +import logging +import os import re import time import uuid @@ -458,7 +460,8 @@ def _iter_json_candidates(text: str) -> Iterable[str]: _WRAPPER_NAMES = ("tool_call", "tool", "function", "function_call", "invoke", "call") -def _unwrap_call(fn: dict, allowed: List[str], depth: int = 0) -> Optional[dict]: +def _unwrap_call(fn: dict, allowed: List[str], depth: int = 0, + strict: bool = True) -> Optional[dict]: """Peel generic wrappers off a call entry until the real one is found. Models sometimes echo the protocol's own vocabulary as the tool name and @@ -483,20 +486,33 @@ def _unwrap_call(fn: dict, allowed: List[str], depth: int = 0) -> Optional[dict] resolved = _resolve_name(name, allowed) if allowed else name # A nested call wins over a wrapper name, even one that happens to resolve. if isinstance(args.get("name"), str) and (not resolved or name.lower() in _WRAPPER_NAMES): - inner = _unwrap_call(args, allowed, depth + 1) + inner = _unwrap_call(args, allowed, depth + 1, strict) if inner: return inner if not resolved: - return None + if strict or name.lower() in _WRAPPER_NAMES: + return None + # Non-strict: the model wrote an explicit call envelope for a name we + # can't match. Clients with lazy tool loading (a tool-search step, MCP + # servers resolved on demand) legitimately call tools that aren't in + # this request's `tools` array, so passing the call through is right -- + # the client owns resolution and reports an unknown tool cleanly. The + # alternative is dumping raw JSON at the user, which is strictly worse. + resolved = name return {"name": resolved, "arguments": args} -def _normalise_calls(obj, allowed: List[str]) -> Optional[List[dict]]: +def _normalise_calls(obj, allowed: List[str], strict: bool = True) -> Optional[List[dict]]: """Pull a list of {name, arguments} out of a parsed JSON object, or None. Lenient about the wrapper (`tool_calls`, `tool_call`, or a bare call) because - the model is not a schema-constrained decoder. Strict about the names: a call - to a tool that wasn't offered is a hallucination, not a call. + the model is not a schema-constrained decoder. + + `strict` controls unknown names. Heuristic salvage paths (prose, call syntax, + XML) pass strict=True: there, a known name is the only thing separating a + real call from ordinary text. An explicit `{"tool_calls": [...]}` envelope + passes strict=False, because the intent to call is unambiguous and the name + may simply not be in this request's `tools` array. """ if isinstance(obj, dict): raw = obj.get("tool_calls") or obj.get("tool_call") or obj @@ -515,7 +531,7 @@ def _normalise_calls(obj, allowed: List[str]) -> Optional[List[dict]]: if not isinstance(item, dict): continue fn = item.get("function") if isinstance(item.get("function"), dict) else item - unwrapped = _unwrap_call(fn, allowed) + unwrapped = _unwrap_call(fn, allowed, strict=strict) if not unwrapped: continue name, args = unwrapped["name"], unwrapped["arguments"] @@ -544,7 +560,7 @@ def extract_tool_calls(text: str, tools) -> Tuple[Optional[List[dict]], str]: obj = json.loads(candidate) except (ValueError, TypeError): continue - calls = _normalise_calls(obj, allowed) + calls = _normalise_calls(obj, allowed, strict=False) if calls: leftover = text.replace(candidate, "", 1) leftover = _FENCE_RE.sub("", leftover).strip() @@ -653,6 +669,11 @@ def stream_chunks_with_tools(model: str, stream: Iterable[str], tools) -> Iterab buf = "".join(d for d in stream if d) conversation_id = getattr(stream, "conversation_id", None) calls, leftover = extract_tool_calls(buf, tools) + if os.getenv("DEBUG_REQUESTS", "").lower() in ("1", "true", "yes", "on"): + logging.getLogger("uvicorn.error").warning( + "reply(stream): calls=%s text=%r", + [(c["function"]["name"], c["function"]["arguments"]) for c in calls or []], + (leftover or buf)[:200]) if calls: delta = {"tool_calls": [ diff --git a/tests/test_tool_calling.py b/tests/test_tool_calling.py index 413e359..fbc91cc 100644 --- a/tests/test_tool_calling.py +++ b/tests/test_tool_calling.py @@ -66,11 +66,15 @@ def test_plain_answer_is_not_a_tool_call(): assert calls is None and left == "YouTube is a video site." -def test_unoffered_tool_is_rejected(): - # The regression that started this: models invent tools that don't exist. +def test_explicit_envelope_passes_unknown_names_through(): + # Clients with lazy tool loading (a tool-search step, MCP servers resolved + # on demand) call tools that aren't in this request's `tools` array. The + # envelope makes the intent unambiguous, so the call goes through and the + # client resolves it -- reporting an unknown tool cleanly if it can't. calls, _ = extract_tool_calls( - '{"tool_calls":[{"name":"browser_get_all_pages","arguments":{}}]}', TOOLS) - assert calls is None + '{"tool_calls":[{"name":"mcp__openbrowser__browser_tabs",' + '"arguments":{"action":"list"}}]}', TOOLS) + assert _names(calls) == ["mcp__openbrowser__browser_tabs"] def test_react_prose_is_salvaged(): @@ -89,6 +93,15 @@ def test_react_prose_resolves_a_dropped_namespace_prefix(): assert _names(calls) == ["mcp__openbrowser__browser_navigate"] +def test_heuristic_paths_stay_strict_about_names(): + # Prose/code/XML salvage has no envelope to signal intent, so a known name + # is the only thing separating a real call from ordinary text. + for text in ('Action: made_up_tool Action Input: {"x": 1}', + 'made_up_tool({"x": 1})', + ''): + assert extract_tool_calls(text, TOOLS)[0] is None + + def test_react_prose_for_an_unoffered_tool_stays_text(): calls, _ = extract_tool_calls("Action: browser_get_all_pages Action Input: {}", MCP_TOOLS) assert calls is None @@ -280,10 +293,17 @@ def test_prompt_forbids_announcing_and_puts_contract_last(): # --- batches: one bad entry must not sink the valid ones ------------------- -def test_batch_keeps_valid_calls_when_one_name_is_invented(): +def test_batch_keeps_every_well_formed_call(): calls, _ = extract_tool_calls( '{"tool_calls":[{"name":"browser_navigate","arguments":{"url":"https://a.com"}},' '{"name":"browser_get_all_pages","arguments":{}}]}', TOOLS) + assert _names(calls) == ["browser_navigate", "browser_get_all_pages"] + + +def test_batch_drops_only_structurally_broken_entries(): + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"browser_navigate","arguments":{"url":"https://a.com"}},' + '{"arguments":{"no":"name"}},"not-an-object"]}', TOOLS) assert _names(calls) == ["browser_navigate"] @@ -294,8 +314,8 @@ def test_batch_keeps_valid_calls_when_the_tail_is_truncated(): assert _names(calls) == ["browser_navigate"] -def test_batch_of_only_invalid_calls_stays_text(): - calls, _ = extract_tool_calls('{"tool_calls":[{"name":"nope","arguments":{}}]}', TOOLS) +def test_batch_with_no_usable_entries_stays_text(): + calls, _ = extract_tool_calls('{"tool_calls":[{"arguments":{}},{"no":"name"}]}', TOOLS) assert calls is None @@ -363,10 +383,17 @@ def test_deeply_nested_wrappers_are_unwrapped(): assert _names(calls) == ["browser_navigate"] -def test_wrapper_around_an_unoffered_tool_stays_text(): +def test_wrapper_around_an_unknown_tool_still_unwraps_to_the_inner_name(): + # The wrapper itself is never the call; the inner name is, known or not. calls, _ = extract_tool_calls( '{"tool_calls":[{"name":"tool_call","arguments":' - '{"name":"delete_everything","arguments":{}}}]}', TOOLS) + '{"name":"mcp__openbrowser__browser_snapshot","arguments":{}}}]}', TOOLS) + assert _names(calls) == ["mcp__openbrowser__browser_snapshot"] + + +def test_wrapper_with_no_inner_name_is_not_a_call(): + calls, _ = extract_tool_calls( + '{"tool_calls":[{"name":"tool_call","arguments":{"foo":"bar"}}]}', TOOLS) assert calls is None From 698d1780313e839e60eb71472ce77a125def5967 Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 17 Aug 2026 22:19:44 -0500 Subject: [PATCH 6/8] Stop fabricated continuations, and recover calls from broken envelopes Two more shapes from driving a real agent loop: - The prompt serialises the conversation with role labels, so the model would emit a call and then keep going -- writing the "Tool result (...)" line and the next turn itself. An invented tool result reaching the caller as if it were real is the worst failure in this file, so the reply is now cut at the first role label. - An envelope can be malformed rather than truncated (an array that never closes) while the call objects inside it are perfectly well-formed. Those are now recovered individually instead of the whole reply falling through to text. A bare object must carry both a `name` and an `arguments`/`parameters` mapping to count, so JSON quoted in prose doesn't become a call. --- README.md | 9 +++++ server/api.py | 5 +++ server/openai_format.py | 72 ++++++++++++++++++++++++++++++++++++++ tests/test_tool_calling.py | 55 ++++++++++++++++++++++++++++- 4 files changed, 140 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2e1fd0e..cc01892 100644 --- a/README.md +++ b/README.md @@ -284,6 +284,15 @@ has consequences worth knowing: complete, so tool-enabled streaming requests emit one delta instead of token-by-token output. Requests without `tools` stream as before. +- **Replies are cut at the first role label.** The prompt serialises the + conversation with `User:` / `Assistant:` / `Tool result (...)` labels, and a + model will happily keep writing past its own turn — inventing the tool result + and the next question. Everything after the first label is dropped, so a + fabricated result can never reach the caller as if it were real. +- **Calls survive a malformed envelope.** If the `{"tool_calls": [...]}` wrapper + itself is broken (an array that never closes), the well-formed call objects + inside it are still recovered. + Set `DEBUG_REQUESTS=1` to log each request's message roles and offered tool names — the quickest way to tell whether a client actually sent `tools`. diff --git a/server/api.py b/server/api.py index d309988..5a352c3 100644 --- a/server/api.py +++ b/server/api.py @@ -45,6 +45,7 @@ from .openai_format import ( completion_response, extract_tool_calls, + trim_at_role_boundary, messages_to_prompt, stream_chunks, stream_chunks_with_tools, @@ -172,6 +173,10 @@ def gen(): return _error(f"DeepSeek request failed: {e}") calls, text = extract_tool_calls(reply.text, tools) + if not tools: + # No tools this turn, so extract_tool_calls returned early — the reply + # can still run on past its turn, so trim it here too. + text = trim_at_role_boundary(text) if DEBUG_REQUESTS: log.warning("reply: calls=%s text=%r", [(c["function"]["name"], c["function"]["arguments"]) for c in calls or []], diff --git a/server/openai_format.py b/server/openai_format.py index 7a2f9f2..d1de0f4 100644 --- a/server/openai_format.py +++ b/server/openai_format.py @@ -514,6 +514,7 @@ def _normalise_calls(obj, allowed: List[str], strict: bool = True) -> Optional[L passes strict=False, because the intent to call is unambiguous and the name may simply not be in this request's `tools` array. """ + wrapped = isinstance(obj, dict) and ("tool_calls" in obj or "tool_call" in obj) if isinstance(obj, dict): raw = obj.get("tool_calls") or obj.get("tool_call") or obj else: @@ -531,6 +532,12 @@ def _normalise_calls(obj, allowed: List[str], strict: bool = True) -> Optional[L if not isinstance(item, dict): continue fn = item.get("function") if isinstance(item.get("function"), dict) else item + # With no `tool_calls` wrapper to signal intent, require the entry to + # actually look like a call. Otherwise any JSON object with a "name" -- + # a config blob quoted in prose -- becomes one. + if not wrapped and not isinstance( + fn.get("arguments", fn.get("parameters")), dict): + continue unwrapped = _unwrap_call(fn, allowed, strict=strict) if not unwrapped: continue @@ -546,6 +553,67 @@ def _normalise_calls(obj, allowed: List[str], strict: bool = True) -> Optional[L return calls or None +def _extract_inner_calls(text: str, allowed: List[str]) -> Tuple[Optional[List[dict]], str]: + """Pick individual call objects out of a malformed envelope. + + The wrapper itself can be broken in ways no repair should guess at — an + array that never closes, a stray brace — while the call objects inside it + are perfectly well-formed. Rather than lose them, scan for objects carrying + both a string `name` and an `arguments`/`parameters` mapping, which is + specific enough that ordinary JSON in prose doesn't qualify. + """ + calls, spans, i = [], [], 0 + while i < len(text): + if text[i] != "{": + i += 1 + continue + complete, partial = _scan_object(text, i) + frag = complete or _repair_truncated(partial or "") or "" + obj = None + if frag: + try: + obj = json.loads(frag) + except (ValueError, TypeError): + obj = None + if (isinstance(obj, dict) and isinstance(obj.get("name"), str) + and isinstance(obj.get("arguments", obj.get("parameters")), dict)): + one = _normalise_calls([obj], allowed, strict=False) + if one: + calls.extend(one) + spans.append((i, i + len(frag))) + i += len(frag) + continue + i += 1 + if not calls: + return None, text + leftover = text + for start, stop in reversed(spans): + leftover = leftover[:start] + leftover[stop:] + leftover = re.sub(r"```(?:json)?", "", leftover) + leftover = re.sub(r'[\s,\[\]{}]*$', "", leftover).strip() + return calls, leftover + + +# The prompt serialises the conversation with role labels, so the model can +# just... keep going: emit a call, then write the "Tool result (...)" line and +# the next user turn itself. Everything past its own turn is fabricated -- most +# dangerously an invented tool result -- so the reply is cut at the first label. +_ROLE_BOUNDARY_RE = re.compile( + r"^\s*(?:Tool result\s*\(|User\s*:|System\s*:|Assistant\s*:|Human\s*:)", + re.MULTILINE, +) + + +def trim_at_role_boundary(text: str) -> str: + """Drop any continuation past the model's own turn.""" + if not text: + return text + m = _ROLE_BOUNDARY_RE.search(text) + if not m or m.start() == 0: + return text + return text[:m.start()].rstrip() + + def extract_tool_calls(text: str, tools) -> Tuple[Optional[List[dict]], str]: """Split a reply into (tool_calls, leftover_text). @@ -555,6 +623,7 @@ def extract_tool_calls(text: str, tools) -> Tuple[Optional[List[dict]], str]: allowed = tool_names(tools) if not allowed or not text: return None, text + text = trim_at_role_boundary(text) for candidate in _iter_json_candidates(text): try: obj = json.loads(candidate) @@ -565,6 +634,9 @@ def extract_tool_calls(text: str, tools) -> Tuple[Optional[List[dict]], str]: leftover = text.replace(candidate, "", 1) leftover = _FENCE_RE.sub("", leftover).strip() return calls, leftover + calls, leftover = _extract_inner_calls(text, allowed) + if calls: + return calls, leftover calls, leftover = _extract_react_calls(text, allowed) if calls: return calls, leftover diff --git a/tests/test_tool_calling.py b/tests/test_tool_calling.py index fbc91cc..5749725 100644 --- a/tests/test_tool_calling.py +++ b/tests/test_tool_calling.py @@ -10,7 +10,11 @@ from fastapi.testclient import TestClient import server.api as api -from server.openai_format import extract_tool_calls, messages_to_prompt +from server.openai_format import ( + extract_tool_calls, + messages_to_prompt, + trim_at_role_boundary, +) from server.schemas import ChatMessage TOOLS = [{"type": "function", "function": { @@ -416,3 +420,52 @@ def test_truncated_fenced_nested_wrapper_the_real_world_case(): def test_fenced_non_call_json_stays_text(): assert extract_tool_calls('```json\n{"note": "nothing here"}\n```', TOOLS)[0] is None + + +# --- the model running past its own turn ----------------------------------- + +def test_fabricated_continuation_is_trimmed(): + # The prompt uses role labels, so the model will happily write the tool + # result and the next turn itself. Everything past its turn is invented. + calls, left = extract_tool_calls( + '{"tool_calls": [{"name": "browser_navigate", "arguments": {"url": "https://a.com"}}]}\n' + "\nTool result (browser_navigate): opened tab 123\n" + "\nAssistant: I opened it and the page loaded.", TOOLS) + assert _names(calls) == ["browser_navigate"] + assert "Tool result" not in (left or "") + assert "opened tab 123" not in (left or "") + + +def test_trim_leaves_ordinary_prose_alone(): + for text in ("The inbox shows 32 unread messages.", + "Here is the result: 5 tabs open.", + "Assistant: is a word that can start a sentence."): + assert trim_at_role_boundary(text) == text + + +# --- malformed envelopes --------------------------------------------------- + +def test_calls_survive_an_envelope_whose_array_never_closes(): + calls, _ = extract_tool_calls( + '{"tool_calls": [{"name": "browser_navigate", "arguments": {"url": "https://a.com"}}}', TOOLS) + assert _args(calls)["url"] == "https://a.com" + + +def test_multiple_calls_survive_a_broken_envelope(): + calls, _ = extract_tool_calls( + '{"tool_calls": [{"name": "browser_navigate", "arguments": {"url": "https://a.com"}},' + '{"name": "browser_navigate", "arguments": {"url": "https://b.com"}}}', TOOLS) + assert len(calls) == 2 + + +def test_json_quoted_in_prose_is_not_a_call(): + # A bare object needs to look like a call; a "name" alone isn't enough. + calls, _ = extract_tool_calls( + 'The config was {"name": "widget", "size": 3} which I updated.', TOOLS) + assert calls is None + + +def test_bare_object_with_arguments_is_a_call(): + calls, _ = extract_tool_calls( + '{"name": "browser_navigate", "arguments": {"url": "https://a.com"}}', TOOLS) + assert _names(calls) == ["browser_navigate"] From 81c30bab274f8983e2fa7465633505f72e3a67e7 Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 17 Aug 2026 22:22:51 -0500 Subject: [PATCH 7/8] Surface upstream error frames instead of returning an empty reply DeepSeek reports failures inside the completion stream as an `event: hint` frame -- {"type": "error", "content": "Messages too frequent. Try again later.", "finish_reason": "rate_limit_reached"} -- not as an HTTP status. The SSE parser skipped any frame it didn't recognise as text, so those errors vanished and the request completed as a 200 with empty content. A caller sees "the model returned nothing": no reason, and nothing to back off from. Agent loops hit this constantly, since every tool round trip is another completion. Error frames now raise UpstreamError, which the endpoint maps to 429 + Retry-After for rate limits and 502 otherwise. Streaming gets the same status: the first delta is pulled before the response starts, so an error lands as a status code rather than arriving mid-body after a 200 already went out. --- README.md | 30 +++++++++++++++++++ deepseek/__init__.py | 4 +-- deepseek/client.py | 23 ++++++++++++++ server/api.py | 61 ++++++++++++++++++++++++++++++++++---- tests/test_tool_calling.py | 57 +++++++++++++++++++++++++++++++++++ 5 files changed, 167 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index cc01892..c37152e 100644 --- a/README.md +++ b/README.md @@ -301,6 +301,36 @@ needed; the upstream client is faked). --- +## Upstream errors + +DeepSeek reports failures inside the completion stream as an `event: hint` +frame, not an HTTP status: + +``` +event: hint +data: {"type":"error","content":"Messages too frequent. Try again later.", + "finish_reason":"rate_limit_reached"} +``` + +Those frames are surfaced rather than dropped, because a dropped one becomes a +`200` with empty content — indistinguishable from the model having nothing to +say, with no reason to show and nothing for a client to back off from. + +| Upstream | Response | +| --- | --- | +| `rate_limit_reached` | `429` + `Retry-After` (`UPSTREAM_RETRY_AFTER`, default 30) | +| any other error frame | `502` with the upstream message | + +Streaming requests get the same status: the first delta is pulled before the +response starts, so an error still lands as a status code rather than arriving +mid-body after a `200`. + +Note this is DeepSeek's own per-account limit on the web chat, separate from +this server's `RATE_LIMIT_PER_MINUTE`. Agent loops are the usual way to hit it — +each tool round trip is another completion. + +--- + ## Concurrency The server bridges a **single** signed-in DeepSeek account behind one shared diff --git a/deepseek/__init__.py b/deepseek/__init__.py index 4856137..829205c 100644 --- a/deepseek/__init__.py +++ b/deepseek/__init__.py @@ -1,7 +1,7 @@ """Unofficial OpenAI-compatible client for chat.deepseek.com.""" from .auth import Session, get_session, login -from .client import DeepSeekClient, Reply +from .client import DeepSeekClient, Reply, UpstreamError from .pow import DeepSeekPow -__all__ = ["Session", "get_session", "login", "DeepSeekClient", "Reply", "DeepSeekPow"] +__all__ = ["Session", "get_session", "login", "DeepSeekClient", "Reply", "UpstreamError", "DeepSeekPow"] diff --git a/deepseek/client.py b/deepseek/client.py index 677c75b..d1cc68e 100644 --- a/deepseek/client.py +++ b/deepseek/client.py @@ -44,6 +44,25 @@ _CID_SEP = ":" +class UpstreamError(RuntimeError): + """An error DeepSeek reported inside the completion stream. + + The stream carries failures as an `event: hint` frame rather than an HTTP + status, e.g. {"type": "error", "content": "Messages too frequent. Try again + later.", "finish_reason": "rate_limit_reached"}. Dropping those frames turns + a rate limit into a silent empty reply, which the caller can only report as + "the model returned nothing" -- no reason to show, nothing to back off from. + """ + + def __init__(self, message: str, finish_reason: str = ""): + super().__init__(message or "DeepSeek reported an error") + self.finish_reason = finish_reason or "" + + @property + def is_rate_limit(self) -> bool: + return "rate_limit" in self.finish_reason.lower() + + def _encode_cid(session_id: str, message_id: Optional[int]) -> str: if message_id is None: return session_id @@ -259,6 +278,10 @@ def _parse_sse(lines, meta: Optional[dict] = None) -> Iterator[str]: except json.JSONDecodeError: continue + if obj.get("type") == "error" or obj.get("finish_reason") == "rate_limit_reached": + raise UpstreamError(str(obj.get("content") or "").strip(), + str(obj.get("finish_reason") or "")) + v = obj.get("v") # Snapshot frame: full response object. diff --git a/server/api.py b/server/api.py index 5a352c3..e1a92e2 100644 --- a/server/api.py +++ b/server/api.py @@ -33,7 +33,7 @@ from starlette.concurrency import run_in_threadpool from deepseek.auth import LoginRequired -from deepseek.client import DeepSeekClient +from deepseek.client import DeepSeekClient, UpstreamError from .config import ( MODEL_MAP, @@ -63,6 +63,8 @@ # at all, which is the difference between "bridge ignored them" and "client # never offered them". DEBUG_REQUESTS = os.getenv("DEBUG_REQUESTS", "").lower() in ("1", "true", "yes", "on") +# Seconds to advertise in Retry-After when DeepSeek rate-limits us. +UPSTREAM_RETRY_AFTER = os.getenv("UPSTREAM_RETRY_AFTER", "30") app = FastAPI(title="DeepSeek OpenAI-compatible API", version="0.1.0") install_rate_limit(app, RateLimiter(limit=RATE_LIMIT_PER_MINUTE, window=60.0)) @@ -92,13 +94,49 @@ def get_client() -> DeepSeekClient: return _client -def _error(message: str, status: int = 500, err_type: str = "server_error"): +def _error(message: str, status: int = 500, err_type: str = "server_error", + headers: dict = None): return JSONResponse( status_code=status, content={"error": {"message": message, "type": err_type}}, + headers=headers or None, ) +def _upstream_error(exc: UpstreamError): + """Map a DeepSeek stream error onto the status a client can act on. + + A rate limit must be a 429 with Retry-After: OpenAI-compatible clients back + off and retry on that, whereas a 200 with empty content just looks like the + model had nothing to say. + """ + if exc.is_rate_limit: + return _error(str(exc), status=429, err_type="rate_limit_exceeded", + headers={"Retry-After": UPSTREAM_RETRY_AFTER}) + return _error(f"DeepSeek reported: {exc}", status=502, err_type="upstream_error") + + +class _Prefetched: + """A stream with its first delta already pulled. + + The endpoint consumes one delta before returning a response, so an error + frame -- which DeepSeek sends immediately -- becomes a real status code + instead of a 200 whose body turns out to be an error mid-flight. + """ + + def __init__(self, stream, first): + self._stream, self._first = stream, first + + def __iter__(self): + if self._first is not None: + yield self._first + yield from self._stream + + @property + def conversation_id(self): + return getattr(self._stream, "conversation_id", None) + + @app.get("/healthz") def healthz(): return {"status": "ok"} @@ -152,11 +190,20 @@ async def chat_completions(req: ChatCompletionRequest): return _error(f"Failed to initialise DeepSeek session: {e}") if req.stream: + raw = client.stream( + prompt, conversation_id=req.conversation_id, + model=model_type, thinking=req.thinking, search=req.search, + ) + it = iter(raw) + try: + first = await run_in_threadpool(lambda: next(it, None)) + except UpstreamError as e: + return _upstream_error(e) + except Exception as e: + return _error(f"DeepSeek request failed: {e}") + def gen(): - stream = client.stream( - prompt, conversation_id=req.conversation_id, - model=model_type, thinking=req.thinking, search=req.search, - ) + stream = _Prefetched(raw, first) if tools: yield from stream_chunks_with_tools(req.model, stream, tools) else: @@ -169,6 +216,8 @@ def gen(): client.chat, prompt, req.conversation_id, model_type, req.thinking, req.search, ) + except UpstreamError as e: + return _upstream_error(e) except Exception as e: return _error(f"DeepSeek request failed: {e}") diff --git a/tests/test_tool_calling.py b/tests/test_tool_calling.py index 5749725..01ef258 100644 --- a/tests/test_tool_calling.py +++ b/tests/test_tool_calling.py @@ -469,3 +469,60 @@ def test_bare_object_with_arguments_is_a_call(): calls, _ = extract_tool_calls( '{"name": "browser_navigate", "arguments": {"url": "https://a.com"}}', TOOLS) assert _names(calls) == ["browser_navigate"] + + +# --- upstream errors must not look like an empty reply --------------------- + +def test_stream_error_frame_raises_instead_of_yielding_nothing(): + from deepseek.client import _parse_sse + from deepseek import UpstreamError + frames = ['data: {"type":"error","content":"Messages too frequent. Try again ' + 'later.","finish_reason":"rate_limit_reached"}'] + try: + list(_parse_sse(iter(frames))) + except UpstreamError as exc: + assert exc.is_rate_limit + assert "too frequent" in str(exc) + else: + raise AssertionError("rate-limit frame was swallowed") + + +def test_normal_stream_frames_still_parse(): + from deepseek.client import _parse_sse + frames = ['data: {"p":"response/fragments/-1/content","o":"APPEND","v":"hi"}', + 'data: {"v":" there"}'] + assert list(_parse_sse(iter(frames))) == ["hi", " there"] + + +def _erroring_client(exc): + class _C: + def chat(self, *a, **k): + raise exc + + def stream(self, *a, **k): + def g(): + raise exc + yield # pragma: no cover + return g() + api.get_client = lambda: _C() + return TestClient(api.app, raise_server_exceptions=False) + + +def test_rate_limit_becomes_429_with_retry_after(): + from deepseek import UpstreamError + c = _erroring_client(UpstreamError("Messages too frequent.", "rate_limit_reached")) + for stream in (False, True): + r = c.post("/v1/chat/completions", json={ + "model": "deepseek-chat", "stream": stream, + "messages": [{"role": "user", "content": "hi"}]}) + assert r.status_code == 429, (stream, r.status_code) + assert r.headers.get("Retry-After") + assert r.json()["error"]["type"] == "rate_limit_exceeded" + + +def test_other_upstream_errors_become_502(): + from deepseek import UpstreamError + c = _erroring_client(UpstreamError("Something broke upstream.", "server_error")) + r = c.post("/v1/chat/completions", json={ + "model": "deepseek-chat", "messages": [{"role": "user", "content": "hi"}]}) + assert r.status_code == 502 From 8027030aebe50fb043ca89d90f63c0222071faf4 Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 17 Aug 2026 22:31:48 -0500 Subject: [PATCH 8/8] Parse Python dict repr, and don't replay the prefetched delta - Models emit Python repr as readily as JSON -- {'tool_calls': [...]} with single quotes, True/None -- which json.loads rejects, so a valid call fell through to text. Parsing now falls back to ast.literal_eval, which handles literals only and never becomes an evaluator. - Fix a bug in the error-surfacing prefetch: it re-iterated the stream object rather than continuing the already-advanced iterator, replaying the first delta. Visible as a doubled leading character ("II'll..."), and capable of corrupting a tool call whose reply begins with a brace. --- README.md | 3 +++ server/api.py | 12 ++++++++---- server/openai_format.py | 38 ++++++++++++++++++++++++------------- tests/test_tool_calling.py | 39 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index c37152e..e17db1a 100644 --- a/README.md +++ b/README.md @@ -271,6 +271,9 @@ has consequences worth knowing: prefix. XML forms (``, `...`) are handled too — they turn up when a client's own prompt format leaks into the reply. `deepseek-expert` prefers the code form. +- **Python dict repr is accepted.** `{'tool_calls': [...]}` with single quotes + and `True`/`None` is parsed via `ast.literal_eval` (literals only — it stays a + parser, never an evaluator). - **Nested wrappers are unwrapped.** A model that names the tool `tool_call` and nests the real call inside `arguments` still gets a working call. - **Truncated replies are repaired structurally, never by guessing.** A cut-off diff --git a/server/api.py b/server/api.py index e1a92e2..81947e8 100644 --- a/server/api.py +++ b/server/api.py @@ -124,13 +124,17 @@ class _Prefetched: instead of a 200 whose body turns out to be an error mid-flight. """ - def __init__(self, stream, first): - self._stream, self._first = stream, first + def __init__(self, stream, iterator, first): + # `iterator` is the ALREADY-ADVANCED iterator, not the stream object. + # Re-iterating the stream replays it from the beginning, which silently + # duplicates the prefetched delta ("II'll..." instead of "I'll...") and + # can corrupt a tool call whose reply starts with `{`. + self._stream, self._iter, self._first = stream, iterator, first def __iter__(self): if self._first is not None: yield self._first - yield from self._stream + yield from self._iter @property def conversation_id(self): @@ -203,7 +207,7 @@ async def chat_completions(req: ChatCompletionRequest): return _error(f"DeepSeek request failed: {e}") def gen(): - stream = _Prefetched(raw, first) + stream = _Prefetched(raw, it, first) if tools: yield from stream_chunks_with_tools(req.model, stream, tools) else: diff --git a/server/openai_format.py b/server/openai_format.py index d1de0f4..c826f1d 100644 --- a/server/openai_format.py +++ b/server/openai_format.py @@ -13,6 +13,7 @@ from __future__ import annotations +import ast import json import logging import os @@ -229,10 +230,7 @@ def _extract_call_syntax(text: str, allowed: List[str]) -> Tuple[Optional[List[d obj, partial = _scan_object(text, end) if not obj: obj = _repair_truncated(partial or "") or "" - try: - args = json.loads(obj) - except (ValueError, TypeError): - continue + args = _loads(obj) if not isinstance(args, dict): continue end += len(obj) @@ -333,6 +331,26 @@ def _extract_react_calls(text: str, allowed: List[str]) -> Tuple[Optional[List[d return calls, leftover.strip() +def _loads(fragment: str): + """Parse a JSON object, falling back to Python literal syntax. + + Models emit Python dict repr as readily as JSON — `{'tool_calls': [...]}` + with single quotes, `True`/`None` — which `json.loads` rejects outright. + `ast.literal_eval` handles literals only (no calls, no names), so this + stays a parser and never becomes an evaluator. + """ + if not fragment: + return None + try: + return json.loads(fragment) + except (ValueError, TypeError): + pass + try: + return ast.literal_eval(fragment) + except (ValueError, TypeError, SyntaxError, MemoryError, RecursionError): + return None + + def _scan_object(text: str, start: int) -> Tuple[Optional[str], Optional[str]]: """Scan one JSON object starting at `text[start]` == '{'. @@ -569,12 +587,7 @@ def _extract_inner_calls(text: str, allowed: List[str]) -> Tuple[Optional[List[d continue complete, partial = _scan_object(text, i) frag = complete or _repair_truncated(partial or "") or "" - obj = None - if frag: - try: - obj = json.loads(frag) - except (ValueError, TypeError): - obj = None + obj = _loads(frag) if frag else None if (isinstance(obj, dict) and isinstance(obj.get("name"), str) and isinstance(obj.get("arguments", obj.get("parameters")), dict)): one = _normalise_calls([obj], allowed, strict=False) @@ -625,9 +638,8 @@ def extract_tool_calls(text: str, tools) -> Tuple[Optional[List[dict]], str]: return None, text text = trim_at_role_boundary(text) for candidate in _iter_json_candidates(text): - try: - obj = json.loads(candidate) - except (ValueError, TypeError): + obj = _loads(candidate) + if obj is None: continue calls = _normalise_calls(obj, allowed, strict=False) if calls: diff --git a/tests/test_tool_calling.py b/tests/test_tool_calling.py index 01ef258..0c78123 100644 --- a/tests/test_tool_calling.py +++ b/tests/test_tool_calling.py @@ -526,3 +526,42 @@ def test_other_upstream_errors_become_502(): r = c.post("/v1/chat/completions", json={ "model": "deepseek-chat", "messages": [{"role": "user", "content": "hi"}]}) assert r.status_code == 502 + + +def test_prefetched_stream_does_not_duplicate_the_first_delta(): + # The endpoint pulls one delta to surface upstream errors as a status code. + # That delta must not also be replayed by re-iterating the stream. + class _S: + conversation_id = "conv-1" + + def __init__(self): + self.parts = ["I", "'ll", " go"] + + def __iter__(self): + return iter(self.parts) + + raw = _S() + it = iter(raw) + first = next(it, None) + assert "".join(api._Prefetched(raw, it, first)) == "I'll go" + + +# --- Python dict repr ------------------------------------------------------ + +def test_python_dict_repr_is_parsed(): + # Models emit Python repr as readily as JSON; json.loads rejects it outright. + calls, _ = extract_tool_calls( + "{'tool_calls': [{'name': 'browser_navigate', " + "'arguments': {'url': 'https://a.com'}}]}", TOOLS) + assert _args(calls)["url"] == "https://a.com" + + +def test_python_literals_are_coerced(): + calls, _ = extract_tool_calls( + "{'tool_calls': [{'name': 'browser_navigate', " + "'arguments': {'url': 'https://a.com', 'full': True, 'sel': None}}]}", TOOLS) + assert _args(calls) == {"url": "https://a.com", "full": True, "sel": None} + + +def test_python_repr_in_prose_is_not_a_call(): + assert extract_tool_calls("It printed {'a': 1} to the console.", TOOLS)[0] is None