[None][feat] Preserve cache_salt string in KV cache events

[jthomson04]

Summary

• Thread the original cache_salt string through the full pipeline (Python LLM API → C++ Request → LlmRequest → BlockKey → KVCacheStoredBlockData → nanobind → Python serializer) so it appears in KV cache stored-block events
• Previously only the hashed uint64 cache_salt_id crossed into C++; the original string was discarded, making it impossible for external cache managers to identify which tenant a cached block belongs to without a side-channel mapping
• Follows the precedent set by multimodal UUIDs (MmKey.uuid) — the string is carried alongside the existing cacheSaltID (uint64) and does not participate in hashing or matching

Test plan

• pytest tests/unittest/llmapi/test_llm_kv_cache_events.py — verifies cache_salt field presence in serialized events (None for unsalted, original string for salted)
• pytest tests/unittest/bindings/test_executor_bindings.py — binding integrity
• Verify C++ unit tests still pass (serializeUtilsTest, kvCacheManagerTest)
• Build passes (19 files changed across C++ core, nanobind, Python, tests, example)

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added support for an optional cache_salt parameter to generation requests. This parameter is now propagated and preserved throughout the system for event reporting and serialization purposes, complementing the existing cache salt ID mechanism.

• Tests

  • Added tests to verify that the cache salt value is correctly preserved in KV cache events and accessible for reporting.

[coderabbitai]

📝 Walkthrough

Walkthrough

This change introduces support for propagating an optional string cache_salt field through the request lifecycle. The field is added alongside the existing numeric cacheSaltID across the C++ core (request objects, block keys, KV cache structures), serialization logic, Python bindings, and high-level Python APIs. Constructors are extended with an optional parameter, getter/setter methods are added, and serialization is updated to preserve the value through event reporting.

Changes

Cohort / File(s)	Summary
Core Type Definitions cpp/include/tensorrt_llm/batch_manager/blockKey.h, cpp/include/tensorrt_llm/batch_manager/llmRequest.h, cpp/include/tensorrt_llm/executor/executor.h	Added std::optional<std::string> cacheSalt field alongside constructor parameter extensions to BlockKey, GenericLlmRequest, LlmRequest, and KVCacheStoredBlockData; introduced getter methods (getCacheSalt()) where applicable.
Core Implementations cpp/tensorrt_llm/batch_manager/blockKey.cpp, cpp/tensorrt_llm/executor/request.cpp, cpp/tensorrt_llm/executor/requestImpl.h, cpp/tensorrt_llm/batch_manager/kvCacheEventManager.cpp	Updated buildBlockKeys() to forward cacheSalt via llmRequest.getCacheSalt(), extended Request::Impl constructor and added getter/setter for cacheSalt, updated event payload construction to include cacheSalt in block data.
Serialization Support cpp/tensorrt_llm/executor/serialization.cpp	Extended serialization/deserialization for cacheSalt across Request, KVCacheStoredBlockData, and BlockKey types; updated visitMembers and size-computation logic to include the new field in (de)serialization streams.
Python C++ Bindings cpp/tensorrt_llm/nanobind/batch_manager/bindings.cpp, cpp/tensorrt_llm/nanobind/executor/bindings.cpp, cpp/tensorrt_llm/nanobind/executor/request.cpp	Exposed cache_salt as read-only property on GenericLlmRequest and KVCacheStoredBlockData; extended LlmRequest and Request constructor bindings to accept cache_salt parameter; updated __getstate__/__setstate__ to handle backward-compatible serialization of the new field.
Python High-Level API tensorrt_llm/executor/request.py, tensorrt_llm/executor/executor.py, tensorrt_llm/executor/base_worker.py, tensorrt_llm/_torch/pyexecutor/llm_request.py, tensorrt_llm/llmapi/llm.py	Extended GenerationRequest to store cache_salt attribute; updated GenerationExecutor.generate_async() to accept and forward cache_salt; updated request construction in base_worker.py and llm_request.py to pass through the new parameter; updated BaseLLM.generate_async() to forward user-provided cache_salt.
JSON Serialization & Examples tensorrt_llm/_utils.py, examples/cpp/executor/executorExampleKvEvents.cpp, tests/unittest/llmapi/test_llm_kv_cache_events.py	Extended KV cache event JSON serialization to include "cache_salt" field; updated KVCacheBlock constructor to accept and store cacheSalt; added tests validating cache_salt presence and values in serialized KV cache events (test_cache_salt_in_stored_events).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.45% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main objective: preserving the cache_salt string in KV cache events, which is the core purpose of this PR across all 19 files.
Description check	✅ Passed	The PR description includes a clear summary of what changed and why, test coverage is documented, and the PR checklist is present. However, the description provided by the author is minimal and doesn't fully address all template sections like 'Description' details and explicit test coverage list.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

