[JunyiXu-nv]

Dev Engineer Review

OpenAIServer selects and caches a post-processing strategy from num_postprocess_workers. Response handlers use the strategy for post-processing, and CompletionOutput.postprocess_result provides public access to the worker result. The API-stability reference includes the new property.

For n > 1, worker mode still stores the result on outputs[0] and merges metrics, while inline mode records metrics per candidate. This behavior remains unchanged. No current review findings were supplied, so severity counts are unavailable.

QA Engineer Review

The changes add tests/unittest/serve/test_postproc_strategy.py to tests/integration/test_lists/test-db/l0_cpu.yml. CPU tests cover strategy selection, synchronous and asynchronous application, server-level caching, and the accessor. The modified Anthropic route and executor tests are also listed in l0_cpu.yml. The modified disaggregated response test and async-generation script were not found in the test lists searched.

Reported results include passing strategy, executor, API-stability, and disaggregated tests. The author reports one LLM API apps failure attributed to a driver-detection test that also fails on the base commit in their environment. GPU coverage awaits CI. Coverage verdict: needs follow-up pending CI confirmation and resolution or confirmation of the reported failure.

Per-File QA Perspective

• tensorrt_llm/executor/result.py — Adds the public postprocess_result accessor. Verify that it returns the worker-produced value and returns None when no worker result exists.
• tensorrt_llm/serve/openai_server.py — Changes response handlers to use the cached strategy. Verify worker and inline paths across synchronous, asynchronous, streaming, and Responses API handling.
• tensorrt_llm/serve/postprocess_handlers.py — Adds worker and inline strategy implementations. Verify that worker mode reads the stored result and inline mode invokes or awaits the postprocessor.
• tests/integration/test_lists/test-db/l0_cpu.yml — Adds unittest/serve/test_postproc_strategy.py. The list also contains the modified Anthropic route and executor tests.
• tests/unittest/api_stability/references/completion_output.yaml — Adds postprocess_result to the API-stability reference.
• tests/unittest/disaggregated/test_disagg_responses_api.py — Updates result access in disaggregated response coverage. This test was not found in the test lists searched.
• tests/unittest/llmapi/apps/test_anthropic_routes.py — Updates streamed-frame result access. The test is listed in l0_cpu.yml.
• tests/unittest/llmapi/run_llm_with_postproc.py — Updates result access in async-generation coverage. This file was not found in the test lists searched.
• tests/unittest/llmapi/test_executor.py — Checks that the public accessor exposes the stored result. The test is listed in l0_cpu.yml.
• tests/unittest/serve/test_postproc_strategy.py — Adds CPU tests for strategy selection, application behavior, caching, and the accessor. The test is listed in l0_cpu.yml.
• tests/unittest/grpc/openengine/test_metrics.py — Sets num_postprocess_workers to zero in the metrics test arguments, making the inline mode explicit.

Description

OpenAIServer decided nine times, in four handlers, whether a response's post-processed value comes from a post-processing worker (read the value the worker stored on outputs[0]) or must be computed in the server process (call the post-processor inline). The flag that drives the decision, postproc_worker_enabled, never changes after construction. This PR chooses the path once per server: select_postproc_strategy (new, in tensorrt_llm/serve/postprocess_handlers.py) picks WorkerPostprocStrategy or InlinePostprocStrategy from num_postprocess_workers; __init__ pre-caches the choice and a functools.cached_property (the same pattern as _is_visual_gen) keeps the many unit-test servers built with object.__new__ working — a plain attribute set only in __init__ would break every one of them. CompletionOutput gains the read-only public accessor postprocess_result, so the serving code stops reading the executor's private _postprocess_result field; the field and its writer (the frontend's handling of a postproc worker's output) are unchanged.

Part of the trtllm-serve refactor (epic TRTLLM-17056, task TRTLLM-17273): cleanups that land before the handler moves, so that moved handlers carry one strategy call per site instead of nine copies of a private-field read.

The nine decision points

All in tensorrt_llm/serve/openai_server.py; each if/ternary on self.postproc_worker_enabled becomes one call.

