[JunyiXu-nv]

@coderabbitai summary

Refreshed onto main c0468112f4 after #21406 merged as 9ceb00a84e. This PR now contains only its own commit; the patch is identical to the previously reviewed version. It remains draft while the inherited main-side constructor-baseline failure is repaired by #22174.

Description

With TRTLLM_KVCACHE_TIME_OUTPUT_PATH set to a non-empty value, three request paths force per-request engine perf metrics (SamplingParams.return_perf_metrics=True) right after building the sampling parameters: the completions handler (openai_completion), the gpt-oss chat handler (chat_harmony) and the Responses preprocessing (responses_utils.request_preprocess). The chat completions handler (openai_chat), which every non-gpt-oss chat request takes, did not. This PR adds the same two statements to openai_chat, in the same position, so the four sites run the same check and assignment.

Why every chat request rather than a gpt-oss-only special case: the four request paths now agree, and moving gpt-oss chat onto the common chat handler later needs no model-specific branch for this variable. Enabling the variable for all chat completions was chosen deliberately over keeping a special case for one model.

A prerequisite of the trtllm-serve refactor (epic TRTLLM-17056), not one of its tasks. Uses the fixture factory and fake generators (tests/test_common/serve/) delivered on main by #21406.

Behavior

This PR changes behavior. With TRTLLM_KVCACHE_TIME_OUTPUT_PATH set, chat completion requests (including the Anthropic Messages routes and /health_generate, which use the chat handler) now request per-request engine perf metrics, as completions, gpt-oss chat and Responses already do. Response bodies, status codes and headers are unchanged.

Request path	Before, variable set	After, variable set
/v1/completions (openai_completion)	return_perf_metrics=True	unchanged
/v1/chat/completions for gpt-oss (chat_harmony)	return_perf_metrics=True	unchanged
/v1/responses (request_preprocess)	return_perf_metrics=True	unchanged
/v1/chat/completions for every other model (openai_chat, on the aggregated server and on the disaggregated context and generation workers, which serve chat through the same handler)	not requested	return_perf_metrics=True
Anthropic /v1/messages and /v1/messages/batches (run through openai_chat)	not requested	return_perf_metrics=True
/health_generate (runs through openai_chat)	not requested	return_perf_metrics=True
Any path, variable unset or empty	not requested	unchanged (not requested)
HTTP status codes, response bodies, response headers	—	unchanged

What the per-request flag does: the engine attaches RequestPerfMetrics to the request's results and LLM.generate_async stamps the wall-clock arrival timestamp into metrics_dict. Response bodies never carry these metrics; per-request headers and JSONL records are produced only when server-level collection (return_perf_metrics or perf_metrics_output_dir) is on, which already covered chat. One consumer is unlocked by the per-request flag alone: OpenTelemetry tracing. GenerationResultBase.do_tracing (tensorrt_llm/executor/result.py) returns with insufficient_request_metrics_warning() unless the result carries req_perf_metrics_dict, which is built only from the engine's request_perf_metrics; with otlp_traces_endpoint configured and server-level perf metrics off, chat requests now produce their llm_request span, as completions requests already do; before, tracing stopped at that warning and the request had no span. The KV-cache transfer time CSVs are written by the C++ side when the variable is set, independently of this flag. Server-level perf metrics, the other three sites and the variable's meaning do not change; gathering the environment reads of tensorrt_llm/serve/ into one module is left to a later change, so the new site repeats the two statements of the completions site, with a comment that explains them in place of that site's TODO.

Moves

None.

CI bookkeeping

• Multi-GPU table: none. tensorrt_llm/serve/openai_server.py is already listed, so the multi-GPU stages are added to this PR's pre-merge and stay blocked until the ci: full pre-merge approved label is applied.
• CODEOWNERS: none.
• legacy-files.txt: none (openai_server.py keeps its formatting group; the new test file uses the default ruff configuration).
• Test lists: none. The new file tests/unittest/serve/test_kvcache_time_output_perf_metrics.py is collected through the unittest/serve directory entry of tests/integration/test_lists/test-db/l0_cpu.yml (stages CPU-Generic-x86-1/2 and CPU-Generic-arm-1/2) and carries pytest.mark.cpu_only.

Related open PRs on the same lines

• [TRTLLM-11316][feat] Add kvbm disagg tests and support kvbm disagg slurm task #13400 (draft, last updated 2026-08-31) adds the same two statements to openai_chat as part of a kvbm disaggregated test change. If it merges first, this PR's production change is already on main and a rebase keeps only the tests.
• [None][feat] Add per-rank perf time events capture (TRTLLM_PERF_TIME_EVENTS_PATH) #16900 widens the condition at the completions, gpt-oss and Responses sites to TRTLLM_KVCACHE_TIME_OUTPUT_PATH or TRTLLM_PERF_TIME_EVENTS_PATH and does not touch openai_chat. This PR does not add the second variable (out of its scope). If [None][feat] Add per-rank perf time events capture (TRTLLM_PERF_TIME_EVENTS_PATH) #16900 merges first, the chat site is the one site without the widened condition until the later change that gathers these reads; if it merges second, it should extend the chat site too.
• [TRTLLM-17267][refactor] Move the completions handler into a serving class #21640 (draft) moves the completions handler out of openai_server.py; the completions parity test here goes through the HTTP route and does not depend on where the handler lives.

Test Coverage

New CPU tests in tests/unittest/serve/test_kvcache_time_output_perf_metrics.py (7 tests). Each builds the aggregated server through isolated_serve_env on the scripted fakes (server-level perf metrics off), sets the variable through the environment's configuration, sends one request, asserts that the handler reached the fake generate_async exactly once (so a harness that stops early fails for the right reason), and reads the submitted sampling_params:

