Repository navigation
[https://nvbugs/6781271][fix] Responses API: agent-client fixes and disaggregated serving - #19883
Merged
JunyiXu-nv merged 55 commits intoOct 26, 2026
Merged
Conversation
JunyiXu-nv
force-pushed
the
dev-junyix-fix-responses-api
branch
from
October 6, 2026 08:48
210fedd to
97fd4b2
Compare
JunyiXu-nv
added
the
api-compatible
This was referenced Oct 7, 2026
…e reports
glm4 and glm47 parsed a document differently depending on how it was
streamed. A call that arrived in the final delta could be lost on the chat
path, a declared string was re-typed ("99797" arrived as 99797, true as
"True"), a value quoting </arg_value> was cut there, and markup the model
wrote around a tool name ended up inside the name.
- glm47 derives from glm4 and overrides only how a call body splits into
name and argument text. Both parse paths run each complete
<tool_call>...</tool_call> through one function, so streaming reports
the calls and the text detect_and_parse returns, however the output is
split. BaseToolParser.streaming_matches_whole_parse declares that
property; it is False by default and True for these two parsers.
- A call is reported once its </tool_call> arrives, as one item with the
name and the complete JSON arguments; text after a call waits for the
next increment. finish() reports the complete calls the parser still
holds and releases the rest as text.
- A call is delivered only when its argument text is complete key/value
pairs; otherwise its markup is released as text, as detect_and_parse
reads it. A value runs to the first </arg_value> followed by the next
<arg_key> or the end of the call, so a value may quote that tag.
- Declared strings are delivered verbatim. Other declared types are
decoded as JSON, JSON with escaped quotes or a Python literal, and an
undeclared argument only as strict JSON. A result that is not strict
JSON (NaN, 1e309) keeps the text, and a repeated key keeps its first
value.
- A name with a group qualifier missing or extra, or with markup fused
into it, resolves to the declared tool (BaseToolParser.resolve_tool_name)
and is typed by that tool's schema; prose that mentions a tool never
resolves. A <tool_call> restarted inside a call is parsed from the
restart, and the abandoned prefix is released as text.
- GLM-4 keeps the text between and after calls, as GLM-4.7 does, and
argument text is scanned in linear time.
On the chat path a GLM tool call now arrives as one delta when it
completes, instead of its name first and its arguments in pieces.
The GLM tests are one parity class run for both parsers: typing and
argument-text tables, multi-call responses with malformed, restarted and
cut-off calls, and name repair, each checked whole and over every
chunking. The existing streaming tests that asserted an early name
announcement now assert the single complete item.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
There was no reasoning parser for the GLM family. GlmReasoningParser, registered as glm, glm45, glm47 and glm_moe_dsa, runs one scanner for streaming and whole text, so the two splits agree: - glm, glm47 and glm_moe_dsa start inside the block, which the GLM-4.7 and GLM-5 templates prefill; with enable_thinking false those templates close it in the prompt, and the parser starts outside. glm45 opens the block at a <think> at the start of the output, as GLM-4.5/4.6 write it. - The block ends at the first </think>, or at a <tool_call> opened before it, so a call made without closing the block reaches the tool parser. A later <think> is text. - A </think> before any content, or directly before the first <tool_call>, closes the block again and is dropped. Text from the first <tool_call> on is kept verbatim, so tool arguments that quote the markup survive. The partial-marker holdback is one module-level helper, shared with the Gemma4 and Kimi K3 parsers, which each had their own copy. The CLI reference lists the new keys. The tests are a table checked whole, at small chunk sizes and over every three-way split, plus a seeded stream-versus-whole comparison and a check that deepseek-r1 is unaffected. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
… external access
A client may attach the web_search tool with external access explicitly off.
Codex does exactly that on every turn:
{"type": "web_search", "external_web_access": false}
The guard added for the Responses surface refused any request carrying a
web_search tool, so this locked Codex out of the server entirely: every turn
came back 400, and because Codex treats a 400 as retryable it burned its retry
budget and reported "We're currently experiencing high demand" instead of the
actionable message.
The guard exists because silently dropping the tool lets the model answer from
memory while the client believes a live search was folded in - a wrong answer
the client cannot detect. That reasoning does not apply when the client has
said up front that it does not want the open web: there is no live result to
withhold and nothing it could be misled about. Refusing there is a false
positive, not a safety property.
So only refuse when the request actually asks for external search. A tool with
external_web_access absent or true is unchanged and still fails loudly.
Neither the model catalog nor `[tools] web_search = false` in Codex's own
config suppresses the tool, so there was no client-side workaround short of
editing the catalog by hand.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
(cherry picked from commit 6d4d5a4)
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
/v1/responses was available only on an aggregated server, so agent CLIs that speak the Responses protocol - Codex among them - could not be pointed at a disaggregated deployment at all. Route it the same way /v1/chat/completions already goes: the orchestrator splits the request into a context phase and a generation phase, relays the KV-cache handle between the two workers, and streams the generation worker's bytes back untouched. All Responses semantics stay in the worker's existing handler, so there is one implementation of the event lifecycle rather than a second one in the proxy that would drift against it. The orchestrator has no tokenizer and no engine and still needs neither. What the split needed: DisaggregatedParams / conversation_params / prompt_token_ids(_b64) on ResponsesRequest, mirroring ChatCompletionRequest. The generation worker uses the relayed token ids instead of rendering the chat template again - not only to save the work, but because a template that varies per render would produce a prompt the context worker never prefilled, leaving the relayed KV cache describing different text. A handoff carrier on ResponsesResponse. Completions and Chat Completions hang it on choices[0]; a Responses response has no choices, so it carries the same fields at the top level. Three accessors in the service absorb that difference and everything downstream is unchanged. finish_reason alongside status. The orchestrator decides whether a generation phase is pending by looking for "length"/"not_finished", and status maps both onto "incomplete". finish_reason_mapping also raised outright on "not_finished" - which is exactly what a context worker reports, being capped at one token - so every context request failed with a 500 before this. Context-only responses alone carry those fields. They are read by the orchestrator and never by a client: ctx_info_endpoint names an internal address, and prompt_token_ids would append the whole prompt to every reply. ctx_usage is converted rather than assigned. DisaggregatedParams types it as UsageInfo (prompt/completion/total) and a Responses response reports ResponseUsage (input/output/total): the same numbers under names that do not line up, so a straight assignment survives locally and then loses every count when the generation worker validates the request. A context-only request that finishes without a generation phase is replayed as Responses events. The completions "[DONE]" sentinel does not exist in this protocol, so a streaming client would wait for a response.completed that never arrives. previous_response_id is refused. Response storage is per-worker in-process state and requests are not pinned to a worker, so a stored response is only findable again by chance; failing later as "response not found" would read as the client's mistake. Validated against a one-node deployment (context and generation workers plus the orchestrator): non-streaming and streaming, single-turn and multi-turn with function_call/function_call_output history, tool calls parsed into function_call items, usage accounted, no internal fields reaching the client, and Codex driving the endpoint end to end. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com> (cherry picked from commit 8572abc) This commit also registers /v1/responses on the disaggregated server. That registration originally rode in with a separate change relaying worker 4xx errors, which is not carried over: main already relays worker errors through _upstream_error_message. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…aped tools Two gaps found while cherry-picking the disaggregated Responses API onto this branch, both in the same class: something the client asked for goes unanswered without saying so. `store` on a disaggregated server -------------------------------- The port rejects `previous_response_id` because response storage is per-worker in-process state and requests are not pinned to a worker. `store` has the same cause and a different consequence, and had no answer at all: the response is stored on whichever worker served it, this orchestrator exposes no route to read it back, and it refuses the `previous_response_id` that is the only other way in. The client asked for persistence, got none, and was told nothing. It is a warning rather than a 400 because, unlike `previous_response_id`, it does not stop the request being served -- only a side effect is lost, and refusing an otherwise valid request over a side effect would break clients that set the flag by default and never read a response back. This matches how the aggregated server treats `background`. Gated on `model_fields_set`, which is load-bearing: `store` DEFAULTS TO TRUE, so warning on the value alone fires for every request that never mentioned the field. A warning on every request is one nobody reads, which would have left the gap exactly as invisible as before. Dict-shaped tools ----------------- `is_web_search_tool` and `web_search_tool_spec` read tool fields with `getattr` alone, so every dict-shaped tool failed the gate and the helpers behind it -- which do handle dicts -- never ran. `web_search_rejection_reason` then returned None for a tool it should have had an opinion about, and None reached callers expecting a string: TypeError: argument of type 'NoneType' is not iterable Both shapes genuinely arrive: validated requests carry model objects, while `get_chat_completion_tool_dicts` produces dicts and a raw body inspected before validation is dicts throughout. `web_search_tool_spec` had the quieter half of the same bug -- it would have built a spec describing nothing and named `tool type None` for a tool whose type was in the payload. Verified against the branch's own suites: 43 passed, including the 295-line disaggregated Responses test file this fixes the last failure in. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
GET /v1/models returned 404 on the disaggregated orchestrator. Clients that do model discovery -- Codex among them -- had to be told the model name out of band, and Claude Code's model listing came back empty. The orchestrator cannot answer from its own config: it has no model of its own and DisaggServerConfig carries no model name. So the request is forwarded to a context worker, which does know, via a new `OpenAIClient.get_json` mirroring the existing `post_json`. With no context worker registered the answer is a 503 rather than an empty list, because an empty list is indistinguishable from "this server serves nothing" and sends a client down the wrong path. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Two shapes a client sends back verbatim were refused, each failing the whole
request rather than the item, which ends the conversation instead of the
turn.
A `reasoning` item does not have to carry `content`. The API puts the text in
`summary` when a summary was requested and in `encrypted_content` when it was
not, and an item with neither populated is a shape OpenAI itself emits.
Sharing a branch with `message` meant an empty one raised, so a client
replaying its own history got a 400 -- and what it lost was not the
reasoning, which was never in the payload, but the conversation. The summary
is now used when present and the item skipped when nothing readable is left.
An empty `message` is still rejected: its text lives in `content` and nowhere
else, so dropping it would lose something the client believed it had sent.
An input item carrying a `role` may omit `type`; the API defaults it to
"message". Such an item was returned verbatim, so its parts reached the
chat-completions content parser still spelled in the Responses vocabulary --
`input_text`, `output_text` -- which that parser does not know:
{"detail": "Unknown part type: input_text"}
The same message with an explicit `"type": "message"` took a branch that
handled the parts and succeeded, so whether a request worked turned on a
field the client is free to leave out. A client replaying an assistant turn
received from this server hit it, because those parts come back as
`output_text`.
The parts are translated rather than flattened to text, so an item mixing
text with an image keeps the image: only the two Responses-only text
spellings are rewritten and everything else is passed through.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Codex does not put its tools in `tools`. It sends an input item
{"type": "additional_tools", "role": "developer",
"tools": [{"type": "namespace", "name": "functions", "tools": [...]}]}
and the input-item conversion had no case for it. Carrying no text, it fell
to the catch-all and was dropped:
Responses API: skipping unrecognised input item type 'additional_tools'
with no text content.
so the model was offered nothing to call. A Codex campaign against a
disaggregated GLM-5.3 endpoint made zero tool calls across its whole round
and, with no way to act, narrated a shell session it had invented -- which
reads as a model failure rather than a dropped field, and cost a round to
diagnose.
The payload is already the shape `tools` accepts: `NamespaceTool` is a member
of the tool union, and `_namespaced_tool_names` and
`_get_chat_completion_function_tools` already flatten such entries to
`namespace.tool`. So the item is hoisted into `tools` in a `mode="before"`
validator and its contents are validated as tools like any other.
Hoisting at the protocol boundary rather than in the prompt builder keeps the
reply side in agreement: a call can only be reported back with its namespace
split out if the tool was in `tools` when the reply was parsed, and a bare
`functions.exec` matches nothing the client knows.
The item is removed from `input` as it is hoisted. It declares tools rather
than being a turn, and replaying it as one would put the tool schemas into
the prompt as prose.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Both tool-result branches assigned `output` straight to `content`, which is
right when it is a string and wrong when the client structures it. Codex
sends a list of Responses content parts, so `content` became a list spelled
`input_text` and the chat-completions parser refused the request:
NotImplementedError: Unknown part type: input_text
A tool result arrives on every turn after the model's first tool call, so an
agent that starts using tools has nearly all of its traffic rejected: 483 of
490 requests in one campaign round. The round was reported as the agent
failing on its own scripts, which is what it looks like from the outside --
the server was refusing to serve it.
`function_call_output` carried the same assumption and is fixed with it; only
the custom-tool branch had fired so far because that is the shape this client
happens to send.
The earlier EasyInputMessage fix translates parts for message items and did
not reach here, a tool result not being a message. Both now share one helper.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Codex puts its whole operating brief in messages with role "developer": who
the agent is, how to drive `exec`, the skills instructions, the environment,
the multi-agent rules. GLM-5.3's chat template dispatches on role with
branches for user, assistant, tool and system, and ends with no fallback, so
those messages render to nothing at all:
developer '[gMASK]<sop><|system|>Reasoning Effort: Max<|user|>list the
files<|assistant|><think>' 85 chars
system '...<|system|>You are Codex. <SENTINEL><|user|>...' 139 chars
The model was being asked to drive an agent harness nobody had described to
it. Tool *definitions* travel separately, in `tools`, so it knew which tools
existed and improvised the rest: shell commands into a JavaScript-only
sandbox, invented exec cell ids, sub-agents spawned into the same confusion.
Four campaign rounds were read as the model failing to follow its tool
descriptions when it had never received the instructions that go with them.
`developer` is OpenAI's rename of `system`. Templates written before that
rename only know `system`, and every template knows `system`, so the alias is
lossless where a template handles both and is the difference between the
instructions arriving and not arriving where it is not.
Only the chat-template path is touched; harmony models take a different route
that already maps the two roles the other way for its own format.
test_item_role_is_preserved keeps its other roles: its point is that a role is
never silently replaced by "assistant", which still holds.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
The orchestrator parses a request and re-serialises it for the context and
generation workers. `model_dump` without `by_alias` writes internal field
names, and where a field carries an alias the two differ. `schema` is the
case that bites: pydantic cannot hold a field of that name -- it shadows
`BaseModel.schema` -- so the model declares `schema_` aliased to `schema`.
A client sends
"text": {"format": {"type": "json_schema", "name": ..., "schema": {...}}}
and the worker is handed
{'name': ..., 'schema_': {...}, 'type': 'json_schema', 'strict': True}
with no `schema` in it, so every member of the format union fails validation
and the request comes back 400. One campaign round lost 43 of 195 Responses
requests to this -- every structured-output call its agents made -- and
nothing upstream said why, because the rejection happens on the worker after
a body this server itself wrote.
Only disaggregated serving re-serialises a request, so an aggregated
deployment never sees it. All four serialisation sites are fixed, not just
the JSON one the failure surfaced through; the msgpack path had the same
omission.
Emitting the alias is safe as well as correct: the workers set
`populate_by_name=True` and accept either spelling, and the alias is the wire
name, which is what is being written.
Adapted to main, which now always forwards requests as msgpack: both
forwarding sites go through one helper, _encode_request_body, and the
test decodes the body exactly as the worker does.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
The request-side fix was only half of it. A worker serialises its reply with
`model_dump()`, so a field whose python name differs from its wire name went
back under the python one, and the orchestrator -- rebuilding the reply with
the OpenAI SDK's models, which do not set `populate_by_name` -- rejected it:
3 validation errors for ResponsesResponse
text.format.ResponseFormatTextJSONSchemaConfig.schema Field required
So `text.format` with a JSON schema went from 400 before the request fix to
500 after it. Both directions have to write the alias.
This is not only a disaggregated concern: an aggregated deployment hands the
same `schema_` straight to its client, under a name the API does not have.
Both response paths are fixed, chat completions as well as responses.
Verified end to end against a rebuilt server: the request now returns 200,
where it was 400 before either fix and 500 after only the first.
The unit suite passes either way -- it exercises both sides in one process,
so nothing crosses a serialisation boundary and neither failure can appear
there. That is why this was committed after the live check rather than
before it.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Under disaggregated serving every Responses reply claimed a complete cache hit: `cached_tokens` came back equal to `input_tokens`, on a cold prompt as readily as on a warm one. A client that watches this to decide when to compact its context -- which is what the Codex CLI does with it -- is being told the prompt is free. The number is not wrong so much as unmeasurable where it was read. From the generation worker the entire prompt arrives as KV transferred from the context worker, so `GenerationResult.cached_tokens` is the prompt length by construction. Only the context phase knows how much of that prompt it actually found in its cache, and it sends its own usage across with the handoff. Chat completions has read it from there since PR NVIDIA#14177; the Responses path, added later in PR NVIDIA#18130, was still reading the local counts. Two halves were missing, and either alone leaves the count wrong: The proxy turns the context reply's `ResponseUsage` into the `UsageInfo` shape that `disaggregated_params.ctx_usage` carries, and copied only prompt, completion and total. `input_tokens_details.cached_tokens` was dropped, so even a reader that went looking for it found nothing. `_create_usage` never looked. It now takes the prompt and cached counts from the handoff when there is one, mirroring `_ctx_usage_from_outputs` on the chat side, and falls back to the local counts when there is not -- an aggregated server has no handoff and its own numbers are the right ones. Output tokens stay local either way; only the generation worker knows what it generated. Both the streaming and non-streaming paths reach `_create_usage`, so this covers them together. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
A generation chunk whose raw text spans the closing think tag parses into two halves: the reasoning up to the tag, and the beginning of the answer after it. The streaming dispatch chose its branch on the answer half alone, so the reasoning half fell into an `elif` that could not run, and nothing else read it. It was emitted nowhere and the item was closed in the same breath. Closing it is what made the loss permanent. _close_open_item clears is_reasoning_sent, and _should_send_done_events is guarded on that flag, so the complete text it computes from a non-streaming re-parse was discarded too -- the done event carried the truncated text as well. A client following the documented advice of reading reasoning_text.done rather than accumulating deltas got exactly the same short string. This is not a boundary case. Four independent full censuses of recorded production traffic -- 196,000 reasoning items across 31 instances and two fleets -- put it at 55.5%, 56.0%, 55.9% and 62.1% of reasoning items, with the dropped tail bounded by one chunk (max 369 chars, median ~100) and 3.15% of all reasoning text lost. It is 100% for reasoning short enough to fit inside a single chunk, which is why the damage looked like stray first words: "Let", "The", "OK". Turns that end in a tool call emit no answer text, so the other branch ran and they were never affected -- 0% there, against 87.7% of turns that produced visible text. That split is what identified the branch. The fix flushes the reasoning half before closing, exactly as the `elif` would have. Four tests drive the real dispatch over chunk sequences that straddle, align on a boundary, end without an answer, and hold reasoning shorter than one chunk. Nothing covered this dispatch before, which is how it shipped. Not fixed here, and not the same defect: reasoning lost on streams that end without response.completed, reasoning misclassified into the visible answer, the snapshot ordering reasoning after the message, and encrypted_content being requested on every call and never returned. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
The fake supplied only text_diff. The delta path parses the diff, but the done-event path re-parses output.text -- the whole generation so far -- and raised AttributeError on all four new tests, including the two that pass regardless of the fix. Found by running them; the scope check that built the fake read only the dispatch function and missed what it calls. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…m does The completed-response snapshot appended the message item before the reasoning item, while the streaming path emits reasoning, then message, then tool calls. The two descriptions of one generation disagreed: a client replaying `response.output` read the answer before the reasoning that produced it, and one reconstructing the turn from the snapshot fed the model its own thinking as a follow-up to its own reply. Measured across four full censuses of recorded traffic on 31 serving instances: 99.99% of responses carrying both a message and a reasoning item were inverted. The stream was never affected. Assemble the snapshot in the streamed order instead. Nothing indexes into `response.output` positionally; the only consumer of its order is the tool-call lookup in _parse_response_input, which scans for a matching call_id and is unaffected by where reasoning sits relative to the message. The stored conversation messages are built from a dict by _create_output_messages and do not change either. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Between 0.25% and 1.83% of /v1/responses streams end without response.completed. They carry no error event, no populated response.error and no incomplete_details: the byte stream simply stops, and nothing in it or in the trace distinguishes a cut stream from one still in flight. Because response.completed is the only event that repeats the full text, everything those turns produced survives solely as deltas -- 10.4 million characters of reasoning on one fleet, and a worst case of 610 deltas and 117,695 characters followed by nothing. A guard around the streaming generator now ends such a stream with an error event naming the cause and a response.failed snapshot, and reports the cause to the request trace. response.failed rather than response.incomplete because incomplete_details.reason is a closed enum -- max_output_tokens, max_messages, content_filter, steered -- with no member for a server fault or a client hangup, so response.incomplete could only go out with reason=null, which reads exactly like an ordinary truncation. A client hangup adds nothing to the stream. There is nobody left to send to, and yielding while GeneratorExit propagates raises "async generator ignored GeneratorExit"; the cause reaches the trace instead. The trace also learns to record a cancelled body iterator as client_disconnected rather than error, which is the bucket the census reads. Nothing is added once response.completed has gone out, so a stream that finishes normally is unchanged. No lost content is reconstructed: the two causes cannot yet be told apart well enough to know whether the accumulated buffers were still in memory, and making the failure visible is the prerequisite for deciding that. Adapted to main, which has no request trace: the trace wiring (request_trace.py and its tests) is not carried over, and both servers install the guard without an on_termination callback. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Two defects on the Responses path, each found by measuring live agent traffic against the request/response traces rather than by reading the code. Tool call ids were reminted for the final response. `response.completed` rebuilds `output` from the generation, and `_tool_call_output_item` mints a fresh random id every time it runs, so the snapshot disagreed with the stream on every call - 1224 of 1224 measured. A client keying its tool results off the streamed `call_id` then matches nothing in the final response. The stream now records the ids it published and the final response reuses them positionally, warning when it rebuilds more calls than were streamed rather than silently handing back ids the client cannot match. `reasoning_tokens` was a hardcoded zero, so a client could not tell how much of a turn went into reasoning. It is now counted by re-encoding the text the reasoning parser itself claimed, rather than by searching the generated token ids for the closing marker. The marker search is cheaper and exact, and was the first design; it was dropped because it has to restate the parser's rules and would silently disagree the moment they differ. GLM alone needs three of them: it closes the block more than once and only the first close counts, `<tool_call>` ends the block implicitly, and a turn emitting neither is reasoning to its last character. Deriving the count from the parser's own output cannot drift from it, and stays correct for the parsers that interleave reasoning with content, where no single prefix length describes the split. The tokenizer is taken as an argument rather than held on the streaming processor, which is pickled across the postproc worker boundary and whose worker already has one. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…bodies Codex ships screenshots in three shapes: message content parts, bare top-level items, and inside custom_tool_call_output.output. The first fix covered the first two; 114 Kernel-Trace campaigns then died in 24h through the third (the parts list lives under "output", not "content"). The degrade walk now covers all three, with the traced shape as a regression test. Validation-error bodies are capped at 4KB so a rejected request's base64 payload no longer floods the client's stop reason. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…at preceded it Three stream-vs-final divergences, each measured on production traffic against a GLM-5.3 fleet: - Item IDs: the final rebuild minted fresh UUIDs for reasoning and message items (216 of 221 sampled responses disagreed with their own stream). The streaming tracker now records the item IDs it emitted and the final builders reuse them, the same wiring the tool-call call_id fix used. A count mismatch logs a structural-divergence warning instead of papering over it. - Sequence numbers: with postprocess workers enabled the opening event pair is numbered by the frontend processor and everything after by a pickled copy in the worker, so one stream carried 0,1,0,1,2,... (221 of 221 sampled). Frames are now restamped with one per-response counter at the egress generator, the single point both configurations flow through. The frame is JSON-parsed, not substring-patched: response.completed embeds model output, which can contain the literal field name. - An unterminated <tool_call> at end of stream was held by the tool parser and dropped from the stream while the final rebuild fell back to including it as text - a 429-char message the stream never showed. The flush now releases the held markup as text so both views carry the same characters. parallel_tool_calls=false and non-auto tool_choice on the non-Harmony path remain unenforced by design (these turns are recorded as training data; truncating what the model generated would falsify it) - but the server now says so once per process instead of silently swallowing the options. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…nt sent
Five audit findings (A08, A02-addendum, A07-addendum, A09), each with a
production fixture or a controlled reproduction:
- The KF client serializes sub-agent task payloads as a content part typed
encrypted_content whose same-named field holds plain readable text; the
converter dropped it, so a sub-agent could receive a task with no payload
(a real 721-char task loss). Readable string bodies now convert to text;
genuinely opaque values stay dropped rather than guessed at.
- An explicit {"type": "message", "content": "hello"} converted to empty
content (the typed branch walked the string as parts); the same shape
without "type" was fine. Both now accept plain-string content.
- Tool-call identity is carried by call entity, not filtered-list position:
a call dropped for invalid JSON now retires its own streamed id instead of
donating it to the next call in line.
- tool_choice="none" is enforced losslessly on the non-Harmony path: the
tool parser is bypassed on both the streaming and final views, so the
model's markup stays in the visible text verbatim. required/named remain
unenforced (sampling-level work) and still warn once.
- A max-token-truncated stream now terminates with response.incomplete and
incomplete_details instead of response.completed wrapped around an
incomplete status.
Also repairs test_responses_input_preprocess.py's namespaced-tools test,
which had referenced a helper removed by the tool-resolution refactor and
failed on import ever since - nothing in CI ran the file.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
… that always land
Closes the dormant-path and terminal-trace audit items (A10, A12-half,
A09-residual, A02-non-streaming):
- Store paths: a tool-only assistant turn is now stored (it stored nothing);
a stored message carrying reasoning keeps its tool_calls on replay instead
of vanishing whole and orphaning the later tool result; and store=false on
the CURRENT request no longer suppresses retrieval of PRIOR context -
persistence and retrieval are two switches, gated independently. The full
write -> store -> previous_response_id -> tool-result chain is the test.
- /v1/messages: all four deliberate early returns now write exactly one
terminal trace record (rejected_4xx / error) through a single reject
helper - a rejected request no longer arrives in the requests shard and
vanishes from the responses shard.
- incomplete_details returns to ResponsesResponse (the field was commented
out) and the non-streaming path populates it for length truncation;
status=failed streams as response.failed instead of response.completed;
cancelled deliberately stays (client-initiated, no spec event). The new
field and ten previously untracked internal fields are registered in the
serve API stability reference.
- Non-streaming requests could still deliver a phantom {}-argument call for
terminated-but-unreadable markup; the whole-text result is now verified
against a streaming replay of the same text, and a call the stream would
refuse is dropped (or falls back to text when it was the only one).
Also migrates test_responses_custom_tools.py off two helpers the
tool-resolution refactor removed - the file had failed on import ever since,
so CI never ran it.
Adapted to main, which has no request trace: the /v1/messages terminal
trace records (and their tests) are not carried over.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
An external replay audit of one production week (310k requests) against GLM-5.3 locked divergences between what the stream published and what the final snapshot / rendered prompt said. Streamed bytes are irrevocable, so wherever the two views read the same characters, the snapshot now repeats the stream. - Responses: the streamed item record now carries each item's delta text and each call's parser-level arguments. For streamed requests the snapshot assembles reasoning/message items verbatim from that record (same count, boundaries, ids, bytes) and, under entity alignment, keeps the streamed tool arguments when the whole-text re-parse read malformed markup differently. - Stop stripping edge whitespace in the snapshot: generated whitespace is part of the generation. - Emit the missing response.content_part.done for reasoning parts in both closing branches. - Merge consecutive function_call items of one assistant turn into a single assistant message so the chat template can bind tool results to calls by id (swapping two result call_ids used to render a byte-identical prompt). 1585 serving/parser unit tests pass; new regression tests cover every item above and fail on the unfixed code. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Three test files added in this change were on no test list, so CI would never have run them: test_responses_store_replay.py, test_responses_stream_final_consistency.py and test_responses_usage_disagg.py. They are registered in l0_cpu.yml next to the other Responses tests. test_responses_usage_disagg.py also gains the cpu_only marker, without which tests/unittest/conftest.py drops the file from a stage that runs -m cpu_only, and the NVIDIA copyright header. test_disagg_responses_api.py was listed twice in l0_cpu.yml: on its own and through the unittest/disaggregated directory entry. The file entry is dropped. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
test_standard_and_disagg_register_messages_route stubs the service with a SimpleNamespace, so registering /v1/responses -- which reads _service.openai_responses -- raised AttributeError rather than failing an assertion. The test asserted only /v1/messages and count_tokens, so the two routes added since had no coverage at all and the break surfaced as a stub error. Add the missing attribute and assert both new routes. Each was a client-visible outage before it was added: a Responses-API client got a 404 on /v1/responses, and anything doing model discovery got one on /v1/models. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Main removed the legacy renderer switch (NVIDIA#20775), so the store-replay test no longer patches legacy_render_enabled. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #80533 [ run ] triggered by Bot. Commit: |
Collaborator
Author
|
@brnguyen2 on the scope point: I kept it as one PR. It supersedes #18497, and its parts have been through several full CI runs together, so splitting now would mean validating each part again on its own. The branch keeps one commit per fix (41 of them). If one part regresses, |
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #80552 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #80533 [ run ] completed with state |
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #80584 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #80552 [ run ] completed with state |
Collaborator
|
PR_Github #80584 [ run ] completed with state
|
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #80652 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #80652 [ run ] completed with state |
This was referenced Oct 27, 2026
This was referenced Nov 2, 2026
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
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.
Description
Fixes for
/v1/responsesfound while serving Codex-style agent traffic, plus disaggregated serving support for the endpoint. Supersedes #18497, whose three commits are included here, rebased onto current main.This fixes the non-Harmony half of NVBug 6781271 / #19223. The Harmony (GPT-OSS) items in that issue are deliberately left out: the plan is to refactor the Harmony path onto the same code as the regular path rather than patch it separately (see "Not included").
Disaggregated serving (TRTLLM-15894, from #18497)
/v1/responseson the disaggregated server. The orchestrator splits the request into context and generation phases and streams the generation worker's bytes back as-is.ResponsesRequest/ResponsesResponsecarry the handoff (disaggregated_params,prompt_token_ids[_b64], rawfinish_reason); the handoff fields never reach the client. Context-first and generation-first scheduling are both supported.previous_response_idis rejected with a 400: stored responses live in one worker's memory, and requests are not pinned to a worker.storeis forwarded asfalse, with a once-per-process warning when the client asked for it explicitly./v1/modelsis served on the disaggregated server, from a context worker.text.formatJSON schema survives the hop (it arrived asschema_and failed validation).cached_tokenscomes from the context phase instead of claiming a full cache hit.response.created/response.in_progress/ terminal event, not[DONE].web_searchwithexternal_web_access: falseis accepted.Input and replay
messageitems with stringcontentkeep their text, as untyped ones do. They used to come through empty.reasoningitems withoutcontentfall back tosummary; empty ones are skipped. Multi-part content is joined.Unknown part type: input_text.additional_toolsinput item is hoisted intotools.developermessages render assystemwhen the chat template does not render them (it has nodeveloperbranch, or rejects the role), instead of rendering to nothing. This is decided by rendering a probe message through the template, cached per template, not by searching its source. Templates that renderdeveloper, and the DeepSeek V3.2/V4 tokenizers, are left alone.encrypted_contentcontent part is read as text (the KF agent client sends sub-agent task payloads this way), unless its value is one long base64 token, i.e. a real ciphertext.Stream and final response agree
id,call_id, arguments).sequence_numberis numbered by whoever produces the events: the opening pair is 0 and 1 and the rest continue from 2, so a postprocess worker's copy of the processor numbers consistently. With postprocess workers, one stream used to carry0,1,0,1,2,....content_part.doneis emitted for reasoning parts.streaming_matches_whole_parse, set for the GLM parsers in [None][fix] GLM tool and reasoning parsers: streaming matches the whole-text parse #19935) they are the calls its increments reported, and a reported call closes the open message item; other parsers' calls come from one whole-text parse, as on main. Unterminated call markup at the end of a stream is released as text in both views.function_callitems of one turn fold into one assistant message, so the chat template can bind tool results to calls by id.Terminal events
response.incompleteandincomplete_details;status=failedends withresponse.failed.errorevent (plusresponse.failedon the worker) instead of just stopping, on both aggregated and disaggregated servers. Only engine errors carry their message to the client; transport and internal errors report the exception type only. A relay cut mid-frame terminates that frame before the error event.Storage and continuation
store: falseon a follow-up no longer suppresses loading the prior context. Persistence and retrieval are gated independently.Usage and options
reasoning_tokensis counted by re-encoding the text the reasoning parser claimed, for streamed and non-streamed responses. It used to be a hardcoded 0.tool_choice: "none"is honoured on the non-Harmony path.parallel_tool_calls: false(when set explicitly) and other non-autotool_choicevalues are accepted but not enforced, with a once-per-process warning.exec) and qualified (functions.exec). A custom tool is therefore reported ascustom_tool_callrather than as afunction_callthe client rejects.GLM parsers
The GLM tool and reasoning parser fixes these changes were built and validated with landed in #19935, which this PR was stacked on; it is merged with main and no longer carries those changes. This PR uses their
streaming_matches_whole_parseflag, and some of its tests use the GLM parsers.Status of #19223 items
output_text.logprobs: nullreplay fails validationresponse.completedsent regardless of statusget_final_response_non_store, so a streamed response is never stored; the unreachable storing finalizer is removedprevious_response_idwithstore: falsedrops historyAdaptations to current main
_upstream_error_message. Its/v1/responsesroute registration moved into the disaggregated-serving commit.request_trace. The stream guard is installed without a termination callback._encode_request_body.additional_toolshoisting.Not included
get_final_response) is removed so it cannot be wired back in its stale form./v1/chat/completionson main and are left for a separate fix.Documentation
docs/source/features/disagg-serving.mdhas a Responses API section: the endpoints, both schedule styles,previous_response_idandstoreon the disaggregated server, and the limitations shared by every server.response.incompleteterminal for a response cut short bymax_output_tokens.Test Coverage
Unit tests, all
cpu_onlyand run in the CPU stage (l0_cpu.yml):tests/unittest/llmapi/apps/:test_responses_streaming_events.py,test_responses_streaming_tool_calls.py,test_responses_stream_final_consistency.py(new),test_responses_store_replay.py(new),test_responses_input_preprocess.py,test_responses_usage.py,test_responses_custom_tools.py,test_web_search_shared.py,test_anthropic_routes.pytests/unittest/disaggregated/:test_disagg_responses_api.py(new),test_disagg_openai_client.py,test_router.pytests/unittest/api_stability/: thetrtllm_serve_api.yamlreference is updated for the new internal fields.What they check, beyond one test per fix above:
Local run on this branch (built from source on H100, torch 2.14):
-m cpu_onlyovertests/unittest/llmapi/apps,tests/unittest/disaggregated,tests/unittest/llmapi/test_reasoning_parser.pyandtests/unittest/api_stability, with [None][fix] GLM tool and reasoning parsers: streaming matches the whole-text parse #19935 included: 2840 passed, 0 failed.Two test fixes ride along. The disaggregated server now registers
/v1/responses, so the route-registration test intest_anthropic_routes.pyneedsopenai_responseson its stub service. With that fixed, the test reaches the multiprocess metrics collector and leaks prometheus multiprocess mode into whichever test file runs next in the same process. Its setup moves into a fixture that restores both the environment and prometheus' value class.The original fixes were validated on a GLM-5.x deployment driven by Codex-style agent clients, both aggregated and disaggregated. The parser and streaming changes made after review are covered by the unit tests and fuzzing above, and the pre-review test suite was re-run against them (failures only where behaviour changed on purpose or where it tested removed internals), but they have not yet been re-run on a live deployment.
PR Checklist
🤖 Generated with Claude Code
Dev Engineer Review
The current diff changes formatting only in five serving source files. It reformats imports, docstrings, and debug strings. It does not change runtime behavior, API contracts, configuration, or test lists. No material correctness risk is apparent from this diff.
QA Engineer Review
Two unit-test files changed. The changes reformat imports and one local import; they do not alter test functions, IDs, selectors, or waivers. No integration test-list file changed. No test execution results are supplied. Coverage verdict: needs follow-up.
Per-File QA Perspective
tensorrt_llm/serve/openai_disagg_server.py: Import and docstring formatting only. No observable server behavior change is shown.tensorrt_llm/serve/openai_protocol.py: Import formatting only. No observable API or behavior change is shown.tensorrt_llm/serve/openai_server.py: Import formatting only. No observable endpoint or response behavior change is shown.tensorrt_llm/serve/responses_utils.py: Import, docstring, and debug-string formatting only. No observable Responses behavior change is shown.tensorrt_llm/serve/router.py: Import formatting only. No observable routing behavior change is shown.tests/unittest/disaggregated/test_router.py: Import formatting only. Existing routing test coverage remains unchanged. This unit test is not an integration test-list entry.tests/unittest/llmapi/apps/_test_openai_perf_metrics.py: Import formatting only. Existing performance-metrics test coverage remains unchanged. This unit test is not an integration test-list entry.