Handler	Site	Old	New
_create_chat_response	non-streaming chat	if/else on promise.outputs[0]._postprocess_result vs post_processor(promise, args)	self._postproc.apply(promise, postproc_params)
openai_chat → chat_stream_generator	first response	ternary	self._postproc.apply(first_response, postproc_params)
same	loop	ternary	self._postproc.apply(res, postproc_params)
openai_completion → completion_response	non-streaming	if/else	self._postproc.apply(response, postproc_params)
openai_completion → completion_generator	loop	if/else	self._postproc.apply(output, params)
chat_harmony → create_streaming_generator	first response	ternary	self._postproc.apply(first_response, postproc_params)
same	loop	ternary	self._postproc.apply(res, postproc_params)
openai_responses → create_response	non-streaming	if/else; the inline branch awaits responses_api_create_response(...)	await self._postproc.apply_async(promise, lambda: responses_api_create_response(...)) — the lambda is evaluated only in inline mode, so no coroutine is created and self.enable_store / self.tokenizer are read exactly as before
openai_responses → create_streaming_generator	loop	ternary	self._postproc.apply(res, postproc_params)

The two conditional unpacks that only fed the ternaries (if not self.postproc_worker_enabled: post_processor, args = ... in the chat and harmony stream generators) are deleted; in the Responses streaming generator the unpack keeps only args (post_processor became unused, args.streaming_processor is still read).

As a concrete example, the chat stream generator's first-response site (second row of the table) was:

                if not self.postproc_worker_enabled:
                    post_processor, args = postproc_params.post_processor, postproc_params.postproc_args
                first_response = await anext(promise)
                ...
                pp_results = first_response.outputs[
                    0]._postprocess_result if self.postproc_worker_enabled else post_processor(
                        first_response, args)

and is now:

                first_response = await anext(promise)
                ...
                pp_results = self._postproc.apply(first_response,
                                                  postproc_params)

The strategies themselves (in tensorrt_llm/serve/postprocess_handlers.py):

class WorkerPostprocStrategy(PostprocStrategy):

    def apply(self, result, postproc_params):
        return result.outputs[0].postprocess_result

    async def apply_async(self, result, inline):
        return result.outputs[0].postprocess_result


class InlinePostprocStrategy(PostprocStrategy):

    def apply(self, result, postproc_params):
        return postproc_params.post_processor(result,
                                              postproc_params.postproc_args)

    async def apply_async(self, result, inline):
        return await inline()


def select_postproc_strategy(postproc_worker_enabled: bool) -> PostprocStrategy:
    return (WorkerPostprocStrategy()
            if postproc_worker_enabled else InlinePostprocStrategy())

PostprocStrategy is the abstract base of the two; its docstring records how a server chooses (once, from num_postprocess_workers) and the known n > 1 recording difference below.

