dsh-llm-deepseek drops tool call id/name when provider streams explicit nulls (every tool fails: unknown tool "")

[salaheddine-11]

Summary

Tool calls streamed from OpenAI-compatible providers (proxies) that emit explicit null continuation frames lose their id and name in @deepseek-ai/dsh-llm-deepseek's SSE translation, so every tool call fails with Error: unknown tool "" (ToolNotFoundError). The agent loops ~10 steps and exits 0 with an empty final answer.

Environment

• dsh 0.1.1-rc.2 (npm global install), Node 24.18.0, Windows 10
• Provider: OpenAI-compatible endpoint at https://opencode.ai/zen/go/v1 (model deepseek-v4-flash)

Symptom

[tool/call] {"callId": "", "name": "", "arguments": "{\"command\": \"ls\", \"description\": \"...\"}"}
[tool/result] ... "Error: unknown tool \"\"" (ToolNotFoundError, UNKNOWN_TOOL)

The arguments arrive intact; the identity fields (callId, name) are both empty.

Root cause

The proxy (standard OpenAI streaming behavior, also used by many gateways) emits:

frame 1: {"index":0, "id":"call_…", "type":"function", "function":{"name":"get_weather","arguments":""}}
frame 2: {"index":0, "id":null, "type":"function", "function":{"name":null,"arguments":"{\""}}
frame 3: {"index":0, "id":null, "type":"function", "function":{"name":null,"arguments":"city…"}}

In dsh-llm-deepseek/lib/index.js (translate function):

if (call.id !== void 0) block.callId = call.id;
if (call.function?.name !== void 0) block.name = call.function.name;

null !== void 0 is true, so frame 2 overwrites the good id/name captured in frame 1 with null. closeBlock then yields id: CallId(block.callId ?? "") and name: block.name ?? "" → both empty. Arguments survive because they only accumulate (block.text += fragment).

Verified with raw probes: non-streaming, streaming, and streaming-with-thinking responses from the proxy all contain the name in the first frame; only dsh's fuse drops it.

Suggested fix

Treat null as absent (one-line change, loose inequality covers both undefined and null):

if (call.id != null) block.callId = call.id;
if (call.function?.name != null) block.name = call.function.name;

Same guards likely needed in any other adapter that fuses OpenAI-style streamed tool calls (checked 0.1.1-rc.2: only dsh-llm-deepseek has this pattern).

Workaround

Local patch of the installed dsh-llm-deepseek/lib/index.js (2 lines above); note that a server restart is required for already-running web instances (module loaded in memory).

[argszero]

Verified against rc.2 — your diagnosis is exact, and this is the newest member of a documented bug family the repo has been fighting for months.

1. Source confirmation

packages/llm/llm-deepseek/src/translate.ts:159-160:

if (call.id !== undefined) block.callId = call.id
if (call.function?.name !== undefined) block.name = call.function.name

null !== undefined is true, so your frame 2 ("id": null, "name": null) overwrites the good identity captured in frame 1. The arguments survive because they only accumulate (block.text += fragment, :161-162), but closeBlock yields CallId(block.callId ?? '') (:166) → empty id/name → ToolNotFoundError "unknown tool """. Everything you describe reproduces from these four lines.

2. Family context: this is the wire identity-loss family's newest member (report ~12), and the first at the adapter boundary

