Bug: tool calls are dropped against servers that send null in continuation deltas (id/name overwritten, every call becomes UNKNOWN_TOOL) #161
zhoukuncheng
started this conversation in
General
Replies: 2 comments 1 reply
|
Appreciate the detailed information on Delta Delta Executor. I like posts that explain a technical topic without making everything sound overly complicated, and this one does a good job of keeping things understandable. |
0 replies
|
Confirmed and reproduced with DeepSeek-V4-Flash via SSE streaming — Applied the fix locally in - if (call.id !== undefined) block.callId = call.id
- if (call.function?.name !== undefined) block.name = call.function.name
+ if (call.id !== undefined) block.callId = (block.callId ?? '') + call.id
+ if (call.function?.name !== undefined) block.name = (block.name ?? '') + call.function.name |
1 reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
TL;DR
dsh-llm-deepseekloses a streamed tool call'sidandnameagainst any server that spells "unchanged" as an explicitnullinstead of omitting the key. Every tool call then reaches the executor unnamed and is rejected asUNKNOWN_TOOL, so the harness serves conversation but cannot complete a single tool call. One-line-per-field fix, plus tests, on a branch linked below.I read
CONTRIBUTING.md— external PRs are not being accepted right now, so this is a report rather than a pull request. The branch is there purely so the patch and its tests are cherry-pickable; please treat it as attached evidence, not a submission.Environment
deepseek-harnessat47f94385(0.1.0-rc.5), built from source, Node 22.22.1headless, adapter@deepseek-ai/dsh-llm-deepseek(deepseek-officialroute)deepseek-ai/DeepSeek-V4-Flash-0731, reached by settingbaseURLin a--patchoverlay. It speaks the DeepSeek dialect natively —thinking: {"type": "enabled"}andreasoning_effortare accepted, andreasoning_contentcomes back separated — so nothing about the request side is unusual.Plain chat works. Anything that needs a tool does not.
Symptom
Every tool call fails identically, whatever the tool:
{"type":"tool/result","data":{"message":{"content":[{"type":"tool-result", "content":[{"type":"text","text":"Error: unknown tool \"\""}],"isError":true}]}, "error":{"name":"ToolNotFoundError","code":"UNKNOWN_TOOL"}}}The corresponding
tool/callevent shows well-formedargumentswith an emptynameand an emptycallId:{"type":"tool/call","data":{"turn":1,"step":1,"callId":"","name":"", "arguments":"{\"file_path\": \"/…/package.json\", \"limit\": 1}"}}The failure mode matters more than the error.
unknown tool ""names nothing the model can act on, so it retries the same call a few times and then answers from invention. Asked to print the first line of apackage.json, the run returned a fabricated file plus a footnote that "repeated tool-invocation errors prevented those calls from executing" — a wrong answer delivered with a plausible excuse. From the outside this reads as a model that ignores its tools, not as a decoding defect, which is why it is worth a report even though the fix is small.Root cause
packages/llm/llm-deepseek/src/translate.ts(in thedelta.tool_callsloop):A tool call carries its identity once: the first delta for a wire
indexholdsidandfunction.name, and later deltas for that index carry only anargumentsfragment.!== undefinedacceptsnullas a value, so on a server that sends explicit nulls each continuation delta overwrites the identity the first delta captured. By the last fragment both arenull, andcloseBlockemitsid: CallId('')/name: ''.WireToolCallDeltaintypes.tsdeclaresid?: stringandname?: string, so the type says the key is simply absent. That is true of the official API and not of every server implementing the same schema — and it is what makes theundefinedcomparison look sufficient. (WireDelta.reasoning_contentin the same file is alreadystring | null, so the nullable spelling is not new to this wire.)Wire evidence
Live capture from the endpoint above, one tool call, trimmed to the fields at issue:
I do not think the server is at fault here. Both fields are optional in the chunk schema, and a server that serializes its response model without excluding unset fields emits every unset optional as
null. The rest of the same delta shows exactly that:"role":null,"content":null,"reasoning_content":nullsit beside the nulled tool-call fields. Absence andnullare two spellings of the same statement, and the adapter currently understands only one of them.Reproduce
Point the
deepseek-officialroute at any OpenAI-compatible server that serializes unset optionals asnull(SGLang does by default), then ask for anything requiring a tool:dsh --profile headless --patch ./selfhosted.cordis.patch.yml \ "Run 'git rev-parse --short HEAD' and reply with only that hash."Before the fix:
Error: unknown tool ""on every step, then an invented answer. After: the hash.For a unit-level repro with no server at all, feed
translatea first delta carryingid/namefollowed by continuations whoseidandfunction.namearenull; the assembledblock-endcomes back withid: ''andname: ''.Fix
Record both spellings in the wire type and assign only on a non-nullish value:
Branch: https://github.com/zhoukuncheng/deepseek-harness/tree/fix/tool-call-null-delta-identity (single commit
21564bce5e, cherry-pick freely — no attribution needed).It contains the two guards,
WireToolCallDeltawidened tostring | null | undefinedwith the JSDoc stating that omission andnullboth mean "unchanged", two unit cases, and a bilingualbug-fixAgent Note perAGENTS.md.The two cases cover a single call whose continuations null out both fields, and two parallel calls whose continuations arrive interleaved and out of index order. Both fail against the previous guard and pass after it, so they hold the behavior rather than describe it. The existing cases for the omitted spelling are untouched and still pass, which is what keeps the official-API path covered — behavior there is unchanged, since an omitted key is nullish under either guard.
Checks run on the branch:
pnpm run test packages/llm/llm-deepseek153/153 (151 before, +2),pnpm run typecheck,pnpm run lint, andpnpm run doc-syncall clean, plus thelefthookpre-commit gates.Two smaller things found alongside
Neither is a bug, but both cost me time and might be worth a line in the docs:
Running the source entry from another working directory fails at import:
node --import tsx/esm <repo>/apps/cli/src/bin.tsfrom outside the repository root givesSyntaxError: The requested module '@deepseek-ai/cordis' does not provide an export named 'FiberState'. Since the invoking directory is the workspace root and there is no--cwd, usingdshagainst another project from a source checkout means invoking the builtapps/cli/lib/bin.js.pnpm dshworks, but only from the repository root.Launcher flags must precede app flags, which the reference does state — but the failure is confusing in one direction:
dsh web --port 3099 --patch ./x.ymlreportserror: unknown option '--patch', because--portalready started the app's arguments and the web app then rejects--patchas its own.dsh web --patch ./x.yml --port 3099is fine.Thanks for open-sourcing this — the plugin tree and the session log made the root cause a five-minute read rather than a guessing game.
All reactions