You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[TRTLLM-17557][fix] Make the configuration-database CLI test runnable - #21441
The config collector excludes curated/eplb/ files because they contain expert-placement tables referenced by curated configs, not standalone serving configs. The serving CLI test sets --num_serve_frontends 1, and _MockOpenAIServer provides a FastAPI app for middleware setup. The lookup and generated database add entries for Nemotron Super NVFP4 and two Qwen3.5 V2 GB300 disaggregated scenarios. The PR reports no product-code behavior change.
The changed unit test is not registered in the integration CI or manual-QA test lists. Test-list registration remains a follow-up.
QA Engineer Review
tests/unittest/llmapi/test_config_database.py changed. The change updates config discovery and serving CLI setup. It does not add or remove test functions, IDs, selectors, or waivers. The tests cover config validation, lookup coverage, defaults, KV-cache policy, serving CLI behavior, and middleware. The unit test is not listed in the integration CI or manual-QA lists. Coverage verdict: needs follow-up for test-list registration.
The PR description reports 255 passes for the targeted CLI test, 965 passes and 62 pre-existing failures for the full test file, 11 passes across lookup and middleware checks, and 3,472 passes in broader suites. These results are author-reported.
Per-File QA Perspective
tests/unittest/llmapi/test_config_database.py: Covers curated config discovery and serving CLI behavior, including middleware setup. This unit test is not listed in tests/integration/test_lists/test-db/ or tests/integration/test_lists/qa/.
examples/configs/curated/lookup.yaml: Adds three lookup records. Verify model and architecture values, config paths, GPU compatibility, scenarios, and disagg flags.
docs/source/_static/config_db.json: Adds the generated Nemotron Super NVFP4 record. Verify that it matches its source config and lookup metadata.
Description
tests/unittest/llmapi/test_config_database.py validates every example config under examples/configs/ against TorchLlmArgs and drives the real launch_server through the trtllm-serve CLI with a mocked LLM and server — but CI never runs it (the file is in no
test list), and it no longer passes locally. This PR makes it runnable again, ahead of
registering it in the CPU test list (epic TRTLLM-17056, task TRTLLM-17557):
test_database_yaml_config_serve_cli failed for every config: --num_serve_frontends
now defaults to more than one, so the real launch_server entered the multi-frontend
launcher path, which reads the executor's IPC directory off the mocked LLM
(TypeError ... not Mock in os.path.join) and would otherwise spawn attached frontend
processes by re-executing sys.argv — the pytest command line. The test now pins --num_serve_frontends 1 in its CLI argv (the multi-frontend launcher has its own CPU
tests, e.g. test_serve_multi_frontend_helpers.py, test_serve_frontend_affinity.py). _MockOpenAIServer also gains a real FastAPI app, which launch_server hands to the --middleware applier.
examples/configs/curated/eplb/ is no longer collected as serving configs. Its two
YAML files are MoE expert-placement tables (initial_global_assignments, num_slots)
that qwen3.8-high-throughput{,-mtp3}.yaml reference through moe_config.load_balancer.
They are not TorchLlmArgs configs (pydantic rejects them with extra_forbidden) and
cannot start trtllm-serve on their own, so they failed the schema test and the CLI test
on every run, and giving them lookup.yaml entries would have published unusable trtllm-serve ... --config .../eplb/... commands into the docs config selector. The
exclusion sits next to the existing lookup.yaml exclusion in the test. The alternative
— moving the tables out of examples/configs/curated/ — is the config owners' call.
test_all_configs_have_lookup_entry failed: three curated configs had no examples/configs/curated/lookup.yaml entry. This PR adds them, following the existing
entry shape (model, arch, config_path, scenario, gpu_compatibility, disagg):
the Hopper Marlin NVFP4 Nemotron recipe (from [TRTLLM-12242][feat] Add Marlin NVFP4 backend for MoE and Linear on Hopper #13476; arch: NemotronHForCausalLM per
the checkpoint's config.json and that PR's accuracy tests) and the two Qwen3.5 GB300
agentic disaggregated configs (from [None][doc] Document Qwen3.5 GB300 agentic serving #20514; model nvidia/Qwen3.5-397B-A17B-NVFP4-V2
per the deployment guide). docs/source/_static/config_db.json is regenerated with python3 scripts/generate_config_table.py, as the other lookup changes do; the only
delta is the new Marlin card (disaggregated entries render no card).
Known and left alone (pre-existing, for the config owners): 62 of the file's 1027 items
still fail on current main for config-data vs test-policy reasons — 50 default-valued-leaf
violations, 11 configs that deliberately disable KV-cache block reuse, and one
harness-merge validation conflict on qwen3.5-gb300-agentic-prefill.yaml (its deprecated torch_compile_config.enable_piecewise_cuda_graph: true meets the harness-materialized prefill_cuda_graph_backend: disabled; the real CLI path accepts the config). Also left for
the owners: the pre-existing nemotron-3-super-throughput.yaml entry names arch: NemotronNASForCausalLM, which is not a model-registry key; correcting it switches
that config to NemotronH's model defaults and newly fails a default-leaf check, so it is not
changed here (one concern per PR).
Behavior
No behavior change (test-only code, lookup data and the regenerated docs snapshot; no
product code).
CI bookkeeping
Multi-GPU table: none
CODEOWNERS: none
legacy-files.txt: none
Test lists: none (the registration follows in its own PR)
Test Coverage
All on a CPU-only container, canonical dev setup, pytest -p no:warnings:
pytest "tests/unittest/llmapi/test_config_database.py::test_database_yaml_config_serve_cli" -q:
255 passed in 12.07s (57s wall). Before this PR: 257 collected, 257 failed.
pytest -m cpu_only tests/unittest/llmapi/test_config_database.py -q (as the CPU stage
runs the file): 1027 items, 965 passed, 62 failed in 25.49s, none skipped — the 62 are the
pre-existing data/policy failures above; before this PR the file had 322 failures, and
the failing set after it is a strict subset of the previous one (zero new failures).
The CLI and wiring tests also pass with fastapi==0.136.3 (the low end of the pin)
overlaid; the default run used 0.139.2.
pytest -m cpu_only tests/unittest/llmapi/apps tests/unittest/disaggregated -q:
3472 passed; the remaining failures/errors reproduce without this PR (model-root setup,
etcd key cross-talk from concurrent runs on the shared box, the trtllm-render console
script not installed in the shared venv) and none of those files reads anything this PR
touches.
GPU checks: none needed (the file is cpu_only; CI stage is CPU-Generic).
Open PR #21394 ("Stop trtllm-serve launchers from mutating caller-owned config dicts",
head ffd5c84) edits two other assertions in this file (positional-to-keyword call args
in test_serve_cli_passes_middlewares_to_launch_server and test_serve_cli_enable_visual_gen_routes_to_visual_gen), in hunks this PR does not
touch; whichever merges second rebases cleanly.
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.
If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.
Any new dependencies have been scanned for license and vulnerabilities
test_database_yaml_config_serve_cli runs the real launch_server with a
mocked LLM and server, but the fakes predate two launch_server
behaviors: the serving-frontend count now defaults to more than one, so
launch_server tried to spawn attached frontend processes by re-executing
the test's own command line and crashed reading the mocked executor's
IPC directory, and _apply_fastapi_middlewares reads the server's .app,
which the mock server lacked. Pin --num_serve_frontends 1 in the test's
CLI argv and give the mock server a real FastAPI app.
Stop collecting examples/configs/curated/eplb/ as serving configs: those
files are MoE expert-placement tables that the Qwen3.8 max-throughput
configs reference through moe_config.load_balancer, so they can neither
validate as TorchLlmArgs nor start trtllm-serve on their own (lookup.yaml
is excluded from collection for the same reason).
Add the three examples/configs/curated/lookup.yaml entries that
test_all_configs_have_lookup_entry still requires: the Hopper Marlin
NVFP4 Nemotron recipe and the two Qwen3.5 GB300 agentic disaggregated
configs, and regenerate docs/source/_static/config_db.json from the
lookup files as the other lookup changes do.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Reviewing files that changed from the base of the PR and between 78572c1 and 9f3d439.
Files selected for processing: 1
tests/unittest/llmapi/test_config_database.py
Files skipped from review as they are similar to previous changes: 1
tests/unittest/llmapi/test_config_database.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
Walkthrough
The curated configuration data now includes Nemotron Super NVFP4 and Qwen3.5 V2 GB300 entries. Configuration database tests update curated-file discovery and serve test setup.
Changes
Curated configuration database
Layer / File(s)
Summary
Add curated model entries examples/configs/curated/lookup.yaml, docs/source/_static/config_db.json
The curated lookup and database add a Nemotron Super NVFP4 entry for H100 and H200. The lookup adds Qwen3.5 V2 GB300 entries for agentic disaggregated prefill and decode.
Update configuration database test setup tests/unittest/llmapi/test_config_database.py
Curated config discovery excludes files under curated/eplb/. Serve CLI test arguments set --num_serve_frontends to 1, and the mock OpenAI server initializes a FastAPI app.
The configuration additions match the documented selection behavior. No issue requiring a fix before merge was identified.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title "[TRTLLM-17557][fix] Make the configuration-database CLI test runnable" follows the repository template with a JIRA ticket and the [fix] type. It matches the main change: the CLI test is mad…
Description check
The description follows the repository template in substance. It explains the problem and the fix in detail: the CLI test now pins --num_serve_frontends 1, the FastAPI app is added to the mock server,…
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title "[TRTLLM-17557][fix] Make the configuration-database CLI test runnable" follows the repository template with a JIRA ticket and the [fix] type. It matches the main change: the CLI test is made runnable again. The changeset also adds lookup entries, excludes eplb configs, and regenerates the docs snapshot. The title does not cover those, which is expected for a short title. The title is specific and clear.
Full details: Description check
Explanation
The description follows the repository template in substance. It explains the problem and the fix in detail: the CLI test now pins --num_serve_frontends 1, the FastAPI app is added to the mock server, eplb configs are excluded, and three lookup entries are added. The Test Coverage section lists specific test commands and results. The PR Checklist is ticked. The description does not use the template's exact Description and Test Coverage comment placeholders, but it covers what the template asks for. The description is long and includes some detail that a reviewer must read closely, such as the 62 pre-existing failures and the open PR #21394 reference. This does not make the description incomplete.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
220-221: Minor · Functional Correctness · Quick win
Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.
Preserve coverage of configured frontend counts.
This flag overrides num_serve_frontends in every YAML file. For example, a config with num_serve_frontends: 2 and orchestrator_type: rpc can pass this test even though the normal serve path rejects it. Add a CLI guard check without the override, while mocking child-process creation. (raw.githubusercontent.com)
As per path instructions: “Flag assertions that would pass despite a plausible regression.”
Reviewing files that changed from the base of the PR and between 8d4901e and 78572c1.
Files selected for processing: 3
docs/source/_static/config_db.json
examples/configs/curated/lookup.yaml
tests/unittest/llmapi/test_config_database.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Approving — the comments below are optional touch-ups, not blockers.
The fixes match the failures. The lookup additions only cover configs that already existed without entries. config_db.json gaining just the Nemotron row is expected, because the generator skips disagg: true entries.
One coverage tradeoff from the frontend pin is noted inline. The test-list registration follow-up is already called out in the description.
…nd only
Passing --num_serve_frontends skips the default frontend-count resolution
a plain trtllm-serve --config run goes through.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
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
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
brnguyen2
left a comment
Dev Engineer Review
The config collector excludes
curated/eplb/files because they contain expert-placement tables referenced by curated configs, not standalone serving configs. The serving CLI test sets--num_serve_frontends 1, and_MockOpenAIServerprovides a FastAPI app for middleware setup. The lookup and generated database add entries for Nemotron Super NVFP4 and two Qwen3.5 V2 GB300 disaggregated scenarios. The PR reports no product-code behavior change.The changed unit test is not registered in the integration CI or manual-QA test lists. Test-list registration remains a follow-up.
QA Engineer Review
tests/unittest/llmapi/test_config_database.pychanged. The change updates config discovery and serving CLI setup. It does not add or remove test functions, IDs, selectors, or waivers. The tests cover config validation, lookup coverage, defaults, KV-cache policy, serving CLI behavior, and middleware. The unit test is not listed in the integration CI or manual-QA lists. Coverage verdict: needs follow-up for test-list registration.The PR description reports 255 passes for the targeted CLI test, 965 passes and 62 pre-existing failures for the full test file, 11 passes across lookup and middleware checks, and 3,472 passes in broader suites. These results are author-reported.
Per-File QA Perspective
tests/unittest/llmapi/test_config_database.py: Covers curated config discovery and serving CLI behavior, including middleware setup. This unit test is not listed intests/integration/test_lists/test-db/ortests/integration/test_lists/qa/.examples/configs/curated/lookup.yaml: Adds three lookup records. Verify model and architecture values, config paths, GPU compatibility, scenarios, anddisaggflags.docs/source/_static/config_db.json: Adds the generated Nemotron Super NVFP4 record. Verify that it matches its source config and lookup metadata.Description
tests/unittest/llmapi/test_config_database.pyvalidates every example config underexamples/configs/againstTorchLlmArgsand drives the reallaunch_serverthrough thetrtllm-serveCLI with a mocked LLM and server — but CI never runs it (the file is in notest list), and it no longer passes locally. This PR makes it runnable again, ahead of
registering it in the CPU test list (epic TRTLLM-17056, task TRTLLM-17557):
test_database_yaml_config_serve_clifailed for every config:--num_serve_frontendsnow defaults to more than one, so the real
launch_serverentered the multi-frontendlauncher path, which reads the executor's IPC directory off the mocked LLM
(
TypeError ... not Mockinos.path.join) and would otherwise spawn attached frontendprocesses by re-executing
sys.argv— the pytest command line. The test now pins--num_serve_frontends 1in its CLI argv (the multi-frontend launcher has its own CPUtests, e.g.
test_serve_multi_frontend_helpers.py,test_serve_frontend_affinity.py)._MockOpenAIServeralso gains a real FastAPIapp, whichlaunch_serverhands to the--middlewareapplier.examples/configs/curated/eplb/is no longer collected as serving configs. Its twoYAML files are MoE expert-placement tables (
initial_global_assignments,num_slots)that
qwen3.8-high-throughput{,-mtp3}.yamlreference throughmoe_config.load_balancer.They are not
TorchLlmArgsconfigs (pydantic rejects them withextra_forbidden) andcannot start
trtllm-serveon their own, so they failed the schema test and the CLI teston every run, and giving them
lookup.yamlentries would have published unusabletrtllm-serve ... --config .../eplb/...commands into the docs config selector. Theexclusion sits next to the existing
lookup.yamlexclusion in the test. The alternative— moving the tables out of
examples/configs/curated/— is the config owners' call.test_all_configs_have_lookup_entryfailed: three curated configs had noexamples/configs/curated/lookup.yamlentry. This PR adds them, following the existingentry shape (
model,arch,config_path,scenario,gpu_compatibility,disagg):the Hopper Marlin NVFP4 Nemotron recipe (from [TRTLLM-12242][feat] Add Marlin NVFP4 backend for MoE and Linear on Hopper #13476;
arch: NemotronHForCausalLMperthe checkpoint's
config.jsonand that PR's accuracy tests) and the two Qwen3.5 GB300agentic disaggregated configs (from [None][doc] Document Qwen3.5 GB300 agentic serving #20514; model
nvidia/Qwen3.5-397B-A17B-NVFP4-V2per the deployment guide).
docs/source/_static/config_db.jsonis regenerated withpython3 scripts/generate_config_table.py, as the other lookup changes do; the onlydelta is the new Marlin card (disaggregated entries render no card).
Known and left alone (pre-existing, for the config owners): 62 of the file's 1027 items
still fail on current
mainfor config-data vs test-policy reasons — 50 default-valued-leafviolations, 11 configs that deliberately disable KV-cache block reuse, and one
harness-merge validation conflict on
qwen3.5-gb300-agentic-prefill.yaml(its deprecatedtorch_compile_config.enable_piecewise_cuda_graph: truemeets the harness-materializedprefill_cuda_graph_backend: disabled; the real CLI path accepts the config). Also left forthe owners: the pre-existing
nemotron-3-super-throughput.yamlentry namesarch: NemotronNASForCausalLM, which is not a model-registry key; correcting it switchesthat config to NemotronH's model defaults and newly fails a default-leaf check, so it is not
changed here (one concern per PR).
Behavior
No behavior change (test-only code, lookup data and the regenerated docs snapshot; no
product code).
CI bookkeeping
Test Coverage
All on a CPU-only container, canonical dev setup,
pytest -p no:warnings:pytest "tests/unittest/llmapi/test_config_database.py::test_database_yaml_config_serve_cli" -q:255 passed in 12.07s (57s wall). Before this PR: 257 collected, 257 failed.
pytest -m cpu_only tests/unittest/llmapi/test_config_database.py -q(as the CPU stageruns the file): 1027 items, 965 passed, 62 failed in 25.49s, none skipped — the 62 are the
pre-existing data/policy failures above; before this PR the file had 322 failures, and
the failing set after it is a strict subset of the previous one (zero new failures).
test_all_configs_have_lookup_entry,test_all_lookup_entries_have_arch,test_database_yaml_config_count,test_apply_fastapi_middlewares_supports_class_and_function,test_serve_cli_passes_middlewares_to_launch_server,test_serve_cli_enable_visual_gen_routes_to_visual_gen,test_serve_cli_rejects_middleware_with_grpc, plustests/unittest/tools/test_config_database_sync.py: 11 passed.fastapi==0.136.3(the low end of the pin)overlaid; the default run used 0.139.2.
pytest -m cpu_only tests/unittest/llmapi/apps tests/unittest/disaggregated -q:3472 passed; the remaining failures/errors reproduce without this PR (model-root setup,
etcd key cross-talk from concurrent runs on the shared box, the
trtllm-renderconsolescript not installed in the shared venv) and none of those files reads anything this PR
touches.
cpu_only; CI stage is CPU-Generic).Open PR #21394 ("Stop trtllm-serve launchers from mutating caller-owned config dicts",
head ffd5c84) edits two other assertions in this file (positional-to-keyword call args
in
test_serve_cli_passes_middlewares_to_launch_serverandtest_serve_cli_enable_visual_gen_routes_to_visual_gen), in hunks this PR does nottouch; whichever merges second rebases cleanly.
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