Skip to content

feat(server): stream tool-call arguments incrementally on Chat and Responses - #2504

Merged
adrysn merged 9 commits into
mainfrom
feature/issue-2396-incremental-tool-call-args
Oct 20, 2026
Merged

adrysn merged 9 commits into
mainfrom
feature/issue-2396-incremental-tool-call-args

Conversation

@adrysn

@adrysn adrysn commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Streaming /v1/chat/completions and /v1/responses now send tool-call arguments as they are generated for Hermes / Qwen JSON, Mistral [ARGS] and the [TOOL_CALLS] [..] list, Llama 3 JSON (bare or after <|python_tag|>), and Qwen3-Coder XML. Every other format keeps the one-shot end-of-stream emission.
  • IncrementalToolCallStreamer (src/server/tool_calls/incremental.rs plus incremental/) reads the raw decoded text once, holding only a bounded tail (a partial marker or undecided name; long held values bail out and the final emission sends the rest). It skips thinking blocks and reuses the final parser's tool filter (resolve_tool_name), match_qwen_name, the shared Mistral header regex, the call caps, and a coerce_xml_param-locked string predicate.
  • Behavior change outside streaming: for Hermes / Mistral / Llama 3 (and Granite, which delegates to Hermes), an object-valued arguments is now returned as the model's own text, not a re-serialization (new json_scan module), on both streamed and non-streamed replies. This is what makes the Chat deltas concatenate to exactly the final arguments. One parity expectation changed ({"city": "Seoul"}).
  • End of generation: parse_tool_calls stays authoritative. plan_final matches streamed calls by index and name; a match gets only the unsent rest of its arguments. On Responses, .done and output_item.done carry the parsed arguments.

Tool-call id. Start carries the id the call keeps. A Mistral-list id is used only when it precedes name, validated with is_valid_model_call_id and de-duplicated as try_mistral_nemo does; otherwise the id is minted at Start, and a model id arriving later is ignored (comment on note_model_id). The final emission writes the streamed id back into the parsed calls, so Chat never mints a second id and the completed Responses object carries the streamed call_id.

Disagreement. A different count, name, or a non-prefix of the final arguments logs tracing::warn! with the request / response id. No further delta is sent for that call. The parsed call is emitted one-shot, under a fresh index when the streamed call used its slot. An unconfirmed Responses item is closed as incomplete. Named tool_choice mismatches keep their index slot and are never streamed; count_tool_calls_reaching_client still decides finish_reason.

Changes during review

  • Char-boundary panics (CRITICAL / HIGH) (f29a87dc, b06f234e). step_outer stepped one byte at a time and sliced, so any non-ASCII prose panicked the per-token callback. On Chat this happened inside the cb_state lock. step_json (ObjectStep::Invalid) and the Mistral ValueLead had the same defect. All three now step whole characters, with regression tests on Korean and emoji inputs.
  • Panic containment (b06f234e). IncrementalToolCallStreamer::feed runs under catch_unwind. A caught panic drops that fragment's events, rolls the newest call back to what the client received, stops incremental streaming for the request, and logs at error level. The final plan then sends the rest from the authoritative parse. A test injects a panic and checks the result.
  • Responses output parity (5897a60f). For a streamed request that parses tool calls, response.completed and the stored response (previous_response_id) are now built from the items actually streamed: the same rs_ / msg_ / fc_ ids and output_index order, and each message segment as its own item. Before, the final object put calls first, merged the message segments, and minted new fc_ ids. Test: responses_completed_output_matches_the_streamed_items. call_position replaces linear scans with a binary search.
  • Docs (44959fc7): the raw-arguments behavior change, panic containment and Responses parity are now in CHANGELOG.md, docs/server-features.md and docs/responses-api.md.

Measured (security review): 1.2M fuzz runs (every format, multibyte and control characters, random chunkings) found no panics after the fixes. 150k Qwen string-value runs: deltas always concatenate to valid JSON equal to the final parse. Throughput is 28 to 130 ns per 4-character token, including 1 MB prose, 1 MB argument values and 20k short calls. Held buffers are capped (256 B names and headers, 1 MiB values, 1024 calls).

Not in this PR: bare Qwen <function= calls without <tool_call> and several calls inside one <tool_call> are not streamed (the final parse still sends them). The number of Hermes blocks is uncapped, matching the final parser. Responses streaming still ignores max_tool_calls, as before.

