[None][fix] Make LlmRequest.is_generation_only_request a property to match the C++ binding by eopXD · Pull Request #18858 · NVIDIA/TensorRT-LLM

eopXD · 2026-09-08T02:56:03Z

Description

Two objects reach Python as LlmRequest: the C++ base, which the KV connector trampoline hands to a connector implementation, and the Python subclass, which every other caller holds. The base binds is_generation_only_request as a read-only property; the subclass overrode it with a plain method.

What this means for an API user: request.is_generation_only_request is now a bool on every LlmRequest you can be handed. Before, on the subclass it evaluated to a bound method — always truthy — so any guard reading it treated EVERY request as generation-only. Nothing raised and nothing logged.

The fix is @property on the subclass, plus the eight in-tree callers and the test doubles moved off (). No in-tree behaviour change: each existing caller already used the spelling that reads correctly for the object it holds.

Test Coverage

tests/unittest/_torch/executor/test_kv_connector_generation_only.py — builds a real LlmRequest of each request type and drives it through KvCacheConnectorManager.get_num_new_matched_tokens. No test-list change: unittest/_torch/executor is already an entry in l0_cpu.yml and the file is cpu_only.

How the change is guaranteed — the test run against an installed tensorrt_llm in nvcr.io/nvidia/pytorch:26.05-py3, toggling only the @property line and nothing else:

def is_generation_only_request(self):   ->  1 failed
    E  assert not is_generation_only_request

@property                               ->  1 passed
def is_generation_only_request(self):

Question a reviewer will ask	Test
Does a real request flip the connector guard, or only a stub?	`tests/unittest/_torch/executor/test_kv_connector_generation_only.py:53`
Does a real generation-only request still read `True` through the subclass?	`tests/unittest/_torch/executor/test_request_utils.py:104`
Does the single-token context-graph fallback still see generation-only requests?	`tests/unittest/_torch/executor/test_pytorch_model_engine.py:475`
Does the suffix automaton still initialize only generation-only, non-dummy requests?	`tests/unittest/_torch/speculative/hw_agnostic/test_sa.py:319`
Does the disagg KV slice still take the generation-only branch?	`tests/unittest/disaggregated/test_cache_reuse_adapter.py:493`

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.

…match the C++ binding The C++ base binds the flag as a read-only property (`nanobind/batch_manager/bindings.cpp:221`), while the Python subclass overrode the same name with a plain method. One name, two spellings, and which one is correct depends on the object in hand: the base class needs `req.is_generation_only_request`, the subclass needed `req.is_generation_only_request()`. Both objects reach Python. The connector trampoline (`nanobind/batch_manager/kvCacheConnector.cpp:36`) takes `LlmRequest const&`, which nanobind casts by copy, so a callback invoked from C++ receives a base-class instance; every other caller holds the Python subclass. A guard that reads the attribute on the subclass gets a bound method -- always truthy, never raising, never logged -- and takes the generation-only branch for every request. Add `@property` to the subclass so it matches the base, and convert the eight in-tree callers plus the test fakes. No behaviour changes: every current caller already used the spelling that reads correctly for the object it holds. Signed-off-by: Yueh-Ting Chen <yuehtingc@nvidia.com>

eopXD · 2026-09-08T02:59:12Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-09-08T03:05:08Z

PR_Github #72028 [ run ] triggered by Bot. Commit: 07d5d4d Link to invocation

coderabbitai · 2026-09-08T03:05:36Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 14fc2af0-5801-47d1-aa92-c15415ffef94

📥 Commits

Reviewing files that changed from the base of the PR and between 7fb64ae and 07d5d4d.

📒 Files selected for processing (16)

tensorrt_llm/_torch/disaggregation/transceiver.py
tensorrt_llm/_torch/pyexecutor/llm_request.py
tensorrt_llm/_torch/pyexecutor/model_engine.py
tensorrt_llm/_torch/pyexecutor/perf_metrics_manager.py
tensorrt_llm/_torch/pyexecutor/py_executor.py
tensorrt_llm/_torch/pyexecutor/scheduler/scheduler_v2.py
tensorrt_llm/_torch/speculative/suffix_automaton.py
tests/unittest/_torch/executor/kv_cache/test_mamba_cache_manager.py
tests/unittest/_torch/executor/test_kv_connector_generation_only.py
tests/unittest/_torch/executor/test_perf_metrics_manager.py
tests/unittest/_torch/executor/test_pytorch_model_engine.py
tests/unittest/_torch/executor/test_request_utils.py
tests/unittest/_torch/speculative/hw_agnostic/test_sa.py
tests/unittest/disaggregated/test_cache_reuse_adapter.py
tests/unittest/disaggregated/test_chunked_transfer.py
tests/unittest/disaggregated/test_kv_transfer.py

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

Walkthrough

The change exposes LlmRequest.is_generation_only_request as a read-only property. Production call sites and test fixtures now use boolean property access. A regression test covers scheduler bypass for generation-only requests.

Changes

Generation-only request property

