Repository navigation
[TRTLLM-17264][refactor] Claim VisualGen in CBTS and add the serve import-isolation test - #21567
Draft
JunyiXu-nv wants to merge 28 commits into
Draft
JunyiXu-nv wants to merge 28 commits into
JunyiXu-nv wants to merge 28 commits into
Conversation
…lope Pin the exact 422 body at the route-level entry point (FastAPI RequestValidationError on /v1/images/generations, which declares its request model) and at the handler-level entry points (pydantic ValidationError raised while building the request model on /v1/videos/sync, /v1/videos and /v1/images/edits, JSON and multipart). All entry points must render byte-identical bodies; these tests pin the full body so a later refactor of the rendering code has exact coverage. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…derers OpenAIServer._create_visual_gen_validation_error_response (for FastAPI's RequestValidationError) and _VideoRoutesMixin._render_pydantic_validation_error (for pydantic's ValidationError) were statement-for-statement identical. Replace both with one module-level function, render_visual_gen_validation_error in tensorrt_llm/serve/_visual_gen_errors.py, that accepts either exception and the server's error-response factory, and point all six call sites at it. The factory argument keeps the error envelope and the context-length code rule defined in one place and avoids an import cycle with openai_server. This also removes the only call OpenAIServer makes to a method defined by the video-routes mixin (openai_image_edit called the mixin's renderer). No behavior change: both entry points return the same status and bytes as before; the characterization tests added in the previous commit pass unchanged on both sides of this refactor. The new module joins the mypy file list. 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>
…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>
…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>
… serve/api/visual_gen
Generated move of the VisualGen serving code out of OpenAIServer and the
video-routes mixin, with the shared error-response factory moved to
serve/_base/serving.py:
- serve/openai_video_routes.py -> serve/api/visual_gen/serving_videos.py
(the module helpers and the nine mixin methods, now on VideoServing)
- the image handlers and helpers of OpenAIServer
-> serve/api/visual_gen/serving_images.py (ImageServing)
- _reject_disabled_path -> serve/api/visual_gen/serving_base.py
(VisualGenBaseServing)
- _new_media_dir and _resolve_media_dir -> serve/api/visual_gen/media.py
- create_error_response -> serve/_base/serving.py (BaseServing);
OpenAIServer keeps staticmethod(BaseServing.create_error_response)
- serve/visual_gen_utils.py, serve/visual_gen_metrics.py and
serve/_visual_gen_errors.py move whole to
serve/api/visual_gen/{utils,metrics,errors}.py
The four old modules are deleted without a compatibility shim, and every
import in the repository points at the new modules. The new files that
carry yapf-formatted code are registered in legacy-files.txt and the lint
configs are regenerated from it; the mypy file list follows the renamed
errors module. The errors module docstring gains the blank line between
summary and description that the docstring rules require once the module
leaves its underscore name. The class headers, constructors, router and
composition-root wiring follow in the next commits.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…r into the VISUAL_GEN role The generated move left the handlers in bare classes; this commit adds what a move cannot write: - BaseServing (serve/_base/serving.py) gets its docstring; it holds the shared create_error_response and nothing else. - VisualGenBaseServing (serve/api/visual_gen/serving_base.py) derives from BaseServing and receives generator, model, media_storage_path and the server AdjustedSteadyClock as keyword-only constructor arguments, so missing wiring fails at construction; its _generate() is the one place that submits a generation off the event loop. ImageServing and VideoServing derive from it; VideoServing owns video_gen_tasks as instance state. - The image generation, synchronous video and asynchronous video handlers submit through self._generate(). The image edit keeps its synchronous generate() call: routing it through _generate() would stop it from blocking the event loop, a behavior change that belongs to its own fix (NVIDIA#21403). - _supports_image_edit drops its _is_visual_gen guard: ImageServing only exists for a VisualGen generator, so the guard was always false. - serve/api/visual_gen/api_router.py: one router made with make_router() for the ten image and video routes, bound late to the serving objects on app.state; attach_router refuses an app without them; init_serving_state resolves the media directory once and injects it into both serving objects. The router joins ROUTERS for the VISUAL_GEN role group and the mypy file list. - OpenAIServer no longer derives from the video-routes mixin. For the VISUAL_GEN role it loads the router module once with importlib, calls init_serving_state and registers only /metrics itself; the validation handler reaches the VisualGen renderer through that module. Routers are attached after the role switch, because every attach_router checks that its serving objects are on app.state; /health, /version and /v1/models therefore register after the role-specific routes, which conflicts with no route of any role. - The whole-file-moved utils module gets the repository SPDX header; the bench docstring that named the deleted video-routes module names the new modules. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…dia directory - The test sites that patched or imported names through their old modules now reach the modules the moved code reads: the media clock (api/visual_gen/media.datetime), the image encoder (serving_images.image_to_bytes), the image-edit limits and base64 on api/visual_gen/utils, and resolve_video_format on serving_videos; the module-object imports of the deleted modules follow their new paths. - New tests: images and videos share the one media directory the composition root resolves; the serving classes build without a server and without touching the filesystem; the router answers image and video requests on a bare app with every route on the msgpack route class; attach_router refuses an app whose serving objects are missing and names what to call; image generation, synchronous video and asynchronous video submit through VisualGenBaseServing._generate while the image edit still calls the synchronous generate(); _generate runs the submission off the event loop; OpenAIServer hosts none of the eighteen moved handlers and no video-task table. - test_context_length_error.py: OpenAIServer and the serving classes render errors through the one BaseServing function. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…NTS.md The "Where code goes" list points at tensorrt_llm/serve/api/visual_gen/ (ImageServing and VideoServing on VisualGenBaseServing, the router in api_router.py, one RouterSpec) and records that only the composition root, for the VISUAL_GEN role, loads the package. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ord the errors docstring The generated move dropped the type-only comment inside the video module import block and doubled the blank line after _KNOWN_VIDEO_OUTPUT_SUFFIXES; both are restored so the module reads as it did before the move. The errors module docstring no longer calls the module private: it is a public-named module internal to the VisualGen serving package. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…er a visual_gen path The CI test selector keeps only test entries whose text contains visual_gen/ for VisualGen-only pull requests, so a VisualGen serving test outside such a path silently stops running for them. tests/unittest/llmapi/ test_visual_gen_startup_shutdown.py starts VisualGen serving through launch_visual_gen_server; move it, unchanged, to tests/unittest/_torch/visual_gen/, and follow in the CPU test list and the duration table. Its owner changes from the runtime team to the VisualGen team with the path. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ing package The CI test selector will run only VisualGen tests for a pull request that changes only tensorrt_llm/serve/api/visual_gen/. That is safe only while no other serving role, CLI entry point or test outside a visual_gen/ path depends on the package. Add tests/unittest/llmapi/apps/visual_gen/ test_serve_visual_gen_isolation.py, which fails as soon as that stops being true: - the static import graph of tensorrt_llm/: the only references to the package from outside it are the composition root's role-gated load, the routing table's module path, the lazy re-export in tensorrt_llm.serve.openai_protocol and the VisualGen benchmark client, each listed with its reason; a listed edge that disappears fails too; - at run time, in fresh interpreters, no non-VisualGen serving role (the default LLM role, CONTEXT, GENERATION, MM_ENCODER, EMBEDDING, the disaggregated frontend with a local coordinator and in fleet mode, the fleet coordinator) and no trtllm-serve subcommand or trtllm-render --help loads the package or the media helpers it uses, while the VISUAL_GEN role does (the control); - every test module that exercises VisualGen serving lives under a path that contains visual_gen/, with two allowlisted exceptions that only pass ServerRole.VISUAL_GEN as a value; - test-db entries that name a visual_gen directory keep the trailing slash the selector matches on. The file lives under a visual_gen/ path itself, is registered in l0_cpu.yml, and tensorrt_llm/serve/AGENTS.md names it as the check for the VisualGen serving rule. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ve/api/visual_gen Add tensorrt_llm/serve/api/visual_gen/ to the VisualGen rule's source prefixes, so a pull request that changes only VisualGen serving files is selected as visualgenonly and runs only VisualGen test entries; a pull request that also touches an unclaimed file (for example tensorrt_llm/serve/openai_server.py) still falls back to the full suite. The package stays out of the outward prefixes: nothing outside it imports it except the composition root's role-gated load, the routing table, the lazy re-export in tensorrt_llm.serve.openai_protocol and the VisualGen benchmark client, which tests/unittest/llmapi/apps/visual_gen/test_serve_visual_gen_isolation.py asserts. Describe the package in the rule's docstring and both CBTS READMEs, fix the rule count there (eleven, with the formatting rule), and add tests for the new prefix: it is claimed, it is not outward, two changed files under it narrow to the visual_gen/ entries, and a mixed change falls back with the unclaimed file named. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…ace rows only check_warnings skips rows with warns=False and find_old_path_imports checks only rows with ban_old_path=True. VISUAL_GEN_SHIM_ROWS holds interface rows only (no warning, no ban, "not retired"), so the two tests that ran those checks on it could not fail. Replace them with an assertion that every row is an interface row. If a kept shim with a VisualGen path is ever added, the assertion fails and says to run the warning and old-path checks on this table. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
JunyiXu-nv
force-pushed
the
dev-junyix-feat-serve-cbts-import-isolation
branch
from
November 4, 2026 12:51
d60eca1 to
2fd70e2
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.
Stacked on #21552 (Move the VisualGen handlers into serve/api/visual_gen): the first commits below belong to that PR and disappear from this diff when it merges; this PR's own change is the last 4 commits.
@coderabbitai summary
Description
Lets the CI test selector (CBTS,
jenkins/scripts/cbts/) claimtensorrt_llm/serve/api/visual_gen/: a pull request that changes only VisualGen serving files is selected asvisualgenonlyand runs only VisualGen test entries, while a pull request that also touches an unclaimed file (for exampletensorrt_llm/serve/openai_server.py) still falls back to the full suite. The package stays out of the outward prefixes because it is internal: the only references to it from outside are the trtllm-serve composition root's role-gated load (VISUAL_GEN only), the routing table's module-path string, the lazy re-export of the VisualGen models intensorrt_llm.serve.openai_protocol, and the VisualGen benchmark client. The same PR adds the test that keeps that claim honest,tests/unittest/llmapi/apps/visual_gen/test_serve_visual_gen_isolation.py, moves the one VisualGen serving test that lived outside avisual_gen/path, and brings the CBTS documentation up to date (the eleven registered rules, the new prefix, why the package is internal).Part of the trtllm-serve refactor: VisualGen isolation (epic TRTLLM-17056, task TRTLLM-17264). Builds on #21552 (Move the VisualGen handlers into serve/api/visual_gen), #21509 (Move the VisualGen protocol models into serve/api/visual_gen) and #21448 (Add the trtllm-serve router skeleton and move /health, /version and /v1/models into ops.system). Merges right after #21552.
Behavior
No runtime behavior change. CI selection: after merge, a pull request whose changed files are all under VisualGen-claimed prefixes, now including
tensorrt_llm/serve/api/visual_gen/, runs only VisualGen test entries. Measured with the selector's dry run on this head: a one-line change in a VisualGen handler body, or a field-description change in the VisualGen protocol module, selectsvisualgenonlywith 38 affected stages (11 of them multi-GPU stages, each narrowed to its VisualGen entries),sanity_required: False,perfsanity_required: True(the VisualGen perf-sanity list has a matching block), 305 narrowed entries per probe, all of them VisualGen entries; the narrowedl0_cpu.ymlkeeps the VisualGen api-stability test, the isolation test, the moved startup/shutdown test and theunittest/serve/api/visual_gen/entry. A change that also touchestensorrt_llm/serve/openai_server.pyfalls back to the full suite (unhandled_files). Because a multi-GPU stage is selected, such a pull request now needs theci: full pre-merge approvedlabel; before this change, video-handler, utils, metrics and protocol changes needed no label, and image-handler changes needed it for all multi-GPU stages. These numbers drift with test-db growth.The isolation test
tests/unittest/llmapi/apps/visual_gen/test_serve_visual_gen_isolation.py(CPU, registered inl0_cpu.yml, under avisual_gen/path so VisualGen-only pull requests run it; owned by the runtime team) fails as soon as the claim stops being safe:tensorrt_llm/: every reference to the package from outside it (import statements,if TYPE_CHECKING:included;importlib.import_module/__import__with a literal name; string literals naming a module of the package; imports of one of the eight VisualGen models fromtensorrt_llm.serve.openai_protocol, whose re-export loads the package) must be one of the four allowed edges, each listed with its reason; an allowed edge that disappears fails too. Docstrings are not references.GET /health(sent unconditionally, so a request-time import is caught on every FastAPI version of the pinned range) and none may have loaded the package or the media helpers it uses.trtllm-serve --help, every subcommand's--help, andtrtllm-render --helpload none of them.tensorrt_llm.serve.api.visual_gen.api_routerandtensorrt_llm.media.encoding, so the probe is known to see them.ServerRole.VISUAL_GENorlaunch_visual_gen_serveras a name or attribute, or imports a VisualGen model fromtensorrt_llm.serve.openai_protocol) lives under a path containingvisual_gen/; helper modules are not scanned; two allowlisted exceptions only passServerRole.VISUAL_GENas a value:tests/unittest/disaggregated/test_disagg_internal_auth.py, andtests/unittest/serve/_base/test_routing.py, which comes from [TRTLLM-17261][refactor] Add the trtllm-serve router skeleton and move /health, /version and /v1/models into ops.system #21448 in this stack and is not onmainyet.visual_gendirectory keep the trailing slash the selector matches on (unittest/serve/api/visual_gen/already has it; nothing needed adding).Every failure message says what to change and who reviews it.
Moves
No production code moves.
tests/unittest/llmapi/test_visual_gen_startup_shutdown.pymoves unchanged (git mv) totests/unittest/_torch/visual_gen/test_visual_gen_startup_shutdown.py; its owner changes from the runtime team to the VisualGen team with the path.tests/integration/defs/.test_durationsfollows (both keys of the file).CI bookkeeping
tensorrt_llm/serve/api/visual_gen/is deliberately not in it).l0_cpu.yml:unittest/llmapi/apps/visual_gen/test_serve_visual_gen_isolation.pyadded;unittest/llmapi/test_visual_gen_startup_shutdown.pyreplaced byunittest/_torch/visual_gen/test_visual_gen_startup_shutdown.py;tests/unittest/scripts/cbts/test_visual_gen_rule.pyis collected by the existingunittest/scriptsentry._VG_SRC_PREFIXESgainstensorrt_llm/serve/api/visual_gen/;_VG_OUTWARD_PREFIXESand_VG_ENTRY_PATTERNSare unchanged; the rule's docstring and both READMEs describe the package and the eleven registered rules.jenkins/scripts/cbts/README.mdorjenkins/scripts/cbts/cbts/rules/README.mdand will need a small conflict resolution whichever lands second: [None][infra] Retire legacy L0 perf testing and protect QA benchmarks #21524, [None][chore] GB300 development workflow smoke test #21451, [None][infra] select covered Python files across repository directories #21418, [None][chore] Relax Modeling V2 naming and select Modeling V2 tests by change scope #21376, [None][infra] unify GB300 pre-merge and post-merge test lists #21327, [None][infra] Introduce Pyrefly to do type check #21325, [None][infra] Narrow CBTS perf selection by changed model aliases #21323, [None][feat] Muse Spark Open modeling_v2 TEP target: low-latency optimizations for GB300 and Vera Rubin #20625.Test Coverage
All checks ran on CPU with this branch's code; the VisualGen GPU entries the selector picks are left to CI.
pytest -m cpu_only tests/unittest/llmapi/apps/visual_gen/test_serve_visual_gen_isolation.py -rs: 6 passed, no skips, on fastapi 0.139.2 and on fastapi 0.136.3 (both ends of the pinned range); the per-role/healthprobe answered for all 8 apps on both.tensorrt_llm/serve/chat_utils.pyfails checks 1 and 2 naming the file and the roles; a request-time import insideSystemServing.health()fails check 2 naming the first role that served/health;from tensorrt_llm.serve.openai_protocol import ImageObjectinchat_utils.py(a file with no other VisualGen reference) fails check 1, while importingChatCompletionRequestfrom the same module does not; a VisualGen test copied totests/unittest/llmapi/fails check 5; removing the composition-root allowed edge fails check 1; dropping the trailing slash ofunittest/serve/api/visual_gen/fails check 6.pytest -m cpu_only tests/unittest/scripts/cbts/test_visual_gen_rule.py tests/unittest/scripts/cbts/test_docs.py tests/unittest/scripts/cbts/test_p0_selection.py: 267 passed.pytest -m cpu_only tests/unittest/_torch/visual_gen/test_visual_gen_startup_shutdown.py: 3 passed (as at the old path).pytest -m cpu_only tests/unittest/serve/test_agents_md.py tests/unittest/serve/test_layering.py: 15 passed;pytest tests/unittest/api_stability: 107 passed.python scripts/test_to_stage_mapping.py --tests ...for the isolation test and the moved test: the fourCPU-Generic-*stages; the old path maps to nothing.python scripts/check_test_list.py --l0: no error.PYTHONPATH=tests python3 -m test_common.serve.cpu_tests, 72 entries): 4234 passed; 6 failures in two OpenEngine files this PR does not touch (a 90 s gRPC-ready timeout under heavy host load, andTemporaryDirectorycleanup failing under an NFS basetemp); both files pass alone on a local-disk basetemp (7 and 14 passed).cbts/command/dryrun.pyon a scratch clone of this head): the numbers under "Behavior"; the PR's own range is selected astestsonly.pre-commit run --files <every changed file>twice: clean.tests/unittest/serve/api/visual_gen/test_shim_identity_visual_gen.py(created by [TRTLLM-17261][test] Add route, middleware-order and shim-identity snapshots and a CPU test seam for trtllm-serve #21406 and filled by [TRTLLM-17264][refactor] Move the VisualGen protocol models into serve/api/visual_gen #21509, both below in this stack): the FutureWarning and old-path tests skipped every row of this interface-only table and could not fail, so they are replaced by an assertion that every row is an interface row (warns=False,ban_old_path=False,retire_in="not retired"). It names the offending row and says to runcheck_warningsandfind_old_path_importson the table if a kept shim ever gets a VisualGen path; each of the three conditions, broken alone, fails it. The identity test is unchanged: 2 passed. The serve CPU seam count above predates this change.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)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.🤖 Generated with Claude Code