• test_chat_requests_metrics_when_kvcache_time_output_is_set (/v1/chat/completions, non-streaming; the fake engine is scripted to fail at the call, so the handler stops right after submitting the request and the recorded flag is the value the engine was handed), test_streaming_chat_requests_metrics_when_kvcache_time_output_is_set (/v1/chat/completions with stream: true), test_anthropic_messages_request_metrics_when_kvcache_time_output_is_set (/v1/messages), test_anthropic_batch_requests_metrics_when_kvcache_time_output_is_set (/v1/messages/batches, waits for the batch to end), test_health_generate_requests_metrics_when_kvcache_time_output_is_set (/health_generate): return_perf_metrics is True.
• test_chat_does_not_request_metrics_when_unset: return_perf_metrics is False with the variable unset and server-level perf metrics off.
• test_completions_requests_metrics_when_kvcache_time_output_is_set (/v1/completions): the pre-existing behavior the chat path now matches.

Refresh verification (2026-11-12)

Pinned base c0468112f4, refreshed head c14f872715; git range-diff reports the own commit's patch unchanged. CPU runs used the worktree's code and the same local compiled-op stub on both sides where applicable.

• Focused new test file: 7 passed. Removing only the four production lines makes the five chat-path tests fail (2 parity/unset tests still pass); restoring them gives 7 passed again. The source bytes were restored exactly.
• pytest -m cpu_only tests/unittest/serve tests/unittest/api_stability/test_serve_routes.py -q: 142 passed, 1 failed. The only failure is test_no_new_object_new_sites, with the same six historical baseline discrepancies observed on pinned main (135 passed, 1 failed). [None][fix] Align the serve constructor ratchet with its landing baseline #22174 corrects that main-side baseline; this PR does not change it.
• pytest tests/unittest/api_stability -q: 99 passed.
• Pre-commit on both changed files, twice: clean; no code or test changes beyond the patch-identical refresh.
• The full per-entry serve CPU runner was not repeated for this refresh; its earlier run is recorded below. The supported invocation uses an absolute tests path: LLM_MODELS_ROOT=... PYTHONPATH=$PWD/tests python3 -m test_common.serve.cpu_tests.

Earlier validation on the original stacked version

The following results describe 36fbf3f3d9 on its original base, not the refreshed main-based tree:

• pytest -m cpu_only tests/unittest/serve/test_kvcache_time_output_perf_metrics.py -q -rs: 7 passed, no skips.
• Planted mutations at the chat site, each followed by a byte-verified restore: production change reverted: 5 failed (the five chat-path tests, each on its return_perf_metrics assertion and none on the "never reached generate_async" guard), 2 passed (unset, completions); condition always true (> 0 to >= 0): 1 failed (test_chat_does_not_request_metrics_when_unset), 6 passed; condition always false (> 0 to < 0): 5 failed, 2 passed; statements moved to after the generate_async call: 1 failed (test_chat_requests_metrics_when_kvcache_time_output_is_set), 6 passed; condition restricted to non-streaming requests: 1 failed (test_streaming_chat_requests_metrics_when_kvcache_time_output_is_set), 6 passed.
• git grep -n "TRTLLM_KVCACHE_TIME_OUTPUT_PATH" -- tensorrt_llm/serve/: four lines (chat, completions, gpt-oss chat, Responses).
• pytest -m cpu_only tests/unittest/api_stability tests/unittest/serve -q: 181 passed (route snapshot, middleware order, object.__new__ ratchet, layering, shim identity, the new file).
• pytest -m cpu_only tests/unittest/llmapi/apps/test_anthropic_routes.py tests/unittest/llmapi/apps/test_openai_chat_disagg_multimodal.py tests/unittest/llmapi/apps/test_openai_chat_multimodal_prompt.py -q -rf: 61 passed, the same as on the base of this PR (no failures on either).
• LLM_MODELS_ROOT=... PYTHONPATH=tests python3 -m test_common.serve.cpu_tests (every serve CPU test, one pytest process per test-db entry): 70 processes, 4636 passed, 0 failed, 55 deselected; the unittest/serve entry runs 108 tests including the seven new ones. Four files that fail in this environment on the base of this PR as well (tests/unittest/disaggregated/test_context_first_profile.py, test_lifecycle_activation.py, test_peer.py; tests/unittest/llmapi/test_config_database.py) are not selected by that runner; run with plain pytest -m cpu_only on this branch and on its base they fail with the same 110 test ids (44 + 66), none specific to this change.
• pre-commit run --files tensorrt_llm/serve/openai_server.py tests/unittest/serve/test_kvcache_time_output_perf_metrics.py, twice: all hooks pass, nothing modified.
• Registration: grep -n "unittest/serve" tests/integration/test_lists/test-db/l0_cpu.yml shows the directory entry (line 344); python scripts/test_to_stage_mapping.py --tests "unittest/serve" names CPU-Generic-arm-1, CPU-Generic-arm-2, CPU-Generic-x86-1 and CPU-Generic-x86-2; grep -c "pytest.mark.cpu_only" tests/unittest/serve/test_kvcache_time_output_perf_metrics.py is 1.
• Left to CI (GPU): tests/integration/defs/disaggregated/test_disaggregated.py::test_disaggregated_kv_cache_time_output and test_disaggregated_perf_metrics (A10 list), tests/integration/defs/test_e2e.py::test_openai_chat_harmony_perf_metrics (H100 list).

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-compatible or api-breaking. For api-breaking, include BREAKING in 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