The same shape has been filed at least a dozen times across layers — serializer null/""-delta (#1713, #2090, #2169, #2343, #2540, #2855, #2997, #4265, #4365, #4370), assembler index-fusion (#4427), tool-call block re-keying (#4410), and now SSE translation (#4671). The through-line: every place two streamed deltas fuse into one block must treat identity as first-write-wins and null-skip. Your one-line loose-comparison fix (call.id != null) is the standard correction — and note the sibling llm-pi-ai adapter already does the defensive variant (known.name !== undefined && known.name.length > 0, stream.ts:172), so llm-deepseek is the outlier, not the norm.

3. One layer deeper: first-frame facts

The OpenAI-compatible streaming contract puts a tool call's id and name only in the first frame of that tool-call group; continuation frames carry only arguments fragments. So the robust fix is not just "skip null" but "capture identity once, never overwrite":

// identity fields are first-frame facts; later frames (null or otherwise)
// must never overwrite them
if (block.callId === undefined && call.id != null) block.callId = call.id
if (block.name === undefined && call.function?.name != null) block.name = call.function.name

This also defends against gateways that re-emit a differing (not just null) value mid-stream — the same class of proxy wildness behind #4618/#4620 (interleaved reasoning_content/tool-call frames). The first-write-wins variant is strictly safer than last-write-wins-with-null-skip and costs nothing.

4. Regression test

translate.ts's spec should add a three-frame case: frame 1 carries {index:0, id:"call_x", function:{name:"get_weather", arguments:""}}, frames 2-3 carry {index:0, id:null, function:{name:null, arguments:"{\"co…"}} — assert the emitted block keeps call_x/get_weather while the arguments accumulate. That locks both the null-skip and the first-frame-fact behavior against the next gateway quirk.

[Jstn-1g]

I built a bounded reference implementation against official master at b150a551b8d465e31e418e1b2eaf5e79bbb7d28e.

Thank you @salaheddine-11 for the exact failing frames and source diagnosis, and @argszero for independently confirming the path and identifying the broader first-frame identity family.

The patch models compatible-gateway id / function name continuation fields as nullable and treats only null / undefined as absent. Established identity therefore survives while argument fragments continue to accumulate. This deliberately preserves the adapter's existing latest-non-null behavior: it does not freeze conflicting non-null values, ignore empty strings, or synthesize missing identity.

Evidence:

• focused three-frame translator regression: 31/31 tests, 100% statements/branches/functions/lines for translate.ts;
• keyless assembled ACP test over local SSE: the named tool executes, the provider follow-up retains tool_call_id, and durable tool/call + tool/result events retain the original call id;
• bilingual adapter docs and implementation note: full documentation suite 28/28;
• staged lint, whitespace, translation-pairing, Host build, and client contract typecheck all pass.

Reference branch: https://github.com/Jstn-1g/deepseek-harness/tree/reference/discussion-4671-sse-null-identity

Exact commit: Jstn-1g@138691e

This is a reviewable reference implementation, not a claim of upstream adoption.

[nokkies]

Confirmed and reproduced through the real SSE path, with the frame sequence exactly as you describe it. Your root-cause line is precisely the bug:

if (call.id !== void 0) block.callId = call.id;

null !== undefined is true, so the continuation frame's explicit null overwrites the identity captured on frame 1.

Fix

Both spellings — omitted and null — mean "no update", so neither may overwrite:

if (call.id !== undefined && call.id !== null) block.callId = call.id
const streamedName = call.function?.name
if (streamedName !== undefined && streamedName !== null) block.name = streamedName

Worth pairing that with the wire type, so the guard is honest rather than defensive:

id?: string | null
function?: { name?: string | null; arguments?: string }

That file already models reasoning_content?: string | null for the same reason — the wire genuinely sends null there too. Declaring it is what stops the next reader assuming !== undefined is sufficient.

The half that actually kills the turn

Worth separating the two symptoms, because only one of them is fatal.

#4611 covers an empty callId reaching the session log, and a fix there (synthesizing call-<index> when the id is empty) would give this call a usable id. It would not fix your bug. The empty name is what makes dispatch report unknown tool "", and no id-side fallback restores a name. That is why the loop burns its steps and the run exits 0 with an empty answer.

So this needs the adapter fix above regardless of what happens on #4611 — the identity has to survive where it is lost, not be reconstructed downstream.

[argszero]

Thanks — the id/name split is an important correction to my framing, and it checks out at source. ToolNotFoundError is built from the tool name (core/tools/src/index.ts:504: unknown tool "${toolName}"), so the empty name is the turn-killer; a synthesized id from #4611 cannot restore it. My reply treated "empty identity" as one failure mode; it is two, with different fix boundaries.

One detail the split exposed, on top of your wire-type point: the null-overwrite corrupts the accumulator state, not just the emitted frame. With function: { name: null } on the continuation frame, line 160's block.name = call.function.name sets block.name = null — which then does two things:

1. Line 167's ...block.name !== undefined ? { name: block.name } : {} is true for null, so every subsequent delta for that block emits name: null — a type violation against the harness protocol (types.ts:316 declares name?: string), even though the assembler happens to tolerate it via a falsy check (assembler.ts:70 if (chunk.name)).
2. closeBlock line 72 (name: block.name ?? '') turns it into '' — the exact empty name that dispatches as unknown tool "".

So the null-skip guard fixes both the wire violation and the closeBlock poisoning in one line. (My capture-once spelling — block.name === undefined && value != null — also works; yours is simpler and mirrors the reasoning_content?: string | null precedent at types.ts:140, which as you note is exactly the wire truth.)

Third line of defense, worth noting for the family: the assembler boundary (assembler.ts:68-72) already does capture-once via the falsy if (chunk.name) check — so identity loss is defended at three layers (adapter capture → accumulator state → assembler merge), and dsh-llm-deepseek was the only one missing the first. The regression test should pin the accumulator state too (frame 1 good value, frame 2 null, assert the emitted deltas carry the name and closeBlock preserves it), so the state-poisoning path can't regress silently even if the assembler later tightens its type.

[nokkies]

Taken, and pinned. You are right that the accumulator state is the part my test was not actually proving.

I had been asserting only the assembled block, which passes for the wrong reason: the assembler merges names through a falsy if (chunk.name) check, so it absorbs name: null today. The final value is therefore correct while the emitted deltas are not — and if the assembler ever tightened that check to a !== undefined test, the protocol violation would surface with nothing in the suite objecting.

The test now consumes the raw stream instead of the assembled result, and asserts per delta:

for (const delta of deltas) {
  expect(delta.id).toBe(CallId('call_abc'))
  expect(delta.name).not.toBeNull()
}
expect(deltas.map(d => d.name)).toEqual(['get_weather', 'get_weather', 'get_weather'])

with the block-end block checked in the same pass. So frame 1's identity has to survive on every subsequent delta, not merely at close.

Your three-layer framing is the right way to hold this family: adapter capture → accumulator state → assembler merge, with only the first missing here. Worth adding that the third layer is defence by coincidence rather than by contract — if (chunk.name) rejects null because it also rejects '', which is a different intent that happens to cover this. That is exactly why the second layer needs its own test rather than relying on the outcome staying correct downstream.

The fix itself is unchanged; this is coverage it always deserved and did not have.

[weijiafu14]

Adding the cross-adapter measurement to the excellent id/name-split analysis above:

We fault-injected the real pi-ai openai-completions accumulator (client/SSE parser/assembly unmodified; only the wire bytes synthetic) with this exact delta family — empty-string id/name continuations, late identity, parallel calls with no ids at all (index-keyed interleave), and index-less id-keyed deltas: all assemble correctly. The semantics that make it immune: blocks are keyed by index first, and the first non-empty id/name wins — a later empty/null value cannot overwrite. Re-verified on latest pi-ai 0.84.4. Per-case verdicts: accumulator-experiment.mjs / results; live two-call turn with distinct non-empty ids on real api.deepseek.com: provider-threads-e2e.json.

That measured rule (index-first keying + first-non-empty-wins) is a ready-made semantic for the llm-deepseek fix — several sibling reports converge on it (#3090 has the deepest diagnosis; #2674 the session-history angle). Scope honesty: this doesn't patch llm-deepseek; until it lands, a custom declaration routed through llm-pi-ai rides the immune accumulator (recipe). One known limit of that path for fairness: two calls sharing one index DO merge there — filed upstream as earendil-works/pi#8861.