Repository navigation
[TRTLLM-17268][feat] Serve gpt-oss chat completions through the shared chat handler - #21735
Draft
JunyiXu-nv wants to merge 58 commits into
Draft
JunyiXu-nv wants to merge 58 commits into
JunyiXu-nv wants to merge 58 commits into
Conversation
…PU trigger for chat_tokenization.py Turn the CI bookkeeping that the trtllm-serve refactor PRs must keep (epic TRTLLM-17056) into checks that fail with a fix-it message: - scripts/check_serve_ci_coverage.py, driven by scripts/serve_ci_coverage.yml: the tensorrt_llm/serve/ entries of the multi-GPU trigger table (relatedFileList in getMultiGpuFileChanged) must match files at the current head; once a path on the required list exists, the table must cover it; no table entry may equal a known too-broad prefix; the CBTS VisualGen prefix tuples must match files; the CODEOWNERS rules under /tensorrt_llm/serve and /tensorrt_llm/commands/serve must match files, and files under the required paths must have exactly the listed owners. Only these refactor-managed entries are checked: dangling entries and rules elsewhere in those files are left to their owners. - The checks run as the serve-ci-coverage pre-commit hook (so they also fire on serve/ moves that change no CI file) and as CPU unit tests in tests/unittest/scripts/; the live VisualGen check has its own file and l0_cpu.yml entry under a visual_gen/ path, because CBTS narrows VisualGen-only PRs to test-db entries containing visual_gen/. - jenkins/L0_MergeRequest.groovy: add tensorrt_llm/serve/chat_tokenization.py to the multi-GPU trigger table. It is on the disaggregated path (the KV-aware router and the gpt-oss extension import it) and the e2e tests that cover it run only on multi-GPU stages. No behavior change; no runtime code changes. legacy-files.txt and the configs generated from it are unchanged: each later PR registers its own new files there. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…x of the serve directory The Jenkins multi-GPU trigger table matches changed files by substring, so an entry like "tensorrt_llm/serve" (without the trailing slash) starts the multi-GPU stages for every serve change exactly like "tensorrt_llm/serve/" does, while an exact-equality forbidden check lets it through. Match the forbidden rows with and without their trailing slash, and also reject any other table entry that is a string prefix of the managed serve directory. Two tests pin the no-slash and bare-prefix cases red. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
tests/unittest/conftest.py has an autouse fixture that takes capfd, and pytest refuses capsys and capfd in the same test, so the two CLI tests errored at setup when run through the repository conftest chain. capfd captures the same output at the file-descriptor level. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
yaml.safe_load keeps only the last of a repeated mapping key, so a second required: block pasted into serve_ci_coverage.yml silently replaced the first and the lint still passed. load_data now uses a SafeLoader subclass that raises on a repeated scalar key, reported as a ParseError; merge keys and non-scalar keys load as before. Tests: the duplicate-key case, and the two failure branches of check_visual_gen (a prefix that matches no file, a configured tuple the rule file does not define). Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…diagnostics Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…apshots and a CPU test seam for trtllm-serve Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
… baselines at the merge base The route reference is generated from the live apps; the object.__new__ baseline and the gRPC import allowance record what exists on main today and only shrink. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…n the snapshot matrix An attached frontend of a multi-frontend server registers ten launcher forwarding routes before the local table; seven share a method and path with local handlers and three use a path parameter that matches every batch id, so registration order decides which handler serves them. The new configuration records all of it, a live invariant pins the forwarding-before-local order independently of the reference, and the route-class failure message now says why the msgpack route class matters. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
- AGENTS.md: the CPU test command sets LLM_MODELS_ROOT, which the runner requires, and the stage-mapping note says the mapper matches list-entry text, so a file listed through its directory maps by the directory. - check_contracts matches each from-import as <module>.<name>, so 'from pkg import sub' and 'from .. import sub' count as imports of the submodule; new test for both forms and for the allowance entry. - The VisualGen /v1/models test asserts the 200 status. - The AGENTS.md command check also recognizes commands that start with LLM_MODELS_ROOT=. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…on the current main The disaggregated frontend now registers its routes with KvHintsRoute (NVIDIA#20269), which strips kv_hints from request bodies unless the server trusts its ingress. Regenerate the route reference: the six disaggregated configurations record KvHintsRoute where they recorded APIRoute; their route sets and order are unchanged. The route-class invariant, which a regenerated reference cannot hide, now requires KvHintsRoute on every API route of the disaggregated frontend and APIRoute on the coordinator app. Dropping the KV-hint policy from the frontend fails it even after a regeneration, and both apps still reject the aggregated server's msgpack route class. The object.__new__ baseline records the sites on main at the merge base: test_kv_hints.py builds one server that way (NVIDIA#20269), and test_harmony_parsing.py no longer builds any (NVIDIA#21176). The harmony parsing tests are now registered as a whole file (NVIDIA#21411), so the node-id entry that the CPU-selection test used as its example is gone. The node-id check now runs select_entries on a one-entry list of its own, so it no longer depends on which node-id entries l0_cpu.yml holds. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ail on silent regressions The route snapshot now requires every configuration other than the three invalid ones to build, so a configuration that starts failing at construction cannot pass through a regenerated reference. The object.__new__ ratchet resolves 'from ... import <server class> as <name>' aliases and records each baseline site as its enclosing function or class plus the server class, so swapping one site for another in the same file no longer passes. The baseline keeps the same 52 sites in the same 26 files. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…s outside-subclass contract Every OpenAI request on the disaggregated frontend, and every Anthropic /v1/messages request, goes through OpenAIDisaggServer._wrap_entry_point. Before reorganizing that closure, pin what it does today: - tests/unittest/disaggregated/test_disagg_entry_point.py pins the per-request counters, the chat-template 400, the conversation identity from the headers, the serialization of streaming and non-streaming responses (including the guarded streamed Responses relay and model_dump(by_alias=True)), the error mapping through _handle_exception, and the single disaggregated_request trace span. - tests/unittest/disaggregated/test_disagg_server_contract.py pins the surface NeMo-RL's OpenAIDisaggServerAdaptor subclass relies on: the constructor (config, req_timeout_secs=..., coordinator_url=None), self.app with every route registered when __init__ returns, __call__(host, port, sockets=None), _wrap_entry_point, _handle_exception and _perf_metrics_collector.http_exceptions. - The OpenAIDisaggServer class docstring and tensorrt_llm/serve/AGENTS.md name that surface, so a change to it is agreed with the NeMo-RL owners first. Both files are CPU-only and collected by the unittest/disaggregated entry of tests/integration/test_lists/test-db/l0_cpu.yml. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…into prepare, run, finish and serialize steps No behavior change. The body of the closure OpenAIDisaggServer builds in _wrap_entry_point becomes a composition of five methods, so that later changes (disconnect watching, in-stream errors, request trace) land in the step they belong to instead of growing one closure: - _prepare_request: the request counters, the chat-template permission (HTTPException 400), the conversation identity, the per-request hooks - _run_entry_point: the call into the disaggregated service - _finish_request: success accounting (total_responses) - _finish_failed_request: every failure, through _handle_exception while the exception is being handled (its traceback logging needs that) - _serialize_response: StreamingResponse with the Responses stream guard, or JSONResponse(content=...model_dump(by_alias=True)), as before The trace span stays on the wrapper that FastAPI calls; the steps open no spans of their own. The statements keep their order inside the same try block: the counters run before the template check, total_responses before serialization, and a serialization failure still reaches _handle_exception. Some step parameters (raw_req of _run_entry_point, req and hooks of the finish steps) are not read yet; they are the inputs the known upcoming changes to each step need, so those changes will not have to alter the signatures again. The characterization tests and the subclass contract tests of the previous commit pass unchanged on both sides of the split. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
… and disaggregated servers Create tensorrt_llm/serve/middleware/ and move ServerArrivalTimeMiddleware (from serve/responses_utils.py), PerfMetricsMiddleware (from serve/perf_metrics.py) and _apply_fastapi_middlewares (from commands/serve.py) into it, verbatim. Both servers now install their built-in middleware through serve/middleware/register.py::register_middlewares, which reproduces each app's stack exactly: the aggregated server passes adjusted_clock to both middlewares, the disaggregated server to neither. User --middleware entries are still applied after construction and stay outermost. The old import paths are removed with no compatibility shim; no code outside the repository imports them. commands/serve.py keeps an import of _apply_fastapi_middlewares for its own two launcher calls. CI bookkeeping in the same change: the multi-GPU trigger table gains tensorrt_llm/serve/middleware/; legacy-files.txt registers middleware/arrival_time.py and middleware/user_middleware.py with the configs gen-configs regenerates; ruff-legacy-baseline.json carries the moved docstring's one D212; mypy checks the new register.py. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…g request state
Replace the assignment scope["state"] = {} with scope.setdefault("state", {})
in ServerArrivalTimeMiddleware.__call__. State already present when a request
reaches this middleware - written by a user --middleware entry, or copied in by
the ASGI server from a lifespan that yields a dict - is now kept, and
server_arrival_time is added to it. Before, that state was replaced by an empty
dict. Neither trtllm-serve app yields lifespan state today and the built-in
middleware order is unchanged, so no built-in path changes behavior.
Two tests pin the new behavior (state from an outer middleware and from a
lifespan survive); both fail on the previous commit.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…lm/serve/_base/msgspec_route.py Generated move: _msgpack_decoder, _MsgspecRequest and _MsgspecRoute leave tensorrt_llm/serve/openai_server.py byte for byte; the module imports _MsgspecRoute back for its app router and keeps no re-export (the three names are private). The two disaggregated test importers now import the new module. OpenAIServer.health, .version and .get_model are removed here and replaced by the system router added in the following commits. tensorrt_llm/serve/_base/msgspec_route.py joins legacy-files.txt (its code comes from a legacy-formatted file), and the generated lint configs are regenerated with scripts/legacy_utils.py gen-configs. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ve the system endpoints through it New structure: tensorrt_llm/serve/_base/ (shared serving infrastructure) and tensorrt_llm/serve/api/ (endpoint packages, one router plus one serving class each; the package __init__ imports nothing so routers load lazily, by role). serve/_base/routing.py provides make_router(), which always uses the msgpack-decoding route class, and attach_routers(), which imports and attaches only the routers whose RouterSpec matches the server role. The first endpoint package, serve/api/ops/system/, serves /health, /version and /v1/models for every role. OpenAIServer builds a SystemServing with the injected _health_probe (today's probe body, SIGINT side effect included) and attaches the routers before the role dispatch; the 12 add_api_route lines for the three paths leave the four role tables. Same routes, methods, route classes, bodies and conflict-relevant order: the recorded route reference is unchanged. The layering tests gain the new packages' contracts, the route-snapshot failure message points new endpoints at the router pattern, and the serve agent docs describe where a new endpoint goes. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…w serve packages tensorrt_llm/serve/_base/ and tensorrt_llm/serve/api/ops/ join the multi-GPU trigger list: the msgpack route class decodes the disaggregated frontend's worker requests, and /health is the disaggregated readiness probe, so changes under these directories keep running the multi-GPU stages that exercise that path. The three new modules join the strict mypy file set, and the type-check pre-commit hook now also triggers on changes under tensorrt_llm/serve/ and tensorrt_llm/commands/serve_cli/, so edits to typed serving modules get local type checking; the hook still checks only the declared file set. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
… tests that build routes by hand Two multi-frontend tests build an OpenAIServer without its constructor and call register_routes() themselves. Since /health, /version and /v1/models are attached through the system router by the constructor, those tests no longer saw the three routes: one asserted that /version stays local on an attached frontend (404), the other reported the three per-frontend-safe entries as removed. Both helpers now set the serving object and attach the routers before the hand registration, in the constructor's order, and the route enumeration walks included routers, which FastAPI 0.139 keeps as objects without a path in app.routes. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…n the OpenEngine metrics test The HTTP-enablement case builds a real OpenAIServer through its constructor and looked for /prometheus/metrics by reading .path off every entry of app.routes. Since the constructor attaches the system router, FastAPI 0.139 keeps an included-router object there that has no .path, and the child process failed with AttributeError. The test now matches a GET /prometheus/metrics request against the routes with the public route-matching API, which works on both ends of the pinned FastAPI range. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…d name The check that endpoint packages under tensorrt_llm/serve/api do not import each other read the package off the from-module alone, so `from tensorrt_llm.serve import api` inside an endpoint package passed and `from tensorrt_llm.serve.api.ops import system` inside ops/system failed. It now reads the package off <module>.<name> per imported name, as the layering contracts do, and a synthetic-edge test pins both forms. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…e/api/visual_gen Move the eight VisualGen request and response models (ImageGenerationRequest, ImageEditRequest, ImageObject, ImageGenerationResponse, MediaReferenceItem, VideoGenerationRequest, VideoJob, VideoJobList) out of tensorrt_llm/serve/openai_protocol.py into the new package tensorrt_llm/serve/api/visual_gen/ (protocol.py), unchanged. openai_protocol keeps the eight names importable through a lazy module __getattr__: importing it no longer loads the VisualGen models or tensorrt_llm.visual_gen, and the re-export is part of its own interface, so it never warns and has no removal release. The imports only the moved block used (UploadFile, AliasChoices, MediaRole) leave openai_protocol with it. Every importer in the repository now imports the new module; the benchmarking guide links the new file. protocol.py joins legacy-files.txt (its code comes from a legacy-formatted file) and the derived lint configs are regenerated. No behavior change. OpenAPI component names may change with the move; they are not a contract. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…isualGen team in CODEOWNERS VisualGen serving code now lives under tensorrt_llm/serve/api/visual_gen/. The new rule, placed after the /tensorrt_llm/serve rule so that it wins, makes @NVIDIA/trt-llm-torch-visual-gen-devs the owner of that directory: changes there need the VisualGen team's approval and no longer need runtime-devs. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…y re-export under a visual_gen path Add tests/unittest/api_stability/visual_gen/test_visual_gen_serve_api.py: it runs the checks of test_serve_api.py on the models of references/trtllm_serve_api.yaml that are defined in tensorrt_llm.serve.api.visual_gen.protocol, and checks that tensorrt_llm.serve.openai_protocol re-exports the eight VisualGen names lazily (importing it does not load the VisualGen module; unknown names still raise AttributeError). The file lives under a visual_gen/ path and has its own l0_cpu.yml entry so that CI selection for VisualGen-only pull requests keeps running it; the reference YAML is unchanged. Add the eight identity rows of the VisualGen names to the VisualGen shim identity table: identity only, no warning and no ban on the old path, because the re-export is openai_protocol's permanent interface. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…its no-warning contract Resolve every pinned model with the full gate's own loader, so a VisualGen model pinned in the reference YAML that is later renamed or removed fails this gate too instead of being dropped silently; the four known models stay a secondary check. Make the lazy-load probe's diagnostic best-effort and identity-based, so a failure prints which modules hold the VisualGen classes instead of crashing on module-level __getattr__ hooks while walking sys.modules. In the same fresh-interpreter probe, assert that reading each of the eight names from openai_protocol emits no FutureWarning or DeprecationWarning. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…efore splitting it Three tests run OpenAIServer.openai_chat through a server built with the real constructor and the fake generator, and pin what it answers today: a request the handler rejects keeps its 400 body byte for byte, an engine failure keeps HTTP 200 with body null (and /health_generate keeps turning that into a 500), a response that cannot be JSON-encoded keeps its 400, and a stream keeps its frames and media type. They pass on the unsplit handler and must still pass once it is split into a typed core and an HTTP wrapper. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…serve/_base/sse.py Generated move: the chat_stream_generator closure of OpenAIServer.openai_chat is lifted to module level in tensorrt_llm/serve/_base/sse.py unchanged, apart from its two free variables, which become the keyword-only parameters `server` (was `self`) and `raw_request`. openai_chat imports it back and passes server=self, raw_request=raw_request at its one call. The generator keeps its `except Exception:` clause and its two `_postprocess_result` reads. tensorrt_llm/serve/_base/sse.py joins legacy-files.txt (its code comes from a legacy-formatted file) and the generated lint configs are regenerated with scripts/legacy_utils.py gen-configs; ruff-legacy-baseline.json gains the module's one recorded F821 (the `anext` builtin ruff does not know), which openai_server.py keeps counting until its baseline is regenerated. The Anthropic stream tests patched the logger of openai_server to check what the generator logs; they now patch the logger of serve/_base/sse.py, where the generator lives. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ompletion and an HTTP wrapper OpenAIServer.create_chat_completion is the former body of openai_chat with a typed contract: it returns the ChatCompletionResponse for a non-streaming request and the SSE stream as strings for a streaming one, and it raises ServingError (the HTTP status apart from the OpenAI error body) where it used to return an error response, or EngineFatalError after the three side effects of an engine failure. Nothing else in the body changes: the six early rejections raise where they returned, the ValueError and the generic handler raise what they rendered, and the ServingError handler comes first so an early rejection is neither logged nor re-wrapped. openai_chat keeps its name, signature and route and becomes the wrapper that renders the typed result: JSONResponse for the response, a streaming response for the stream, the error body with its status for ServingError, and HTTP 200 with body null after an engine failure, as before. Its own ValueError branch keeps the 400 for a response that cannot be JSON-encoded, which used to fail inside the core. Every caller (the route, /health_generate, the Anthropic messages and batch paths) is unchanged. tensorrt_llm/serve/_base/errors.py holds error_body, ServingError, serving_error and EngineFatalError; create_error_response builds its body with error_body, so a ServingError body and a rendered error can never differ (the over-length-prompt code lives in one place). Tests: the typed core and its errors through a server built with the real constructor; the error builders against create_error_response; the lifted SSE generator with fakes. The three answer-pinning tests added before the split pass unchanged. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
The new error module is written from scratch, so it joins the mypy file list in pyproject.toml; scripts/run_mypy.sh passes with it (the strict global settings apply). tensorrt_llm/serve/_base/sse.py stays out: it is moved legacy code and joins type checking with its package later. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…se in the multimodal prompt test The chat handler now serializes a ChatCompletionResponse and streams any other result of its core, so a stand-in for _create_chat_response that only has model_dump() is no longer rendered as JSON. The test stubs a real ChatCompletionResponse instead; what it checks (the rendered prompt, the decoded image and chat_template_applied reaching the engine) is unchanged. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…eware package branch as the base for the shared app assembly Base for the shared app assembly: both open PRs it builds on, NVIDIA#21447 (shared middleware package for the aggregated and disaggregated servers) and NVIDIA#21448 (router skeleton and the ops.system endpoints), merged into one branch. Conflicts resolved by keeping both sides: the mypy file list in pyproject.toml and the import-direction contracts in tests/unittest/serve/test_layering.py. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ve/api/anthropic Move tensorrt_llm/serve/anthropic_adapter.py, anthropic_protocol.py and anthropic_batches.py whole into tensorrt_llm/serve/api/anthropic/ as adapter.py, protocol.py and batches.py, and rewrite every import of them in the repository. The old paths are deleted without a compatibility shim: no code outside the repository imports them. Generated change: the three files are copied byte for byte apart from their import lines; the importers change only in their import lines. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
… mark the new package Add tensorrt_llm/serve/api/anthropic/__init__.py, retarget the two module imports of the adapter in test_anthropic_adapter.py to the new path, and add tests/unittest/serve/api/anthropic/test_messages_behavior.py: characterization tests of what the aggregated and the disaggregated server answer on the Anthropic routes today (success, streaming, a stream without chunks, rejected requests, context length, upstream errors, unexpected errors, engine failures, an unconvertible chat response, and the aggregated Message Batches), with the served model and the requested model named differently so that each server's own answer is pinned. The follow-up PR that gives both servers one Anthropic Messages serving class must keep this file unchanged and green. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…feat-serve-anthropic-serving-class (integration base, not part of the reviewable change) Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ix-feat-serve-anthropic-serving-class (integration base, not part of the reviewable change) Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…erving classes The two classes receive the handlers that leave OpenAIServer in the next commit. Their constructors take what those handlers read from the server (the render-resources factory, the batch item runner), so missing wiring fails at construction instead of falling back to a default. The module is registered as a legacy-formatted file before any moved code lands in it, so the formatters keep the moved methods as they are; ruff-legacy.toml and the managed blocks of pyproject.toml and .pre-commit-config.yaml are regenerated from legacy-files.txt. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ndlers out of OpenAIServer Generated move: anthropic_count_tokens goes to AnthropicCountTokensServing and _batch_store, the six Message Batches handlers and _no_such_batch go to AnthropicBatchesServing in tensorrt_llm/serve/api/anthropic/serving.py, verbatim except for the declared rewrite in count_tokens (RenderResources.from_server(self) becomes self._render_resources(), the injected factory). anthropic_messages, _run_batched_message and _synthetic_request are deleted here and replaced by the serving class and the request context added in the next commit. The import cleanup is the formatters' (autoflake, isort, yapf). Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
… aggregated and disaggregated servers The Anthropic Messages orchestration existed three times (aggregated messages, the aggregated Message Batches runner, disaggregated messages), each serializing the chat result, parsing it back and sniffing the error body for a message; the batch runner entered the chat pipeline through a synthetic ASGI request. One AnthropicMessagesServing now runs it for any ChatBackend: the aggregated backend is the typed chat core create_chat_completion, the disaggregated backend is the entry-point wrapper's prepare, run and finish steps without the serialization step (the new _call_entry_point, in the same span). A RequestContext carries what the chat core reads from a request (headers, state, raw JSON, disconnect polling); the batch path builds one directly. The eight aggregated Anthropic routes come from the new router in tensorrt_llm/serve/api/anthropic/api_router.py, attached through the router table and gated by the new harmony_model capability as the route registration was gated. The launcher forwarding routes of an attached frontend register before the routers, so that they keep winning over the local Message Batches handlers. Each server keeps its own answers: the streamed model name and the error-message extraction come from its backend (stream_model, error_message); an engine failure stays a 500 on both. No behavior change. The new modules join the mypy file list; the two serving-class modules join the multi-GPU trigger table because they carry the disaggregated messages path. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ntext and the router on the real apps New tests: tests/unittest/serve/api/anthropic/test_serving.py (the serving class over a fake chat backend: the stream is reframed from the backend's own iterator, the backend's model and message reach the answer, each error kind keeps its status, the batch path hands the backend a non-streaming request and an internal context whose JSON is the converted request); tests/unittest/serve/_base/test_request_context.py (delegation to the HTTP request including its msgpack decoding, the internal context, the disconnect watcher returning at once for it); tests/unittest/serve/api/anthropic/test_api_router.py (the eight routes come from the router with the msgpack route class on the LLM roles only, none on a harmony model or another role, count_tokens decodes a msgpack body, attaching needs the serving objects). All three carry the cpu_only marker and run through the unittest/serve entry of the CPU list. Rewritten on the fixture factory: test_anthropic_routes.py posts to the real apps and their own validation handler instead of a mirror handler on bare servers, drives the batch and count-tokens routes through the serving objects on app.state, and hands the synthetic-request test to test_request_context.py; TestCountTokens in test_render_callers.py builds the count-tokens serving over a namespace. The chat-core and SSE-generator tests follow the renamed parameter; the multi-frontend tests set the Anthropic serving objects before attaching the routers. The object.__new__ ratchet drops to 2 and 1 sites for the two rewritten files; a layering contract keeps the Anthropic package from importing a server. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ared assembly of the trtllm-serve apps tensorrt_llm/serve/_base/app_builder.py holds the two functions both servers build on instead of a shared base class. build_http_app() assembles a FastAPI app in one fixed order: the lifespan, the optional route class before any route exists, the exception handlers, the routers in order, and the built-in middleware last so it stays outermost. serve_uvicorn() runs the app through the telemetry-aware uvicorn server, with an optional task that runs next to the server and receives it. The module is typed (added to the mypy file list) and imports only FastAPI, Starlette, uvicorn, the standard library and serve/_telemetry.py. Its tests pin the assembly order, the route class, the middleware stack order, the lifespan and exception-handler pass-through, every uvicorn argument, and the import boundary. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…m-serve apps with build_http_app and serve_uvicorn OpenAIServer and OpenAIDisaggServer construct their FastAPI app through build_http_app() and run uvicorn through serve_uvicorn(). Every difference between the two apps stays an explicit argument at the call site: the msgpack-decoding route class on the aggregated app and the KV-hint policy route class (KvHintsRoute) on the disaggregated frontend, both of which strip kv_hints from untrusted request bodies; the adjusted clock for the aggregated middleware and none for the disaggregated one; "info" and the 5 s keep-alive for the aggregated server, logger.level and the configured keep-alive for the disaggregated one; the worker-registration task after start only on the aggregated server. Both middleware installers read the server's perf-metrics attributes when they run, after the routes, as the aggregated server did before. Both servers keep reading TRTLLM_TRUST_KV_HINTS into app.state.trust_kv_hints, now right after the build; nothing reads the flag before the first request. No behavior change. The validation handlers keep their bodies and are registered with app.add_exception_handler instead of the decorator; the aggregated server's role dispatch goes unchanged into _attach_role_routes(app), and its post-start closure becomes the method _register_after_serving(server). On the disaggregated app the two built-in middlewares are now added after the routes instead of before; nothing in its route registration adds middleware, so the resulting stack is the same and the middleware-order snapshot is unchanged. Tests: tests/unittest/serve/test_server_uvicorn_settings.py builds both servers through the test factory and pins what each hands to uvicorn (log level, keep-alive, sockets, the post-start task only on the aggregated server) and what build_http_app must carry for each app: its route class on every API route and the KV-hint trust flag. The unit test that patched uvicorn through openai_disagg_server now patches it where serve_uvicorn looks it up and also checks the disaggregated log level. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
… one submodule per API tensorrt_llm/serve/openai_protocol.py held every OpenAI-dialect request and response model plus the helpers that turn them into engine parameters. It becomes the package tensorrt_llm/serve/openai_protocol/ with one submodule per API: common, sampling, tools, chat, completions, embeddings, responses, tokenize, models, kv_cache, control and disagg. Every definition moves unchanged (93 definitions; generated by an AST tool and checked definition by definition against the former module). The package __init__.py is a lazy facade: a name-to-module table and a module __getattr__ import a submodule only when one of its names is first read, so every import from tensorrt_llm.serve.openai_protocol keeps working for every name the former module had, the underscore helpers that tests import included, and the VisualGen models re-exported from tensorrt_llm.serve.api.visual_gen.protocol. The facade is the package's own interface, not a compatibility shim: no warning, no removal release, and no importer in the repository changes. Its TYPE_CHECKING block imports the public names from their submodules so type checkers resolve them; the VisualGen names are left out of it on purpose and appear only as strings in the table, so that importing the facade never loads the VisualGen package. DisaggScheduleStyle, which the former module imported and two importers took from it, stays reachable through the facade. Submodules import each other directly, never through the package, along an acyclic graph: common imports no sibling; sampling, tools, models, tokenize, embeddings and control import common; completions and responses import common and sampling; chat imports common, sampling and tools; kv_cache imports common, tools and chat; disagg imports common, completions and chat. torch is imported only by sampling and disagg, openai_harmony only by chat, and the serving-extension registry only by sampling, so code that needs only the tool types no longer loads the chat or Responses models. The 12 submodules are registered in legacy-files.txt (their code comes from a legacy-formatted file) and the deleted module's line is removed; the derived lint configs are regenerated with scripts/legacy_utils.py gen-configs. The facade is new code and stays under the default formatter. No behavior change. OpenAPI component names may change for classes that share a short name with an openai type, because pydantic qualifies them with the module path; they are not a contract. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…d app assembly branch as the base for the per-dialect error renderers Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ziness and layering tests/unittest/serve/openai_protocol/test_openai_protocol_facade.py keeps the package tensorrt_llm/serve/openai_protocol/ honest: every name the former single-file module defined (plus DisaggScheduleStyle) is reachable through the facade and is the object its submodule defines; the facade table is frozen at those names; __all__, dir() and the TYPE_CHECKING block agree with the table; unknown names raise AttributeError; the submodule import graph stays inside the allowed acyclic graph and no submodule imports the package itself; importing the package loads no submodule, importing tools loads neither chat nor responses nor completions, and reading ChatCompletionRequest does not load the serving extensions; the private helpers that other tests import through the package stay importable. tests/unittest/serve/api/visual_gen/test_openai_protocol_facade_visual_gen.py holds the VisualGen assertions under a visual_gen/ path so that CI selection runs them for VisualGen-only changes: the eight VisualGen models map to tensorrt_llm.serve.api.visual_gen.protocol, nothing but a lookup of one of them loads that module, and the facade names it only as strings in its table, never in an import statement. tests/unittest/serve/test_layering.py gains the contract that the protocol package imports no handler module (servers, handlers and routers import the models, never the reverse), and tensorrt_llm/serve/AGENTS.md tells new code to import the submodule that defines a model rather than the package. Both test files are collected by the existing unittest/serve and unittest/serve/api/visual_gen/ entries of l0_cpu.yml and carry the cpu_only marker. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…s as strings and bind the test's culprit list unconditionally The table comment in tensorrt_llm/serve/openai_protocol/__init__.py said that a TYPE_CHECKING import of the VisualGen models would load their package at run time, which a TYPE_CHECKING block never does. The real reason is the static import-isolation check that lets CI run only the VisualGen tests for VisualGen-only changes: it forbids an import edge from this package to the VisualGen package even under TYPE_CHECKING, so the facade names that module only as a string. In tests/unittest/serve/openai_protocol/test_openai_protocol_facade.py the list of submodules that import the serving extensions was bound only on the failing path and read in the assertion message; it is now bound before the branch, so an unconditional message can never raise UnboundLocalError. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…serve apps before rendering them per dialect Posts invalid bodies to servers built through the real constructors and checks status, body and side effects: the aggregated server's OpenAI shape with its 4096-character cap, the Anthropic envelope, the VisualGen 422 envelope and the metrics call; the disaggregated frontend's uncapped OpenAI shape, Anthropic envelope, counter and throttled log with escaped control characters. The validation text is read from the exception FastAPI raised, so the test does not re-implement the rendering. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…error boundary and the per-dialect error registry tensorrt_llm/serve/_base/errors.py gains the dialect-neutral error types a chat backend raises for any wire dialect to render (DialectNeutralError with InvalidRequestError, ContextLengthExceededError, UpstreamError, UnavailableError, InternalServingError; neutral_error picks the type for an HTTP status), ErrorRule and serving_error_boundary, the error ladder of an HTTP handler as a declaration, with CHAT_LADDER for the chat completion wrapper. The OpenAI body keeps its single builder, error_body. tensorrt_llm/serve/exception_handling/register.py is the per-app registry of error renderers keyed by (dialect, exception type): route_dialect marks an endpoint's dialect, dialect_of reads it from the matched endpoint, default_error_renderers gives the OpenAI and Anthropic renderers for validation errors and the typed errors, and install_exception_handlers puts the registry on app.state and installs the app-level validation handler with a per-server observer for side effects. The module imports no server or VisualGen code; the layering test holds that. Typed (mypy strict), with unit tests for the registry, the boundary and the neutral errors, including that the OpenAI rendering of a neutral error equals OpenAIServer.create_error_response and that the chat completion wrapper answers as before through the boundary. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…re dialect
Both servers replace their validation handler's path test
(request.url.path.startswith("/v1/messages")) and the aggregated server's
VisualGen role test with the per-dialect registry: install_exception_handlers
resolves the dialect of the matched endpoint (the eight aggregated and two
disaggregated Anthropic endpoints carry @route_dialect(ANTHROPIC)), falls back
to the server's default dialect (VisualGen on the VisualGen role, OpenAI
otherwise), calls the server's _observe_validation_error for its side effects
and renders through the registry. Side effects stay as they were: the
aggregated server counts a rejection on its metrics collector and caps the
OpenAI-shaped text at 4096 characters; the disaggregated frontend counts on
its Prometheus counter and logs the first and every 1000th OpenAI-shaped
rejection. The VisualGen 422 renderer is registered with the VisualGen routes,
for that role only, and a VisualGen server without it fails at startup.
The chat completion wrapper's four except branches become
@serving_error_boundary(OPENAI, ladder=CHAT_LADDER, on_engine_fatal=lambda:
None) with the same answers. The Anthropic route tests install the servers'
own handler instead of a copy of it. The multi-GPU trigger list gains
tensorrt_llm/serve/exception_handling/.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…a layering contract test_registry_modules_import_no_visual_gen scanned every import statement of serve/exception_handling/register.py and serve/_base/errors.py, at any scope including if TYPE_CHECKING:, for three VisualGen modules. The layering contract for serve/exception_handling/ already forbids those modules for register.py at run time, so the scan's own part was serve/_base/errors.py and the type-checking imports of both files. A layering contract over serve/exception_handling/ and serve/_base/errors.py, with the same three modules and type-checking imports included, replaces the test and its helpers. The contract also resolves relative imports and from-imports of a submodule through its parent package (from tensorrt_llm.serve import openai_video_routes), which the scan, matching module names only, did not see. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ke completions and Responses With TRTLLM_KVCACHE_TIME_OUTPUT_PATH set to a non-empty value, the completions handler, the gpt-oss chat handler and the Responses preprocessing force SamplingParams.return_perf_metrics=True for every request; the chat completions handler did not. Chat completion requests (and the Anthropic Messages routes and /health_generate, which run through the chat handler) now request per-request engine perf metrics in the same way, with the same statements as the other three sites. Response bodies, status codes and headers are unchanged. The new CPU tests build the server through the fixture factory and read the sampling_params the handlers hand to the scripted generate_async, for /v1/chat/completions (variable set and unset), /v1/messages, /v1/messages/batches, /health_generate and the pre-existing /v1/completions behavior. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…outer and pass the kv-hints handlers a request tests/unittest/llmapi/apps/test_responses_usage.py mounted OpenAIServer.anthropic_messages, which no longer exists: the Anthropic routes live in tensorrt_llm/serve/api/anthropic/api_router.py and read their serving object from app.state. The test's bare app now mounts the router's route function and carries the AnthropicMessagesServing the server's constructor built, so the Messages route still runs over the same chat core as the chat route. tests/unittest/llmapi/test_kv_hints.py called the handlers with raw_request=None. The chat wrapper now wraps the request in a RequestContext, which needs an object to delegate to, so the handlers get a minimal request (empty headers, an empty state), as the chat core tests do. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ix-feat-serve-gpt-oss-openai-chat (integration base, not part of the reviewable change) Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…-junyix-feat-serve-gpt-oss-openai-chat (integration base, not part of the reviewable change) Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…-feat-serve-gpt-oss-openai-chat (integration base, not part of the reviewable change) Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…-junyix-feat-serve-gpt-oss-openai-chat (integration base, not part of the reviewable change) Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…d chat handler gpt-oss (harmony) chat requests go through openai_chat, which asks the gpt-oss serving extension for everything model-specific: harmony prompt rendering (render_prompt), token-level output (output_mode), the harmony stop tokens (the new extra_stop_token_ids hook) and the request features the harmony path does not provide (the new unsupported_features hook, with the UnsupportedFeature names and the legacy rejection messages). Every rejection the dedicated chat_harmony handler makes stays, with the same status and message; the handler stays reachable behind TRTLLM_GPT_OSS_LEGACY_CHAT=1, which also keeps /health_generate on the text path, until a future release removes the switch. Structured output for a token-output model is placed by the extension's structured_output_format hook, passed to to_sampling_params through the new keyword-only structured_output_format parameter, so the final-channel placement no longer depends on a reasoning-parser name. harmony_enabled_for asks the extension registry (the model type's extension consumes harmony tokens) instead of comparing a model name, and RenderResources.harmony_hooks is the extension whose token-level hooks apply with the harmony switch. The shared handler hands the engine streaming=bool(request.stream): a null stream field is a non-streaming request on every model, as chat_harmony already treated it, where the generic path passed None. gpt_oss becomes an accepted --tool_parser / --reasoning_parser value (a serving-extension CLI alias, registered next to the extension); the CLI splits it off before the parser registries and the engine arguments see it (once, when both flags name it), and a gpt-oss server configured with any other tool or reasoning parser now fails at startup, except with DISABLE_HARMONY_ADAPTER=1. With --grpc the alias is rejected like the other HTTP-only options. The --post_processor_hook guard reads the extension's output mode. The server factory used by the CPU tests gains a parser_aliases field so the startup check can be exercised through the real constructor. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…e new serving-extension hooks tests/unittest/serve/extensions/test_gpt_oss_chat_path.py builds gpt-oss servers through the real constructor and sends the same request bodies through openai_chat and the legacy chat_harmony handler (mounted on a second route for the comparison): every rejection answers with the same status and body (the pairs include the two features checked before rendering), the engine receives the same harmony prompt token ids and sampling parameters for non-streaming and streaming requests, a null stream field reaches the engine as streaming=False on both paths, the structured-output constraint sits on the final channel, and only the listed capabilities differ (cache_salt, the preprocess hook, prompt_token_ids_b64 from any client, the engine-failure answer, the type of unexpected and over-length errors). The switch, /health_generate on both paths, DISABLE_HARMONY_ADAPTER=1, the startup check, the hook guard and the CLI wiring of the gpt_oss alias (reaches launch_server as parser_aliases, never the parser arguments; rejected with --grpc) are pinned too. The serving-extension tests cover the defaults of the two new hooks, the gpt-oss declarations, the CLI alias registry and the generic chat path; the router test compares the KV-aware router's harmony tokens with what the default handler sends; the disaggregated placement test gains a gpt-oss worker built through the real constructor; the post-processor-hook guard test passes the output mode. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
JunyiXu-nv
force-pushed
the
dev-junyix-feat-serve-gpt-oss-openai-chat
branch
from
November 6, 2026 03:45
c17e7d8 to
ed6663c
Compare
This branch has not been deployed
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.
@coderabbitai summary
Stacked on #21551 (Converge the three Anthropic orchestrations into one serving class) and the open PRs that PR itself stacks on: #21406 (Add route, middleware-order and shim-identity snapshots and a CPU test seam for trtllm-serve), #21448 (Add the trtllm-serve router skeleton and move /health, /version and /v1/models into ops.system), #21442 (Split the disaggregated server's entry-point wrapper into prepare, run, finish and serialize steps), #21515 (Split the chat handler into a typed core and lift the SSE generator into serve/_base) and #21518 (Move the Anthropic serving modules into serve/api/anthropic). The first commits below belong to those PRs and disappear from this diff as they merge. On top of that stack, four more open PRs are merged in as the integration base, each as one
Merge branch ... (integration base, not part of the reviewable change)commit: #21566 (Add per-dialect error renderers in serve/exception_handling; it carries #21556 Extract the shared HTTP app assembly into serve/_base/app_builder.py and #21447 Shared middleware package for the aggregated and disaggregated servers), #21544 (Split openai_protocol into a package; it carries #21509 Move the VisualGen protocol models into serve/api/visual_gen), #21170 (Add trtllm-serve CI coverage lint and a multi-GPU trigger for chat_tokenization.py) and #21667 (Chat completions honor TRTLLM_KVCACHE_TIME_OUTPUT_PATH like completions and Responses). This PR's own change is the last 2 commits; reviewgit diff 23b255e337...HEAD.Description
gpt-oss (harmony) chat completions stop going to the dedicated
chat_harmonyhandler and go throughopenai_chat, which asks the gpt-oss serving extension for everything model-specific: harmony prompt rendering (render_prompt), token-level output (output_mode), the harmony stop tokens (new hookextra_stop_token_ids) and the request features the harmony path does not provide (new hookunsupported_features, withUnsupportedFeaturenames and the legacy rejection messages). Every rejectionchat_harmonymakes today stays, with the same status and body; the capabilities gpt-oss gains from the shared handler are listed below.chat_harmonystays reachable behindTRTLLM_GPT_OSS_LEGACY_CHAT=1until a future release removes the switch.Also in this PR: structured output for a token-output model is placed by the extension's
structured_output_formathook, passed toto_sampling_paramsthrough a new keyword-onlystructured_output_formatparameter (so the final-channel placement no longer depends on the parser namegpt_oss);harmony_enabled_forasks the extension registry instead of comparing a model name, andRenderResources.harmony_hooksis the extension whose token-level hooks apply under the harmony switch;gpt_ossbecomes an accepted--tool_parser/--reasoning_parservalue (a serving-extension CLI alias registered next to the extension, split off by the CLI before the parser registries and the engine arguments see it) and a gpt-oss server configured with any other parser fails at startup; the--post_processor_hookguard reads the extension's output mode.Part of the trtllm-serve refactor (epic TRTLLM-17056, task TRTLLM-17268: gpt-oss onto the serving-extension hooks). No earlier PR of this refactor has merged into
mainyet: this PR stacks on #21551 and its stack, with #21566, #21544, #21170 and #21667 merged in as its base (first paragraph). That base is why the edits sit where they do. It carries the typed chat core and the shared chat SSE generator (#21515), so the token-output path is written intocreate_chat_completioninserve/openai_server.py; the chat serving class has not been written yet, so the route choice stays inregister_routes. It carries theopenai_protocolpackage (#21544), so the structured-output parameter lives inserve/openai_protocol/sampling.pyandchat.py. The new tests build servers with the test factory and fakes of #21406 and #21448. The multi-GPU trigger forchat_tokenization.py(#21170) and the KV-cache time output for every chat request (#21667) are the two prerequisites this change needs merged before it. The reasoning-parser package, the serve CLI package, the config sources and the typed env module do not exist yet, so the CLI edits are incommands/serve.pyand the switch is read withos.getenvin_init_llm.Behavior
Behavior changes, for gpt-oss (harmony) deployments; the last item also touches every other model.
/v1/chat/completionshandler:chat_harmony->openai_chatwith the gpt-oss serving extension's hooks.cache_saltis honored (was ignored).prompt_token_ids_b64is decoded on generation workers and encoded on context workers when requested (was ignored). It is honored from any client on every gpt-oss server, not only from the disaggregated orchestrator (the field is not a protected disaggregated field), and a malformed value answers 400BadRequestError.preprocesshook runs (was skipped).CppExecutorError: SIGINT, exit-code telemetry and HTTP 200 with bodynull, as for every other model (was 400internal_error, no SIGINT, no telemetry).type="BadRequestError"with the traceback logged at ERROR (wastype="internal_error"with the traceback at DEBUG; the ERROR log of the full messages, tools and request on a rendering failure is gone).RequestErrorbefore the response starts):typeinternal_error->BadRequestError; status 400 andcode="context_length_exceeded"unchanged./openapi.jsonof a gpt-oss server:POST /v1/chat/completionshasoperationIdopenai_chat_v1_chat_completions_post(waschat_harmony_v1_chat_completions_post),summaryOpenai Chat(wasChat Harmony) and nodescription(waschat_harmony's docstring); the switch restores the old document.nvtx_mark("generation ends")is emitted (profiling marker only). Disconnect logging is unchanged: the chat stream's bareexcept:was fixed before this PR in [None][fix] Anthropic Messages: read the chat stream to its end so its metrics are recorded #20867./health_generateon gpt-oss renders with harmony and uses the harmony post-processor (was the HF chat template and the text post-processor).--tool_parser/--reasoning_parservalues other thangpt_ossnow fail on a gpt-oss server (they were silently ignored: gpt-oss parses tool calls and reasoning from its token stream); skipped withDISABLE_HARMONY_ADAPTER=1. Release note needed.--tool_parser gpt_ossand--reasoning_parser gpt_oss: the explicit choice of harmony parsing TRTLLM-11399 asks for. The alias never reachesLlmArgs.reasoning_parseror the parser registries.--grpcwith--tool_parser gpt_ossor--reasoning_parser gpt_ossfails withArgument 'parser alias' is not supported when running in gRPC mode. ...(was Click's invalid-choice usage error).TRTLLM_KVCACHE_TIME_OUTPUT_PATH: unchanged for gpt-oss, because [None][fix] Chat completions honor TRTLLM_KVCACHE_TIME_OUTPUT_PATH like completions and Responses #21667 applies it to every chat request; pinned bytest_kvcache_time_output_requests_metrics_for_gpt_oss.top_logprobs, a sampling-parameter error (for example an out-of-rangelogit_biasid), a thinking budget or an internal-disaggregation validation error now reports that other error, because rendering runs after validation; every other combination reports the same error as before."stream": nullis served as a non-streaming request on every model: the shared chat handler now passesstreaming=bool(request.stream)to the engine. gpt-oss already did so throughchat_harmony; the generic chat path passedstreaming=None, and the executor's request binding (tensorrt_llm.bindings.executor.Request) rejectsstreaming=NonewithTypeError: incompatible function arguments(checked by constructing the binding'sRequestdirectly withFalse,TrueandNone). Pinned bytest_stream_null_is_a_non_streaming_requeston both paths.Unchanged on purpose:
chat_templateon a server that does not allow it (400,chat_template cannot be supplied per request unless request-level chat templates are enabled at server startup.); a namedtool_choice(400,tool_choice with a named function is not yet supported for harmony / GPT-OSS models. Tracking follow-up: harmony sub-task of TRTLLM-12758.);tool_choice="required"(400,tool_choice='required' is not supported for this model.);logprobswithtop_logprobs > 0(400,`gather_generation_logits` must be `True` to use `top_logprobs`);thinking_token_budget(400,thinking_token_budget is not supported by the Harmony GPT-OSS serving path; use reasoning_effort instead). The parity test sends each through both handlers and compares status and body.n > 1,echoandlogprobs=true(accepted, never requested from the engine) behave as before; the Anthropic routes are still not mounted on an aggregated gpt-oss server;DISABLE_HARMONY_ADAPTER=1deployments are unaffected (text path, text parsers, no startup check beyond a misplaced alias)./v1/chat/completions, so the behavior changes above also apply to Anthropic Messages served over disaggregated gpt-oss workers (the adapter mapsthinking.budget_tokenstothinking_token_budgetandoutput_config.formattoresponse_format, which the two handlers reject or place identically).Conditional risk, strict JSON on historical tool-call arguments: resolved rather than listed. The token-output path does not parse the messages (the harmony adapter renders them itself, as
chat_harmonydid), so an assistant turn whosetool_calls[].function.argumentsis not JSON is still accepted;test_history_with_non_json_tool_arguments_is_acceptedpins it.Kill switch:
TRTLLM_GPT_OSS_LEGACY_CHAT=1restores thechat_harmonyroute and makes the shared handler use the generic hooks for gpt-oss, so/health_generatebehaves as before; it is logged once at WARNING at startup. It does not undo the startup check, the CLI aliases or the registry-derived harmony switch. It will be removed in a future release; the warning says so without naming the release.Rolling upgrade in disaggregated serving: a context worker with this change answers
return_prompt_token_ids_b64withprompt_token_ids_b64; a generation worker without it (chat_harmony) ignores the field and re-renders the same harmony tokens from the messages. No code is needed.Relation to TRTLLM-11399: this is the chat half of that ticket's scope as the refactor reads it. No new tool or reasoning parser and no parser-interface change; gpt-oss goes through serving-extension hooks; the automatic default stays (harmony on for
model_typegpt_oss, off withDISABLE_HARMONY_ADAPTER=1); "explicit" is met by thegpt_ossaliases and the startup check against other parsers. The Responses path removes its harmony branches in a separate PR.Moves
None.
chat_harmonystays where it is, unchanged except that only the switch reaches it.CI bookkeeping
tensorrt_llm/; the changed filesserve/openai_server.py,serve/serving_extensions.py,serve/extensions/,serve/render/andserve/openai_protocol/are already listed, so the multi-GPU stages run and need theci: full pre-merge approvedlabel).tensorrt_llm/);ruff-legacy-baseline.json: none.tests/unittest/serve/extensions/test_gpt_oss_chat_path.pyis collected by the existingunittest/serveentry ofl0_cpu.ymland containspytest.mark.cpu_only. The test helpertests/test_common/serve/app_factory.pygainsAggConfig.parser_aliasesso the startup check can be exercised through the real constructor.Test Coverage
New:
tests/unittest/serve/extensions/test_gpt_oss_chat_path.py(47 tests): route selection with and without the switch (log capture with a positive control), rejection parity withchat_harmonyover nine request shapes (the pairs include the two pre-render features together), the generation inputs both handlers send for non-streaming and streaming requests (prompt token ids, every compared sampling field, structural tag, post-processors, the SSE frames, the single gained keywordcache_salt),"stream": nullon both paths, structured output on the final channel vs the raw grammar on a generic model,logprobswithouttop_logprobs, non-JSON historical tool arguments, the gained capabilities (cache_salt, preprocess,prompt_token_ids_b64from any client and its malformed values, engine failure answered 200/null with SIGINT vs 400internal_errorthroughchat_harmony), the error types for unexpected and over-length errors on both handlers,DISABLE_HARMONY_ADAPTER=1, the startup-check matrix and its wiring at construction, the hook guard,/health_generateon both paths, the KV-cache time output, and the CLI wiring of thegpt_ossalias (TestCliAliasWiring: the alias reacheslaunch_serverasparser_aliases=("gpt_oss",), neverLlmArgs.reasoning_parseror the positionaltool_parser, deduplicated when both flags name it;--grpcwith either alias flag fails with the gRPC "not supported" error).Edited:
tests/unittest/llmapi/apps/test_serving_extensions.py(hook defaults, declared features are known, gpt-oss declarations, CLI aliases, the generic chat path through the factory),tests/unittest/disaggregated/test_router.py(router tokens equal the default handler's input; original tool-conversion error kept throughopenai_chat),tests/unittest/disaggregated/test_openai_disagg_service.py(a gpt-oss worker built through the real constructor in the placement test),tests/unittest/executor/test_postprocessor_hook.py(the guard takes the output mode),tests/unittest/api_stability/references/trtllm_serve_cli.yaml(the twoChoicelines gain'gpt_oss').CPU runs (no GPU on the development host; everything GPU-only is left to CI):
pytest -m cpu_only tests/unittest/serve tests/unittest/llmapi tests/unittest/disaggregated tests/unittest/api_stability tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py -q -p no:randomlyon this head (run before the last push, which changed only the switch's warning text and a code comment): 111 failed, 7726 passed, 3 skipped, 2 xfailed. The same command on the reviewable base23b255e337(the independent reviewer's run): 111 failed, 7651 passed, 3 skipped, 2 xfailed. Compared by test id and first-line message, the 111 failing ids are the same on both sides with the same messages (test_config_database.py62,test_context_first_profile.py26,test_lifecycle_activation.py16,test_mpi_session.py3,test_peer.py2,test_executor.py1,test_llm_utils.py1), all pre-existing at the base and unrelated to this change; no test fails only on this head.pytest -rs tests/unittest/serve/extensions/test_gpt_oss_chat_path.py tests/unittest/llmapi/apps/test_serving_extensions.py tests/unittest/disaggregated/test_router.py::<the four gpt-oss tests> tests/unittest/disaggregated/test_openai_disagg_service.py::test_subagent_affinity_http_worker_adp_placement tests/unittest/executor/test_postprocessor_hook.py -q-> 252 passed, no skip (test_postprocessor_hook.pyrun without-m cpu_only, as its CI stage does).LLM_MODELS_ROOT=... pytest -rs tests/unittest/llmapi/apps/test_harmony_parsing.py tests/unittest/llmapi/apps/test_harmony_channel_validation.py -q-> 79 passed, no skip (nothing "not importable").pytest tests/unittest/api_stability -q(whole directory, no-m): passes with the two changedChoicelines and nothing else (part of the run above;test_serve_cli.pyandtest_serve_api.pyincluded).PYTHONPATH=tests python3 -m test_common.serve.routes-> exit 0;tests/unittest/api_stability/references/trtllm_serve_routes.yamlis unchanged (the route set, methods and route classes do not change).fastapi==0.136.3and0.139.2trees onPYTHONPATH):pytest -m cpu_only tests/unittest/api_stability/test_serve_routes.py tests/unittest/serve/api/visual_gen/test_routes_visual_gen.py tests/unittest/serve/extensions/test_gpt_oss_chat_path.py tests/unittest/serve/exception_handling/test_server_validation_errors.py -q-> 92 passed on each.tests/unittest/disaggregatedis part of the first run (thetest_context_first_profile.py/test_lifecycle_activation.py/test_peer.pyfailures are pre-existing, see above).trtllm-serve serve --help:gpt_osslisted in both--reasoning_parserand--tool_parser;--tool_parser bogusis still rejected by Click. The CLI plumbing and the gRPC rejection are pinned on CPU byTestCliAliasWiring(the serve command driven through Click withdevice_count,get_is_diffusion_only_modelandlaunch_serverpatched, as the existing serve CLI wiring tests do). A gpt-oss server with--tool_parser qwen3failing at startup is covered on CPU bytest_startup_check_runs_at_constructionand on GPU by the H100 e2e tests.harmony_hooks, the gpt-oss declarations, the alias registry) were each applied alone in an isolated worktree and each was caught by at least one of the new tests (19/19); three more from the independent review (stop ids ordetokenizeleft unchanged on streaming requests only, the two pre-render rejections checked in the other order) are caught by the streaming cases of the generation-inputs test and thenamed_tool_choice+top_logprobsrejection case (3/3).gpt_ossparser-name path,harmony_enabled_foragainst the old rule,harmony_hooks): no counter-example in the generated inputs (300/200/600/300/300/200 examples per property); a 14-mutation gate planted one bug at a time in the production files and every property that constrains it failed (14 detected, 0 missed). A green property run is not a proof: it says no counter-example was found within the generator's bounds.pre-commit run --files <all changed files>twice: no modification on the second run.scripts/legacy_utils.py check-configs: up to date.scripts/check_serve_ci_coverage.py: 3 checks passed.GPU, left to CI:
tests/integration/defs/test_e2e.py::test_openai_chat_harmonyand::test_openai_chat_harmony_perf_metrics(H100), with and withoutTRTLLM_GPT_OSS_LEGACY_CHAT=1;test_disaggregated_gpt_oss_120b_harmonyandtest_disaggregated_gpt_oss_20b_kv_cache_aware(multi-GPU; need theci: full pre-merge approvedlabel).PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
Any new dependencies have been scanned for license and vulnerabilities
I have read and understood the CODING GUIDELINES and this PR follows them.
🤖 Generated with Claude Code