examples/cpp/executor/executorExampleKvEvents.cpp (1)

2-2: ⚠️ Potential issue | 🟡 Minor

Update copyright year on this modified source file.

The file now contains new changes, but Line 2 still ends at 2024.

As per coding guidelines "Add NVIDIA copyright header on ALL new files and update year on modified files."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@examples/cpp/executor/executorExampleKvEvents.cpp` at line 2, Update the SPDX
copyright year range on this modified file so it reflects the current
modification year; specifically, in executorExampleKvEvents.cpp replace the
trailing year "2024" in the SPDX header comment with the current year (e.g.,
change "2022-2024" to "2022-2026") so the header complies with the project
policy.

cpp/include/tensorrt_llm/batch_manager/llmRequest.h (1)

2-2: ⚠️ Potential issue | 🟡 Minor

Update the copyright year in this modified header.

Line 2 still ends at 2025, but this file includes 2026 changes.

As per coding guidelines "Add NVIDIA copyright header on ALL new files and update year on modified files."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cpp/include/tensorrt_llm/batch_manager/llmRequest.h` at line 2, Update the
top-of-file copyright header in the modified header to include 2026: locate the
copyright line that currently reads "Copyright (c) 2022-2025, NVIDIA
CORPORATION." and change the year range to include 2026 (e.g., "2022-2026") so
the header on tensorrt_llm/batch_manager/llmRequest.h reflects the modification
year.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cpp/include/tensorrt_llm/executor/executor.h`:
- Around line 733-734: The Doxygen for the Request constructor is missing
documentation for the newly added parameter cacheSalt; update the constructor's
Doxygen comment (the one above the Request(...) declaration) to add a line
documenting the cacheSalt parameter (std::optional<std::string> cacheSalt =
std::nullopt), placed after the disaggRequestId entry, using the repository's
single-line Doxygen style (//! and `@param` or the existing param lines format) so
the Request constructor docs include cacheSalt and its default behavior;
reference the Request constructor signature and the cacheSalt parameter name
when adding the doc line.

In `@cpp/tensorrt_llm/executor/requestImpl.h`:
- Around line 51-52: The cacheSalt parameter and setCacheSalt(std::string) must
be validated and kept truly optional: add a single helper (e.g.,
normalizeCacheSaltOptional or sanitizeCacheSalt) that accepts
std::optional<std::string>, enforces a MAX_CACHE_SALT_LEN, truncates or rejects
overlong salts (prefer reject/log), and returns std::optional<std::string> to
store; call this helper from the constructor overloads (the ctor taking
std::optional<std::string> cacheSalt and any other ctors that accept cacheSalt)
and from setCacheSalt (change setCacheSalt(std::string) to route through the
helper or add an overload taking std::optional), preserving the ability to clear
to std::nullopt, and ensure BlockKey and stored-block serialization use the
validated optional value.

In `@tensorrt_llm/executor/request.py`:
- Line 107: Validate and bound the cache_salt parameter before attaching it to
Request objects: in the function(s) that accept cache_salt (the parameter
declared as cache_salt: Optional[str] and where the Request/requests are
created/annotated—e.g., the constructor or factory that stores cache_salt),
ensure cache_salt is either None or a str, and enforce a maximum length (define
a constant like MAX_CACHE_SALT_LEN); if it exceeds the limit either truncate it
to that max length or raise a ValueError (be consistent with existing input
validation policy). Apply the same guards where cache_salt is accepted again
(the other occurrence around the second signature) so per-block KV cache events
only carry type-checked, bounded strings.

---

Outside diff comments:
In `@cpp/include/tensorrt_llm/batch_manager/llmRequest.h`:
- Line 2: Update the top-of-file copyright header in the modified header to
include 2026: locate the copyright line that currently reads "Copyright (c)
2022-2025, NVIDIA CORPORATION." and change the year range to include 2026 (e.g.,
"2022-2026") so the header on tensorrt_llm/batch_manager/llmRequest.h reflects
the modification year.

In `@examples/cpp/executor/executorExampleKvEvents.cpp`:
- Line 2: Update the SPDX copyright year range on this modified file so it
reflects the current modification year; specifically, in
executorExampleKvEvents.cpp replace the trailing year "2024" in the SPDX header
comment with the current year (e.g., change "2022-2024" to "2022-2026") so the
header complies with the project policy.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: baf02ea9-81e4-422f-9df2-7441618cc6ac

📥 Commits

Reviewing files that changed from the base of the PR and between 2335ef8 and c9cb6ba.

📒 Files selected for processing (19)

• cpp/include/tensorrt_llm/batch_manager/blockKey.h
• cpp/include/tensorrt_llm/batch_manager/llmRequest.h
• cpp/include/tensorrt_llm/executor/executor.h
• cpp/tensorrt_llm/batch_manager/blockKey.cpp
• cpp/tensorrt_llm/batch_manager/kvCacheEventManager.cpp
• cpp/tensorrt_llm/executor/request.cpp
• cpp/tensorrt_llm/executor/requestImpl.h
• cpp/tensorrt_llm/executor/serialization.cpp
• cpp/tensorrt_llm/nanobind/batch_manager/bindings.cpp
• cpp/tensorrt_llm/nanobind/executor/bindings.cpp
• cpp/tensorrt_llm/nanobind/executor/request.cpp
• examples/cpp/executor/executorExampleKvEvents.cpp
• tensorrt_llm/_torch/pyexecutor/llm_request.py
• tensorrt_llm/_utils.py
• tensorrt_llm/executor/base_worker.py
• tensorrt_llm/executor/executor.py
• tensorrt_llm/executor/request.py
• tensorrt_llm/llmapi/llm.py
• tests/unittest/llmapi/test_llm_kv_cache_events.py

[nvpohanh]

@eopXD @thorjohnsen could you review this? Thanks

@lowsfer @yizhang-nv does this affect manager v2? Does v2 support this?

[jthomson04]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #44803 [ run ] triggered by Bot. Commit: 77e7b77 Link to invocation

[tensorrt-cicd]

PR_Github #44803 [ run ] completed with state SUCCESS. Commit: 77e7b77
/LLM/main/L0_MergeRequest_PR pipeline #35153 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

Link to invocation

[pcastonguay]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #45943 [ run ] triggered by Bot. Commit: b0d90db Link to invocation

[tensorrt-cicd]

PR_Github #45943 [ run ] completed with state ABORTED. Commit: b0d90db

Link to invocation

[pcastonguay]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #46156 [ run ] triggered by Bot. Commit: b0d90db Link to invocation

[tensorrt-cicd]

PR_Github #46156 [ run ] completed with state SUCCESS. Commit: b0d90db
/LLM/main/L0_MergeRequest_PR pipeline #36280 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

Link to invocation

[pcastonguay]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #46380 [ run ] triggered by Bot. Commit: b0d90db Link to invocation

[pcastonguay]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #51883 [ run ] triggered by Bot. Commit: 27f1d9b Link to invocation

[tensorrt-cicd]

PR_Github #51883 [ run ] completed with state SUCCESS. Commit: 27f1d9b
/LLM/main/L0_MergeRequest_PR pipeline #41241 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

[jthomson04]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #52111 [ run ] triggered by Bot. Commit: 27f1d9b Link to invocation

[tensorrt-cicd]

PR_Github #52111 [ run ] completed with state SUCCESS. Commit: 27f1d9b
/LLM/main/L0_MergeRequest_PR pipeline #41440 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

[pcastonguay]

/bot skip --comment "Flaky CI"

[tensorrt-cicd]

PR_Github #52376 [ skip ] triggered by Bot. Commit: 27f1d9b Link to invocation

[tensorrt-cicd]

PR_Github #52376 [ skip ] completed with state SUCCESS. Commit: 27f1d9b
Skipping testing for commit 27f1d9b

Link to invocation

[jthomson04]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #53113 [ run ] triggered by Bot. Commit: 2da8d93 Link to invocation

[tensorrt-cicd]

PR_Github #53113 [ run ] completed with state FAILURE. Commit: 2da8d93
/LLM/main/L0_MergeRequest_PR pipeline #42320 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

[jthomson04]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #53232 [ run ] triggered by Bot. Commit: 28c89a0 Link to invocation

[tensorrt-cicd]

PR_Github #53232 [ run ] completed with state SUCCESS. Commit: 28c89a0
/LLM/main/L0_MergeRequest_PR pipeline #42427 completed with status: 'SUCCESS'

CI Report

Link to invocation

[thorjohnsen]

Looks great.