Deliberately unchanged — the eleven submission-side uses of postproc_worker_enabled: what generate_async receives (_postproc_params=... if ... else None, and the completion path's unconditional pass), filling postproc_args.tokenizer/num_prompt_tokens for in-server post-processing, the Responses store decision and its warning, plus the property itself. They decide what a request carries, not how a result is read; unifying them would change behavior.

Behavior

No behavior change. With more than one candidate (n > 1) the two modes already differ in what the frontend records (worker mode: only outputs[0] carries the result and metrics arrive merged; inline mode records metrics per candidate). That difference is documented in the PostprocStrategy docstring and deliberately not fixed here; it is a separate behavior-change PR.

The accessor (API review)

• CompletionOutput.postprocess_result (read-only property, Any): what the post-processor returned for this output when post-processing ran in a postproc worker; None otherwise.
• tests/unittest/api_stability/references/completion_output.yaml records the new property. references_committed/ is unchanged. Requesting the api-compatible label.

Moves

None.

CI bookkeeping

• Multi-GPU table: none (no new directories; openai_server.py and executor/ are already listed, so the multi-GPU stages run on this PR)
• CODEOWNERS: none
• legacy-files.txt: none (the three edited source files are already listed; the new test file is a new-code file under the default lint)
• Test lists: unittest/serve/test_postproc_strategy.py registered in tests/integration/test_lists/test-db/l0_cpu.yml (after the unittest/scripts entry)

In-flight work on the same code

Checked open PRs touching serve/openai_server.py, serve/postprocess_handlers.py and executor/result.py at branch time. "Typed chat completion core behind a thin HTTP wrapper" has not landed, and the chat SSE generator's error-handling change (#20867, merged) already moved the two chat stream decision points to their current shape, which this PR edits in place. The Responses streaming-generator rewrite (keepalive, sequence renumbering) has not landed; the Responses decision points are edited where they are today. Open PRs that touch nearby code but not the nine decision points: #21072, #21060, #14092 (Responses handlers), #21111 (chat path), #19461 (completion merge helper), #20946 (# type: ignore comments). Whichever lands second rebases.

Test Coverage

New (CPU, registered in l0_cpu.yml; scripts/test_to_stage_mapping.py names its four stages: CPU-Generic-arm-1, CPU-Generic-arm-2, CPU-Generic-x86-1, CPU-Generic-x86-2):

• tests/unittest/serve/test_postproc_strategy.py: the flag→strategy mapping; the worker strategy reads outputs[0].postprocess_result of a real GenerationResultBase and accepts postproc_params=None; the inline strategy calls the post-processor exactly once and returns its value; worker apply_async returns the stored value without calling inline; inline apply_async awaits it exactly once; a server chooses from num_postprocess_workers (0 → inline, 2 → worker) through the real postproc_worker_enabled property and caches the choice; CompletionOutput(0).postprocess_result is None and returns the stored value.

Changed tests:

• tests/unittest/grpc/openengine/test_metrics.py: its minimal generator args now explicitly set num_postprocess_workers=0, so the real server constructor can select the inline strategy during the HTTP metrics contract test.
• tests/unittest/llmapi/test_executor.py: also asserts the public accessor returns the worker's value.
• tests/unittest/llmapi/run_llm_with_postproc.py (GPU, postproc-worker e2e): reads the public accessor, exercising it end to end.
• tests/unittest/llmapi/apps/test_anthropic_routes.py, tests/unittest/disaggregated/test_disagg_responses_api.py: the fake engine outputs now carry the public attribute name the worker strategy reads (one-word rename each; assertions unchanged).

Commands run locally (CPU-only dev box, source-built artifacts matching the base commit; each file in its own pytest process, as the CPU stage runs them):

• pytest -m cpu_only tests/unittest/serve/test_postproc_strategy.py -q — 8 passed
• pytest -m cpu_only tests/unittest/llmapi/test_executor.py -q — 44 passed (needs LLM_MODELS_ROOT: 3 of its 44 tests load tokenizers)
• pytest tests/unittest/api_stability -q — 64 passed (run without -m, so the serve API and CLI references are checked too)
• pytest -m cpu_only tests/unittest/disaggregated/test_disagg_responses_api.py tests/unittest/disaggregated/test_openai_disagg_service.py tests/unittest/disaggregated/test_openai_server_stream_errors.py tests/unittest/disaggregated/test_router.py -q — 322 passed
• pytest -m cpu_only tests/unittest/disaggregated -q — 1411 passed, 1 skipped, 2 xfailed
• pytest -m cpu_only tests/unittest/llmapi/apps -q — 2260 passed, 1 failed: test_render_standalone.py::TestCpuOnlyStartup::test_the_launcher_loads_a_cuda_stub_only_when_the_driver_is_missing fails identically on the unmodified base commit in this environment (driver-detection test; unrelated to this change)
• git grep -n "_postprocess_result" -- tensorrt_llm/serve — no output; grep -c "postproc_worker_enabled" tensorrt_llm/serve/openai_server.py — 13
• python scripts/test_to_stage_mapping.py --tests "unittest/serve/test_postproc_strategy.py" — names the four CPU stages listed above; grep -c "pytest.mark.cpu_only" — 1
• pre-commit run --files <all changed> — twice, all hooks pass, no file re-modified
• A differential property check (Hypothesis, 300 generated cases) that the strategy dispatch returns/raises exactly what the old per-site expressions did — including post-processor call counts and arguments, and that the Responses inline coroutine is created only in inline mode — against the shipped code (AST-extracted), gated by 7 planted mutations (flag inversion, wrong post-processor arguments, missing await, accessor returning None, wrong output index, worker-mode inline call): 7/7 detected

Follow-up validation on 5806927953: reproduced the CPU CI http_enablement failure before the fixture correction; 73 CPU tests passed across OpenEngine metrics, post-processing strategy, runtime control, RL control authentication, and sleep-mode construction after the correction. Focused lint and commit hooks passed. The all-files hook run passed its formatting/lint/consistency checks, but type checking could not complete because mypy is unavailable and that hook interpreter has a native-library ABI mismatch.

GPU coverage is left to CI: the e2e wrappers that run every handler with num_postprocess_workers 0 and 2 (chat and completions on the single-GPU stage; harmony and Responses on the H100 stage), and the postproc-worker example test that now reads the public accessor.

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

[JunyiXu-nv]

/bot run

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: NVIDIA/TensorRT-LLM
• Branch: main
• File: .coderabbit.yaml
• Commit: 5d86481
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 174c9b74-4532-431c-b937-c03c02a1c848

Commits

Reviewing files that changed from the base of the PR and between e6df420 and 5806927.

Files selected for processing: 1

• tests/unittest/grpc/openengine/test_metrics.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

Completion outputs now expose the stored postprocessed value. OpenAI server handlers select and use worker or inline postprocessing strategies for chat, completion, Harmony, and Responses API requests.

Changes

Postprocessing strategy

Layer / File(s)	Summary
Result accessor and strategy behavior tensorrt_llm/executor/result.py, tensorrt_llm/serve/postprocess_handlers.py, tests/unittest/serve/test_postproc_strategy.py, tests/unittest/api_stability/references/completion_output.yaml, tests/unittest/disaggregated/test_disagg_responses_api.py, tests/unittest/llmapi/apps/test_anthropic_routes.py, tests/unittest/llmapi/run_llm_with_postproc.py, tests/unittest/llmapi/test_executor.py, tests/integration/test_lists/test-db/l0_cpu.yml	CompletionOutput.postprocess_result returns the stored value. Worker and inline strategies implement synchronous and asynchronous processing. Tests cover strategy behavior and update result access to use the public property.
Server strategy selection and response handling tensorrt_llm/serve/openai_server.py, tests/unittest/serve/test_postproc_strategy.py, tests/unittest/grpc/openengine/test_metrics.py	OpenAIServer selects and caches a strategy. Chat, completion, Harmony, and Responses API handlers use it for synchronous, asynchronous, and streaming processing. The metrics test arguments now set num_postprocess_workers to 0.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant OpenAIServer
  participant select_postproc_strategy
  participant WorkerPostprocStrategy
  participant InlinePostprocStrategy
  participant CompletionOutput
  participant InlinePostprocessor
  OpenAIServer->>select_postproc_strategy: select strategy from worker setting
  select_postproc_strategy-->>OpenAIServer: return strategy
  OpenAIServer->>WorkerPostprocStrategy: apply generated result
  WorkerPostprocStrategy->>CompletionOutput: read postprocess_result
  CompletionOutput-->>WorkerPostprocStrategy: return stored value
  OpenAIServer->>InlinePostprocStrategy: apply generated result
  InlinePostprocStrategy->>InlinePostprocessor: invoke or await processing
  InlinePostprocessor-->>InlinePostprocStrategy: return processed value

Loading

Possibly related issues

• Issue 21309 reports repeated streamed text for n > 1 in the trtllm-serve flow. This change refactors streaming postprocessing but does not address the reported detokenization cause.

Suggested reviewers: juney-nvidia

Merge Risk: Minimal · up to 58069

No actionable merge-blocking risk was identified in the reviewed change.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title is concise, specific, and accurately describes the main change: selecting the trtllm-serve post-processing path once per server.
Description check		The description includes the required Description, Test Coverage, and PR Checklist sections. It clearly explains the motivation, implementation, behavior, API change, tests, CI results, and known unre…
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: Description check

Explanation

The description includes the required Description, Test Coverage, and PR Checklist sections. It clearly explains the motivation, implementation, behavior, API change, tests, CI results, and known unrelated failure.

Full details: Docstring Coverage

Explanation

Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. 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 16 functions across 9 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

[tensorrt-cicd]

PR_Github #81846 [ run ] triggered by Bot. Commit: e6df420 Link to invocation

[tensorrt-cicd]

PR_Github #81846 [ run ] completed with state FAILURE. Commit: e6df420
/LLM/main/L0_MergeRequest_PR pipeline #67498 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

• Please check the failed tests and fix your PR
• If you cannot view the failures, ask the CI triggerer to share details
• Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

[JunyiXu-nv]

/bot run --disable-multi-gpu-test

[tensorrt-cicd]

PR_Github #81865 [ run ] triggered by Bot. Commit: 5806927 Link to invocation

[tensorrt-cicd]

PR_Github #81865 [ run ] completed with state SUCCESS. Commit: 5806927
/LLM/main/L0_MergeRequest_PR pipeline #67517 (Partly Tested) completed with status: 'SUCCESS'

CI Report

Link to invocation