Test plan

  • cargo test --profile test-fast -p mlxcel --lib with server::tool_calls::, server::routes::, server::responses, server::muse_atem, server::router_front
  • cargo clippy -p mlxcel --lib --tests -- -D warnings, cargo fmt --check
  • Unit tests feed each case per character and under other chunkings and compare against parse_tool_calls: Hermes, two-call Mistral [ARGS], Qwen3-Coder string + int (deltas before </parameter>), named tool_choice, thinking, unknown tools, Mistral-list ids, bounded holds.
  • Route tests drive the real handlers through scripted_streaming_for_route_tests (the muse_atem tests replay the route by hand) and show several argument chunks for one call on both endpoints.
  • Not run here (no Metal or CUDA): cargo test --workspace --features metal,accelerate and the integrated Qwen3-Coder-30B-A3B-Instruct-4bit OpenAI SDK check.

Resolves #2396

The streaming routes send a tool call only after generation ends, so agent clients see a long gap and then one burst. IncrementalToolCallStreamer reads the raw decoded text as it arrives and reports Start { index, name, id } and ArgsDelta { index, delta } events for Hermes JSON, Mistral [ARGS] and the [TOOL_CALLS] JSON list, Llama 3 JSON (bare or after <|python_tag|>), and Qwen3-Coder XML. It skips thinking blocks, applies the final parser's tool filter, name rules (match_qwen_name, the shared Mistral header regex) and call caps, and holds only a bounded tail between fragments. plan_final reconciles the streamed calls with parse_tool_calls, which stays authoritative, and logs a warning on any disagreement.

The JSON-bodied parsers now keep an object-valued arguments member as the raw model text instead of re-serializing it (new json_scan module), so the streamed deltas concatenate to exactly the final arguments on both the streamed and non-streamed paths.

Validated with cargo test server::tool_calls:: (per-character and chunked cases compared against parse_tool_calls) and clippy --lib --tests.

Refs #2396
Both streaming routes now feed each decoded token to the incremental streamer and forward its events as they occur: Chat Completions sends tool_call_start plus several function.arguments deltas, Responses opens the function_call item (closing an open reasoning or message item first) and sends response.function_call_arguments.delta events.

At end of generation parse_tool_calls stays authoritative. A streamed call that agrees with the parse keeps its call id (the final emission never mints a second one; the id is also written into the completed Responses object) and gets only the unsent rest of its arguments. Responses closes it with .done and output_item.done carrying the parsed arguments. On disagreement the route logs a warning with the request id, sends no further delta for that call, and emits the parsed call in one shot; a streamed Responses item the parse does not confirm is closed as incomplete. Formats the streamer does not cover keep the one-shot emission, and count_tool_calls_reaching_client still decides finish_reason.

Route-level tests drive the real handlers through the scripted streaming provider one character per token.

Refs #2396
Describe the streamed formats, the chunk shapes on Chat Completions and Responses, the id rule (a Mistral list id is used only when it precedes the name, otherwise the id minted at the first chunk is kept), the disagreement path, and the raw-text arguments the JSON formats now return. Add a CHANGELOG entry.

Refs #2396
Pin the path where the streamer walks past the first </tool_call> as plain text and re-enters on the second <tool_call>, and note in the docs when Start fires for bare Llama 3 JSON and Qwen3-Coder.

Refs #2396
@adrysn adrysn added status:review Under review type:enhancement New features, capabilities, or significant additions priority:medium Medium priority area:inference Generation, sampling, decoding (incl. speculative, DRY) labels Oct 20, 2026
The incremental streamer's visible-text scan advanced one byte at a time and sliced the input at each position, so any non-ASCII character in prose before or between tool calls (Korean, accented text, emoji) panicked with a char-boundary error inside the per-token callback of every streamed request that declares tools, on both Chat Completions and Responses.

The scan now advances by the width of the current character. Every opener it looks for starts with an ASCII byte, so no marker can begin mid-character. Regression tests feed Hermes, Qwen3-Coder and Mistral replies with Korean text under every chunking and compare against the final parse.

Validated with the server::tool_calls:: and server::routes:: test targets, clippy with -D warnings, and cargo fmt --check.

Refs #2396
Fuzzing every streamed format with multibyte fragments under per-character and random chunkings found two more char-boundary panics: the JSON call reader consumed only the first byte of a non-ASCII character the object walker rejected (`<tool_call>{"arguments"🎉: ...`), and the Mistral bracket reader did the same for a value starting with a multi-byte scalar (`[TOOL_CALLS]f[ARGS]日`). The next step then sliced mid-character. Both now consume the whole character.

