[spped2000]

Dev Engineer Review

DetokenizedGenerationResultBase._handle_response now fully decodes retained token IDs when stop-word trimming removes IDs that were already streamed. It clears incremental decoder state and updates the token-ID watermark. This prevents stale stop-word text and replacement characters from appearing in final .text.

The change does not retract stop-word prefixes already sent in OpenAI-compatible stream deltas. A holdback mechanism remains out of scope. No public API changes are reported.

QA Engineer Review

Added tests/unittest/executor/test_stop_word_streaming_text.py and listed it in the CPU CI file tests/integration/test_lists/test-db/l0_cpu.yml. The tests cover both detokenization backends, chunk sizes one and two, previously streamed stop IDs, final-response stops, include_stop_str_in_output, length finishes, and sibling sequences. The author reports that all 20 CPU-only cases pass; test execution was not independently verified here.

Coverage verdict: sufficient for the reported regression and listed edge cases. The tests also assert that joined deltas still contain </answer for the answer-tag case; previously emitted deltas cannot be retracted.

Per-File QA Perspective

• tensorrt_llm/executor/result.py: Verify that final streamed text matches non-streamed text after trimming previously streamed stop IDs, and that ordinary incremental decoding remains unchanged when no prior IDs were trimmed.
• tests/unittest/executor/test_stop_word_streaming_text.py: Covers stop trimming, incomplete Thai byte sequences, final-response and length cases, and sibling sequences across both backends. Listed in the CPU CI test database.
• tests/integration/test_lists/test-db/l0_cpu.yml: Adds the regression test to CPU CI selection.

Description

Fixes #21318.

When the sampler matches a stop word by token id and include_stop_str_in_output is False, _handle_sequence trims the stop ids from token_ids (_trim_stop_word_outputs). A non-streamed request then decodes the trimmed ids. A streamed request does not: the stop ids that arrived in earlier responses were already passed to decode_incrementally, and token_ids_diff is empty after the trim. So the final .text keeps the start of the stop string, for example '<answer>42</answer' instead of '<answer>42'.

Since #20424, the HF backend flushes held bytes on the final response. When the stop word completes a character whose lead bytes the decoder was still holding, that flush appends U+FFFD. This happens for every Thai digit ๐-๙ and the marks ๊ ํ ๎: on the Qwen3 tokenizer and the Typhoon 2.5 / Typhoon-S tokenizers built on it, each of these 13 characters is token 8158 plus one continuation-byte token ("๒" is [8158, 240]). With stop="๒", the streamed text is 'ข้อ ๑ ถูก ข้อ �' against 'ข้อ ๑ ถูก ข้อ ' non-streamed. The TRTLLM backend shows both symptoms too.

Fix. In DetokenizedGenerationResultBase._handle_response, if the trim reached ids from earlier responses (len(token_ids) < _last_token_ids_len), skip the incremental step. Instead:

• decode the kept ids with tokenizer.decode, as the non-streaming and beam-search paths do;
• reset _incremental_states, which drops the pending flush;
• move _last_token_ids_len to the kept ids. token_ids_diff is [] before and after this, so consumers see no difference. It makes the gate fire once per trimmed output: with n>1, later responses for sibling candidates still walk every output, and without it the finished candidate would be fully decoded again on each of them.

The final streamed .text is then tokenizer.decode of the kept token_ids, as in the non-streaming path. In every case tested here it equals the non-streamed .text.

The gate is false, so the incremental path runs as before, for:

• generating responses;
• a stop word that arrives entirely in the final response, whose ids were never decoded;
• include_stop_str_in_output=True;
• END_ID and LENGTH finishes;
• stops matched by text (such as the pinned "Hello there! " case in test_llm.py);
• every response after the one that trimmed the output.

With n>1 streaming, an output that a response does not touch still re-feeds its previous token_ids_diff to the incremental decoder. That is older than this change and is what #9802 (closed) tried to address; this PR does not change it.

Effect.

• The LLM API's final streamed .text is fixed.
• The gRPC OpenEngine finish should be fixed as well, from code reading (not run): grpc/openengine/streaming.py:104-130 sends output.text up to its full length once finish_reason is set.
• For the Thai cases, the '�' is gone from the last text_diff, which is what the OpenAI-compatible stream handlers send as the last delta.