Layer / File(s)	Summary
Property contract and consumer migration `tensorrt_llm/_torch/...`	`LlmRequest.is_generation_only_request` is now a read-only property. Executor, scheduler, disaggregation, metrics, and speculative code access it directly.
Fixture and regression validation `tests/unittest/_torch/...`, `tests/unittest/disaggregated/...`	Test fixtures use boolean values. A regression test verifies that generation-only requests fail before scheduler access.

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

Merge Risk: ⚪ Minimal · up to 07d5d

Generation-only classification now uses a boolean property, so context requests reach connector scheduling while generation-only requests are rejected as intended. The updated callers and regression coverage indicate no remaining merge-blocking risk.

Suggested reviewers: juney-nvidia

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 47.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 16 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title follows the required [None][fix] format and clearly identifies the main change: converting LlmRequest.is_generation_only_request to a property matching the C++ binding.
Description check	✅ Passed	The description explains the problem, solution, API impact, updated callers, regression coverage, test execution, and checklist status. It is complete and directly related to the changes.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

zhaoyangwang-nvidia

This is exactly the follow-up split we discussed on #18762 — scoped to just the property/method mismatch, all eight in-tree callers converted consistently, and the new test constructs a real LlmRequest via executor_request_to_llm_request and exercises both the context and generation-only paths through the real connector manager, which pins down the base-class vs. subclass attribute-access bug this fixes. LGTM.

tensorrt-cicd · 2026-09-08T05:49:52Z

PR_Github #72028 [ run ] completed with state FAILURE. Commit: 07d5d4d
/LLM/main/L0_MergeRequest_PR pipeline #59086 completed with status: 'UNSTABLE'

CI Report

⚠️ Multi-GPU Label Required:
Multi-GPU tests require the ci: full pre-merge approved label on this PR. Ask a member of NVIDIA/trt-llm-ci-approvers to add the label, then re-trigger CI with the same bot command (no rebase needed).

⚠️ 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

Link to invocation

eopXD · 2026-09-08T06:37:12Z

/bot skip --comment "The change of the current MR should be covered by the single-GPU pipeline. It is safe to skip the multi-GPU pipeline."

tensorrt-cicd · 2026-09-08T06:44:13Z

PR_Github #72098 [ skip ] triggered by Bot. Commit: 07d5d4d Link to invocation

tensorrt-cicd · 2026-09-08T06:54:43Z

PR_Github #72098 [ skip ] completed with state SUCCESS. Commit: 07d5d4d
Skipping testing for commit 07d5d4d

Link to invocation

VALLIS-NERIA

trivial

eopXD requested review from a team as code owners September 8, 2026 02:56

eopXD requested review from asfiyab-nvidia, bo-nv, liji-nv, mikeiovine, pcastonguay and yizhang-nv September 8, 2026 02:56

github-actions Bot assigned eopXD Sep 8, 2026

eopXD mentioned this pull request Sep 8, 2026

[TRTLLM-12891][feat] Support KV cache connector for v2_kvcm and extend VSWA support #18762

Open

1 task

zhaoyangwang-nvidia approved these changes Sep 8, 2026

View reviewed changes

eopXD enabled auto-merge (squash) September 8, 2026 06:37

chuangz0 approved these changes Sep 8, 2026

View reviewed changes

VALLIS-NERIA approved these changes Sep 8, 2026

View reviewed changes

eopXD merged commit 20333a9 into NVIDIA:main Sep 8, 2026
19 checks passed

Navigation Menu

Uh oh!

[None][fix] Make LlmRequest.is_generation_only_request a property to match the C++ binding - #18858

[None][fix] Make LlmRequest.is_generation_only_request a property to match the C++ binding#18858
eopXD merged 1 commit into
NVIDIA:mainfrom
eopXD:fix/is-generation-only-request-property

eopXD commented Sep 8, 2026 •

edited

Loading

Uh oh!

eopXD commented Sep 8, 2026

Uh oh!

tensorrt-cicd commented Sep 8, 2026

Uh oh!

coderabbitai Bot commented Sep 8, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

zhaoyangwang-nvidia left a comment

Uh oh!

tensorrt-cicd commented Sep 8, 2026

Uh oh!

eopXD commented Sep 8, 2026

Uh oh!

tensorrt-cicd commented Sep 8, 2026

Uh oh!

tensorrt-cicd commented Sep 8, 2026

Uh oh!

VALLIS-NERIA left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

eopXD commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test Coverage

PR Checklist

GitHub Bot Help

Uh oh!

eopXD commented Sep 8, 2026

Uh oh!

tensorrt-cicd commented Sep 8, 2026

Uh oh!

coderabbitai Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

zhaoyangwang-nvidia left a comment

Choose a reason for hiding this comment

Uh oh!

tensorrt-cicd commented Sep 8, 2026

Uh oh!

eopXD commented Sep 8, 2026

Uh oh!

tensorrt-cicd commented Sep 8, 2026

Uh oh!

tensorrt-cicd commented Sep 8, 2026

Uh oh!

VALLIS-NERIA left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

eopXD commented Sep 8, 2026 •

edited

Loading

coderabbitai Bot commented Sep 8, 2026 •

edited

Loading