`IncrementalToolCallStreamer::feed` now runs under `catch_unwind`, so any future scanner panic cannot unwind through the per-token callbacks or poison the Chat callback mutex. A panicked fragment's events are dropped, the newest call's sent text is rolled back to what the client received, and the streamer stops; the end-of-generation plan then sends the unsent rest from the authoritative parse.

Validated with 1.2M fuzz runs without a panic, the tool_calls, routes, responses, muse_atem and router_front suites, clippy, and fmt.

Refs #2396
@adrysn

adrysn commented Oct 20, 2026

Copy link
Copy Markdown
Member Author

Security and performance review

Fixed in b06f234

  • HIGH: two more char-boundary panics on untrusted model output. The JSON call reader consumed one byte of a rejected non-ASCII character (<tool_call>{"arguments"🎉: ...), and the Mistral bracket reader did the same for a multi-byte scalar value ([TOOL_CALLS]f[ARGS]日). Both now consume the whole character. Regression tests added.
  • HIGH: IncrementalToolCallStreamer::feed now runs under catch_unwind. On a panic it drops the fragment's events, rolls the newest call back to what the client received, and stops streaming. The final plan sends the rest from the authoritative parse, so a panic can no longer poison the Chat cb_state mutex or unwind through the callback.

Verified, no change needed

  • Fuzz: 1.2M runs over all formats with multibyte, control and marker fragments under per-char, whole and random chunkings, with no panic after the fix. Qwen string fragments always concatenate to valid JSON equal to the final parse.
  • Throughput (test-fast, 4-char tokens): 28-130 ns per token across 1 MB of prose, 1 MB argument values (Hermes, Qwen, Mistral), 20k short calls, 1 MB of whitespace and 1 MB of [ nesting. This is far below the per-token decode cost, so the per-char Vec in step_outer is not material.
  • Memory: names and headers are capped at 256 B (Mistral, Qwen), JSON names at 512 B, ids at 128 B, and held values at 1 MiB. Qwen and Mistral calls and params are capped at 1024. The JSON scanner is iterative with a counter, so deep nesting cannot overflow the stack.

Remaining (LOW, not fixed)

  • Hermes <tool_call> count is uncapped, as in the final parser. ResponsesToolCallStream::item_for and plan_final scan calls linearly, which is O(calls^2) over a reply with thousands of calls. The cost is bounded by output length. Calls are pushed in increasing index order, so a binary search would fix it.

With tool-call streaming, a call that starts mid-prose closes the open message item and later text opens a second one, but response.completed was still assembled by build_response_object: one combined message, function calls first, and freshly minted fc_/msg_/rs_ ids. Clients that accumulate by output_index or item id disagreed with the completed response.

ResponseStreamSender now records every output_item.done it sends. For a streamed request that parses tool calls, the completed object (and the copy persisted for previous_response_id) uses those items in output_index order: the same ids, message segments as separate items, function calls with the streamed fc_ id, call_id and parsed arguments, an unconfirmed call kept as incomplete, and a one-shot call under the id its events used. output_text is the concatenation of those messages. Non-streaming responses are unchanged.

Streamed-call lookups by index now use a binary search, since calls are started in increasing index order.

A new route test streams prose, a call, then prose through /v1/responses and asserts the streamed items equal response.completed's output; it failed before this change.

Refs #2396
…es output parity

Bring the #2396 docs in line with the follow-up fixes. Hermes, Granite (via Hermes), Mistral and Llama 3 now return an object-valued arguments as the model's raw JSON text, streamed or not, which changes the previous re-serialized form; the CHANGELOG and server-features now say so. The docs also describe the char-boundary fixes and the catch_unwind fallback to the final parse, and the streamed /v1/responses response.completed output, which mirrors the streamed items (separate message segments, same ids and order, same object stored for previous_response_id).

Validated with the server::tool_calls::, server::routes::, server::responses and server::router_front lib tests, clippy -D warnings and cargo fmt --check.

Refs #2396
@adrysn adrysn added status:done Completed and removed status:review Under review labels Oct 20, 2026
@adrysn
adrysn merged commit 8f30824 into main Oct 20, 2026
80 of 82 checks passed
@adrysn
adrysn deleted the feature/issue-2396-incremental-tool-call-args branch October 20, 2026 17:54
adrysn added a commit that referenced this pull request Oct 21, 2026
Phase B of #2397, after phase A (#2520): automatic compaction on `/v1/responses` through `context_management`, the output cap at the remaining context, and the streamed `mlx.compaction.*` events.

## Behavior

- `context_management: [{"type":"compaction","compact_threshold":N}]` on `/v1/responses` and `/responses`. The rendered input (chained `previous_response_id`/`conversation` history plus `input`, capsules expanded) is counted on the `/v1/responses/input_tokens` path. At the threshold, phase A's `compact_history` runs with the same `ResponsesCompactionBackend` and default `keep_tokens`, and generation runs on instructions + summary + retained turns. Summary budget is upstream's automatic one: context/16, clamped to 128..1024.
- `output` starts with the `compaction` item. `usage` counts the final generation only, as upstream does.
- Stored shape: the response stores the instructions as its input and the capsule as `output[0]`. The next `previous_response_id` hop therefore assembles instructions + capsule + tail, replays the summary, and compacts again only at the threshold. A `conversation` transcript appends the new input plus output.
- Streaming: `mlx.compaction.started` (`input_tokens`) and `mlx.compaction.completed` (`input_tokens_before`/`input_tokens_after`), in upstream's payload shape plus `sequence_number`, come right after `response.in_progress`. The capsule follows as `output_item.added`/`done` at `output_index` 0, and generated items start at 1. `response.completed` lists the same capsule once, including the tool-streamed path from #2504.
- Without the field, or with `[]`/`null`: no extra count and no cap, and the response is unchanged.

## Decisions to review

- **Threshold is `>=`**, as in upstream and OpenAI's "reaches", although the issue says "exceeds". Compaction also fires below the threshold when the input leaves no room for output.
- **Output cap and `incomplete` apply only to `context_management` requests.** Today every `length` stop reports `completed`, and with `--context-shift` generation may run past the window. A general cap would change both. `incomplete_details.reason` is `max_output_tokens`, the OpenAI and upstream value, not a literal `length`. The streamed reply then ends with `response.incomplete`. As in OpenAI's shape, `completed_at` is null and the cut-off message item is `status: "incomplete"`, in the final object and in its streamed `output_item.done`. A limited reasoning budget above the capped limit is lowered to one token below it. A server with no free slot returns 503 before any count.
- **Failures are HTTP errors before any output, on both arms.** Compaction runs before the SSE stream opens, because `ResponseStreamSender` sends from the blocking task. A streamed overflow therefore returns 400 `exceed_context_size_error`/`context_length_exceeded`, as a stream admission overflow does today. The cost is that the two events carry counts, not live progress.
- **No safe split** (one turn, or one unfinished tool group): the request proceeds uncompacted while the input fits, as upstream does. Otherwise it returns `context_length_exceeded`. Every other compaction failure keeps phase A's error.
- A malformed `context_management` returns 400 `invalid_request_error`: not an array, more than one entry (duplicates included), a type other than `compaction`, or a threshold that is not an integer from 1 to 100000000.

## Test plan

- [x] `cargo test --profile test-fast -p mlxcel --lib` filters: `responses_auto_compact` (15, including the reasoning-budget clamp, the busy-server 503, the `conversation` path and a streamed request without `context_management` that keeps its pre-PR event sequence), `responses_compact` (28), `server::routes::` (508), `server::responses` (74), `server::types` (107, before the review fixes), `streaming_responses` (4)
- [x] `cargo clippy -p mlxcel --lib --tests -- -D warnings`, `cargo fmt --check`
- [ ] Known limit: a streamed request that compacts runs the summary pass on the request task, and a client disconnect does not cancel it.
- [ ] Unverified (no Metal or CUDA here): a real model such as `mlx-community/Qwen3-30B-A3B-4bit`. Media and video prompts with `context_management` have no test: each count takes a worker slot, and video substitution runs again after compaction.

Out of scope, per the issue: Chat Completions and `/v1/messages` auto-compaction, the Codex `compaction_trigger` input, and multi-pass summarization. Follow-up: #2523.

Closes #2397
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:inference Generation, sampling, decoding (incl. speculative, DRY) priority:medium Medium priority status:done Completed type:enhancement New features, capabilities, or significant additions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Stream tool-call arguments incrementally on Chat and Responses

1 participant