Not covered. The OpenAI-compatible stream handlers send each text_diff as soon as it is decoded. Deltas that already carried a stop prefix (คำตอบ, </answer) before the stop matched are not taken back. That needs a holdback like the gRPC path's _StopPrefixTracker and is left for a separate change.

Test Coverage

• New tests/unittest/executor/test_stop_word_streaming_text.py (cpu_only, registered in l0_cpu.yml, 20 cases). It drives the real DetokenizedGenerationResultBase with a small byte-fallback tokenizer, for both detokenization backends and for 1 and 2 tokens per response:

  • Final text matches. A Thai digit stop (held lead bytes) and a </answer> stop (leading tokens already decoded) both give a final streamed .text equal to the non-streamed text.
  • No U+FFFD. The joined Thai deltas contain no U+FFFD.
  • n=2. Candidate 0 streams to a trimmed Thai stop, then candidate 1 streams to LENGTH. Candidate 0 keeps the clean text, has an empty text_diff on each of candidate 1's responses, and is fully decoded exactly once.
  • Behaviour that must not change. A stop word that arrives entirely in the final response, include_stop_str_in_output=True, and a LENGTH finish that still flushes held bytes.

• Pre-fix vs post-fix. These runs use the 1.4.0rc2 release container on CPU. Its executor/result.py is byte-identical to main (7d8d94684) before this change.

  • Before this change, 14 of the 20 new cases fail. The 6 that pass are the must-not-change cases.
  • With this change, all 20 pass.
  • Run together with the existing tests listed below: 14 failed and 56 passed before, 70 passed after. All 14 failures are in the new file.

• Existing tests that still pass:

  • tests/unittest/executor/test_stop_word_side_output_trim.py
  • tests/unittest/llmapi/test_tokenizer_decode.py
  • tests/unittest/executor/test_postprocessor_hook.py
  • tests/unittest/llmapi/test_executor.py::test_result_stream_sequence_alignment

• Real tokenizer (CPU). With the Qwen3-0.6B tokenizer, the streamed and non-streamed .text differ in 6 of 6 cases before the change and in 0 of 6 after, on both backends.

• n=2 decode count (CPU). In an interleaved n=2 probe, the finished candidate is fully decoded 6 times without the watermark move and once with it.

