Repository navigation
[TRTLLM-17264][refactor] Move the VisualGen protocol models into serve/api/visual_gen - #21509
Draft
JunyiXu-nv wants to merge 16 commits into
Draft
JunyiXu-nv wants to merge 16 commits into
JunyiXu-nv wants to merge 16 commits into
Conversation
This was referenced Nov 3, 2026
…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>
…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>
JunyiXu-nv
force-pushed
the
dev-junyix-feat-serve-visualgen-protocol-package
branch
from
November 4, 2026 12:50
4e58cfe to
18a8b2f
Compare
JunyiXu-nv
added a commit
to JunyiXu-nv/TensorRT-LLM
that referenced
this pull request
Nov 4, 2026
…e VisualGen protocol-package branch (NVIDIA#21509) Integration base for the VisualGen handler move: both open PRs must be on main before the handler move can merge, and the move is regenerated on the latest main at that time. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
1 task done
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.
Stacked on #21448 ([TRTLLM-17261][refactor] Add the trtllm-serve router skeleton and move /health, /version and /v1/models into ops.system), which is itself stacked on #21406 ([TRTLLM-17261][test] Add route, middleware-order and shim-identity snapshots and a CPU test seam for trtllm-serve): the first commits below belong to those PRs and disappear from this diff when they merge; this PR's own change is the last 4 commits.
@coderabbitai summary
Description
Move the eight VisualGen request and response models of
trtllm-serve(ImageGenerationRequest,ImageEditRequest,ImageObject,ImageGenerationResponse,MediaReferenceItem,VideoGenerationRequest,VideoJob,VideoJobList) out oftensorrt_llm/serve/openai_protocol.pyinto the new packagetensorrt_llm/serve/api/visual_gen/(protocol.py), unchanged, so that VisualGen protocol changes land in the package that CI selection will later claim for VisualGen-only pull requests.openai_protocolkeeps every old name importable through a lazy module__getattr__: importing it no longer loads the VisualGen models ortensorrt_llm.visual_gen, and the re-export is part ofopenai_protocol's own interface, so it never warns and has no removal release. The same PR givestensorrt_llm/serve/api/visual_gen/to the VisualGen team in.github/CODEOWNERSand adds an api-stability gate for the pinned VisualGen models under avisual_gen/path, because once the package is claimed, the existing serve API gate would not run on VisualGen-only PRs.Part of the trtllm-serve refactor (epic TRTLLM-17056, task TRTLLM-17264: VisualGen isolation). Builds on #21448 (the router skeleton creates
tensorrt_llm/serve/api/) and #21406 (the shim identity table this PR adds rows to). Should follow #21408 ([TRTLLM-17264][refactor] Merge the two VisualGen validation-error renderers), which is independent of this change.Behavior
No behavior change. OpenAPI component names may change with the move; they are not a contract.
Until the VisualGen handlers move out of
openai_server.py, every LLM role still imports the new module throughopenai_server.py's image-model imports; the import-isolation tests arrive with the PRs that move the handlers and let CI selection claimserve/api/visual_gen/.Moves
Pure-move check
Pure-move check: base d3aacdd, head 18a8b2f, tools 517fc468330a
ImageEditRequestImageGenerationRequestImageGenerationResponseImageObjectMediaReferenceItemVideoGenerationRequestVideoJobListVideoJob8 definitions moved verbatim; 0 differences; shim exports 8 names
One observable difference for star-imports:
from tensorrt_llm.serve.openai_protocol import *no longer brings in the eight names (the module has no__all__, and lazily re-exported names are invisible to a star-import); nothing in the repository star-imports them, and explicit imports keep working.Coordination with work in flight: #21226 ([TRTLLM-16807][feat] Support combined Omni references in video serving) edits
VideoGenerationRequest.image_reference,video_referenceandaudio_referenceinside the moved block. If it lands first, this move is regenerated on top of it right before merging; if it lands second, it re-applies that hunk totensorrt_llm/serve/api/visual_gen/protocol.py(same text, new file). #21408 adds import lines next to the ones rewritten here; whichever lands second resolves a trivial import-block conflict.This re-export is
openai_protocol's permanent interface: noFutureWarning, no removal release, and the eight names have identity-only rows in the VisualGen shim identity table (tests/unittest/serve/api/visual_gen/test_shim_identity_visual_gen.py: no warning check, no ban on the old path). Whenopenai_protocol.pyis later split into a package with a lazy facade, the eight names become rows of that facade's name-to-module table and this block disappears into it. No shim is kept and no old path is deleted:openai_protocolkeeps its path. Code inside the repository imports the new module (every importer is rewritten in this PR). History:git blame -C -C tensorrt_llm/serve/api/visual_gen/protocol.pyattributes the moved lines to their original commits.CI bookkeeping
tensorrt_llm/serve/api/visual_gen/is deliberately not added: VisualGen serving is to be claimed by CI selection for VisualGen-only PRs, and a prefix there would drag it into the multi-GPU stages.tensorrt_llm/serve/openai_server.pyis in the table and its import lines change, so this PR runs the multi-GPU stages (approval label needed)./tensorrt_llm/serve/api/visual_gen @NVIDIA/trt-llm-torch-visual-gen-devs, in the VisualGen section (after the/tensorrt_llm/serverule, so it wins). Effect: VisualGen serving files (protocol.pynow, the handlers later) need the VisualGen team's approval and no longer need runtime-devs.tensorrt_llm/serve/api/visual_gen/protocol.pyadded (legacy-formatted group, because its code comes fromopenai_protocol.py, which is in that group); derived configs regenerated withscripts/legacy_utils.py gen-configs(ruff-legacy.toml, the managed blocks ofpyproject.tomland.pre-commit-config.yaml);ruff-legacy-baseline.json: none (openai_protocol.pyhas no baseline entry, andruff check --config ruff-legacy.tomlpasses on both files).tests/integration/test_lists/test-db/l0_cpu.ymlgainsunittest/api_stability/visual_gen/test_visual_gen_serve_api.pyright afterunittest/api_stability. The new file is also collected by the directory entry on unclaimed PRs (about one second twice); its own entry is what keeps it running on VisualGen-only PRs, whose CI selection keeps only entries containingvisual_gen/.tensorrt_llm/serve/scripts/BENCHMARKING_VISUAL_GEN.mdlinks the new file. The published module pathtensorrt_llm.serve.scripts.benchmark_visual_gendoes not change.The VisualGen api-stability gate
tests/unittest/api_stability/visual_gen/test_visual_gen_serve_api.pyloads the checks oftest_serve_api.pyand runs them on the models ofreferences/trtllm_serve_api.yamlthat resolve, throughtensorrt_llm.serve.openai_protocol, to classes defined intensorrt_llm.serve.api.visual_gen.protocol(todayImageGenerationRequest,ImageGenerationResponse,MediaReferenceItem,VideoGenerationRequest); it also checks that importingopenai_protocoldoes not load the VisualGen module, that the eight names are listed bydir(), and that unknown names still raiseAttributeError. It lives undertests/unittest/api_stability/so that the API review committee stays its owner (the alternative,tests/unittest/_torch/visual_gen/, would have made the VisualGen team the only owner of a gate on their own models). The reference YAML is unchanged, so noapi-compatiblelabel is needed.Test Coverage
All checks ran on a CPU-only host from this branch's worktree (
PYTHONPATHoverlay over a prebuilt wheel); GPU end-to-end tests are left to CI.pytest -m cpu_only tests/unittest/api_stability/visual_gen/test_visual_gen_serve_api.py→ 9 passed (the 7 inherited checks on the VisualGen models + 2 lazy re-export tests). Mutation proofs, run and reverted: renaming a pinned model inprotocol.pyturns the gate red (7 errors: "Model 'ImageGenerationResponse' is in the stability reference YAML but not in tensorrt_llm.serve.openai_protocol"); an eager VisualGen import added totensorrt_llm/serve/__init__.pyfails the probe with the message naming the modules that hold the classes.pytest tests/unittest/api_stability/test_serve_api.py→ 7 passed, unchanged: the reference YAML resolves through the lazy re-export.pytest -m cpu_only tests/unittest/_torch/visual_gen/test_trtllm_serve_endpoints.py tests/unittest/_torch/visual_gen/test_visual_gen_utils.py tests/unittest/_torch/visual_gen/test_visual_gen_params.py→ 323 passed, the same count as on the base branch.tests/unittest/serve/test_shim_identity.py,tests/unittest/serve/api/visual_gen/test_shim_identity_visual_gen.py) → 11 passed with the 8 new identity-only rows.-rs→ 79 passed, no "not importable" skip;tests/unittest/llmapi/apps/test_serve_lazy_import.py→ 5 passed; route snapshot (test_serve_routes.py,test_routes_visual_gen.py) → 36 passed, no route change;tests/unittest/serveandtests/unittest/serve/api/visual_gen/→ 112 passed (layering test included).PYTHONPATH=tests python3 -m test_common.serve.cpu_tests, every test-db entry it selects): green except two environmental failures that reproduce identically on the base branch —test_render_standalone.py::TestCpuOnlyStartup::test_the_launcher_loads_a_cuda_stub_only_when_the_driver_is_missing(thetrtllm-renderconsole script is not installed in the overlay) and OpenEnginetest_frontend_lifecycle.pychild-process waits timing out on a heavily loaded host (14/14 on a quieter re-run).tensorrt_llm.serve.openai_protocoldoes not loadtensorrt_llm.serve.api.visual_gen.protocol; the first read of each of the 8 names emits no FutureWarning or DeprecationWarning; each name is the same object as in the new module; unknown names raise AttributeError;dir()lists all 8.dir()), with a 6-mutation gate that every property catches (local harness, not committed).git grepfor old-path imports of the 8 names undertensorrt_llm/andtests/: none;MediaRole,UploadFile,AliasChoicesinopenai_protocol.py: none; no test patches these names.python scripts/legacy_utils.py check-configspasses;python scripts/legacy_utils.py lint-precommiton both protocol modules passes; pre-commit passes twice on every changed file with no further modification.python scripts/test_to_stage_mapping.py --tests "unittest/api_stability/visual_gen/test_visual_gen_serve_api.py"→ the CPU-Generic stages; the file containspytest.mark.cpu_only.scope: None, no narrowed stages (the changedopenai_protocol.pyandopenai_server.pyare unclaimed, so the full suite runs).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 run the self-review checklist on the PR
🤖 Generated with Claude Code