Support OpenAI tool calling (function calling) - #13
Open
dylansantwani wants to merge 8 commits into
Open
Conversation
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.
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.
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 (`<some_tool arg="value">`, the shape several agent frameworks train on) and `<tool_call>` 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.
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.
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.
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.
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.
- 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.
|
use my project instead https://github.com/FANATFANATA/DanyAPI |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
A request carrying OpenAI-style
toolshad them silently discarded.toolsisn't a field onChatCompletionRequest, so pydantic dropped it and DeepSeek never learned the tools existed.The visible failure isn't an error — it's worse. The model, asked to use tools it can't see, narrates the attempt in prose:
…then invents tool names that were never offered, and writes out imagined results as if the calls had run. A client driving a tool loop gets text where it expected
tool_calls, and the run quietly goes off the rails.The approach
DeepSeek's web chat has no native function-calling channel, so this emulates one:
tools/tool_choice, plustool_calls/tool_call_id/nameon messages.tool_callswithfinish_reason: "tool_calls". Accepts fenced JSON (what the contract asks for), bare JSON, and OpenAI-shaped wrappers.tool_call.toolsstream token-by-token exactly as before.DEBUG_REQUESTS=1— logs each request's message roles and offered tool names, which is what distinguishes "the server ignored my tools" from "my client never sent any."Worth a look during review
The ReAct-prose salvage (
Action:/Action Input:→ a real call, including when the model drops a namespace prefix) is the pragmatic part. A prompt isn't a decoder constraint, so the model does sometimes ignore the contract, and parsing the prose is what makes the real-world case work. If you'd rather keep the parser strictly JSON-only, I'm happy to drop it into a follow-up — it's self-contained in_extract_react_calls.The README section is deliberately explicit that this is emulated and best-effort, so nobody reads it as parity with the paid API.
Testing
tests/test_tool_calling.py— 14 tests covering each reply shape, hallucinated-tool rejection, prompt replay of prior calls and results,tool_choice: none, and the endpoint in both streaming and non-streaming modes. They fake the upstream client, so they run without a DeepSeek session:Also verified end to end against the live bridge with a real agent (hermes): it issued a genuine
read_filecall and returned the file's actual contents, where before it hallucinated them.🤖 Generated with Claude Code