• GB10 end to end (DGX Spark, release:1.4.0rc2 with this change's result.py copied in for the after run, Qwen3-0.6B, greedy, LLM.generate_async(streaming=True), stop "๒" and "๓"; the engine matches [8158, 240] / [8158, 241]):

  • Before this change, the streamed .text is ' �' against ' ' non-streamed.
  • After this change, both are ' ', and so are the joined deltas.
  • In a third case, the model emitted ">\n", so the stop matched only at the text level. Its final .text is '' both before and after, as expected. Its joined deltas are '</answer' both before and after. That is a live example of the delta leak described under Not covered.

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.

---

AI assistance: this change, its tests and this description were prepared with the help of Claude Code; every test and repro cited here was run locally.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ECBhwNjbBwLHSMWvZEbxTD

[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: f58c2fe
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 1fea41e6-be53-40a5-8040-996679d181a0

Commits

Reviewing files that changed from the base of the PR and between 4acebf2 and 92fe570.

Files selected for processing: 1

• tests/unittest/executor/test_stop_word_streaming_text.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

When streamed token IDs are trimmed after stop-word matching, the result handler decodes the retained IDs and resets incremental decoder state. CPU tests compare streamed and non-streamed results and cover stop-word, length-completion, and multi-sequence cases.

Changes

Streamed stop-word text

Layer / File(s)	Summary
Decode trimmed streamed output tensorrt_llm/executor/result.py	When retained token IDs are shorter than the prior watermark, the handler uses full decoding. It clears incremental decoder state and updates the watermark to the retained token count.
Test streamed stop-word handling tests/unittest/executor/test_stop_word_streaming_text.py, tests/integration/test_lists/test-db/l0_cpu.yml	CPU tests cover both detokenization backends, stop words, length completion, and multiple sequences. The CPU integration test list includes the test module.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

Suggested reviewers: juney-nvidia, junyixu-nv, brnguyen2

Merge Risk: Minimal · up to 92fe5

No merge-blocking risk is established for this change.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% 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 clearly identifies the fix: it removes streamed stop-word IDs from the final streamed text. It uses the required issue and type format.
Description check		The description explains the issue, implementation, scope limits, test coverage, observed results, and checklist status. It matches the required template and provides sufficient technical detail.
Linked Issues check		Issue [#21318] requires final streamed .text to match non-streamed text after token-ID stop trimming, including multi-token stops and pending Thai byte flushes. `DetokenizedGenerationResultBase._han…
Out of Scope Changes check		The changes are limited to the linked issue's detokenization fix, focused automated tests, and CPU test-list registration. The test cases validate required behavior and unchanged behavior. No unrelate…

Full details: Linked Issues check

Explanation

Issue [#21318] requires final streamed .text to match non-streamed text after token-ID stop trimming, including multi-token stops and pending Thai byte flushes. DetokenizedGenerationResultBase._handle_response now detects when trimming removes previously streamed IDs, decodes the retained IDs with tokenizer.decode, clears _incremental_states, and advances _last_token_ids_len. The new CPU tests cover HF and TRTLLM backends, chunk sizes one and two, Thai replacement-character cases, answer-tag stops, final-response-only stops, include_stop_str_in_output, length finishes, and interleaved n=2 responses. The change does not claim to retract already emitted OpenAI-compatible deltas, which #21318 excludes.

Full details: Out of Scope Changes check

Explanation

The changes are limited to the linked issue's detokenization fix, focused automated tests, and CPU test-list registration. The test cases validate required behavior and unchanged behavior. No unrelated product behavior or unrelated files appear in the reviewed change summary.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% 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 4 functions across 2 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

_{Comment @coderabbitai help to get the list of available commands.}

[brnguyen2]

Approving — the comments below are optional touch-ups, not blockers.

The gate is correct as written. _handle_sequence sets _last_token_ids_len before it extends token_ids, and the trim only removes trailing ids. So whenever any stop id came in an earlier response, the final response holds only stop ids and len(token_ids) < _last_token_ids_len fires exactly then. The sibling (n>1) test is a good catch for the watermark update.

Two optional follow-ups:

1. Pin the known delta gap in a test. For answer_tag, the concatenated streamed deltas still end in </answer while the final .text is <answer>42, because the earlier chunks already sent that prefix. The description says this is out of scope, but no test records it. Add an assertion (or an xfail with a pointer to #21318) on the joined deltas for answer_tag, so a future holdback change has a baseline and nobody reads test_streamed_deltas_do_not_end_in_replacement_character as covering multi-char stops in general.

2. Make the trim signal explicit (nit). _handle_response works out "a trim reached streamed ids" by comparing two lengths that _handle_sequence happens to leave in a particular order. If _trim_stop_word_outputs (or its caller) recorded how many already-streamed ids it removed, this path wouldn't depend on the watermark ordering in _handle_sequence. Not blocking.

[spped2000]

Thanks for the review, @brnguyen2.

1. Done in 92fe570. test_streamed_text_drops_stop_word_ids_streamed_earlier now asserts the joined deltas too: "<answer>42</answer" for answer_tag (chunk sizes 1 and 2) and "ข้อ ๑ ข้อ " for thai_digit. A comment above CASES points to [Bug]: Streamed final text keeps the start of a multi-token stop word; Thai digit and tone-mark stops end in U+FFFD #21318, so a holdback change knows which value to update. The file passes (20 tests) in the 1.4.0rc2 image, CPU only.

2. Agreed that the length comparison relies on the order in _handle_sequence. I kept the comparison so this PR stays a small change inside _handle_response. Having _trim_stop_word_outputs return how many already-streamed ids it removed is a small change too (it has one caller), so I can add it here if you prefer.

[spped2000]

Thanks for the reviews, @brnguyen2 and @zhaoyangwang-nvidia. Could a maintainer trigger /bot run when convenient, so CI can run on 92fe570? Thanks!

[zhaoyangwang-nvidia]

/bot run

[github-actions]

Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements.

[tensorrt-cicd]

PR_Github #82648 [ run ] triggered by Bot. Commit: 92fe570 Link to invocation

[tensorrt-cicd]

PR_Github #82648 [ run ] completed with state SUCCESS. Commit: 92fe570
/LLM/main/L0_MergeRequest_PR pipeline #68236 completed with status: 'SUCCESS'
Pipeline passed with automatic retried tests. Check the rerun report for details.

CI Report

Link to invocation