[None][doc] add a guide for modifying APIs #7866
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughWalkthroughAdds a new Developer Guide document documenting LLM API change processes (levels, committed vs non‑committed APIs), YAML schema management, modification and promotion/deprecation workflows, validation/testing steps, and examples; updates docs index to include the new guide. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/bot run --stage-list "A10-Build_Docs" |
|
PR_Github #19327 [ run ] triggered by Bot |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
docs/source/developer-guide/api-change.md (6)
30-31: Use Sphinx-friendly cross‑refs instead of site‑root URL.Replace the absolute link with a :doc: (or :ref:) role to avoid broken links in versioned docs.
- - See [API status documentation](/llm-api/reference.html) for complete details + - See :doc:`API status <llm-api/reference>` for complete details
72-91: Add description in schema example for discoverability.Including “description” in YAML keeps docs/tests aligned with code.
garbage_collection_gen0_threshold: type: int default: 20000 status: beta # Must match the status in code + description: Threshold for Python GC of gen0 objects.
171-184: Clarify non‑breaking changes policy for committed APIs.State explicitly that adding optional params to committed APIs is allowed (backward‑compatible) and still requires schema/doc updates.
209-213: Add docstring update to promotion steps.Explicitly instruct removing any “status” mention in docstrings/type metadata when promoting.
216-227: Avoid hardcoding removal version (“v2.0”).Use a concrete planned version or a generic “a future release” to prevent stale docs.
- "This method is deprecated and will be removed in v2.0. " + "This method is deprecated and will be removed in a future release. "
195-203: Tiny UX improvement for test commands.Consider adding -q/--maxfail=1 for faster signal during local iterations.
-python -m pytest tests/unittest/api_stability/ +python -m pytest -q --maxfail=1 tests/unittest/api_stability/
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/source/developer-guide/api-change.md(1 hunks)docs/source/index.rst(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: venkywonka
PR: NVIDIA/TensorRT-LLM#6029
File: .github/pull_request_template.md:45-53
Timestamp: 2025-08-27T17:50:13.264Z
Learning: For PR templates in TensorRT-LLM, avoid suggesting changes that would increase developer overhead, such as converting plain bullets to mandatory checkboxes. The team prefers guidance-style bullets that don't require explicit interaction to reduce friction in the PR creation process.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (3)
docs/source/index.rst (1)
86-86: LGTM: adds the API Change Guide to Developer Guide toctree.Navigation entry looks correct and consistent with existing .md usage.
docs/source/developer-guide/api-change.md (2)
41-48: Replace "Pydantic dataclass" with "Pydantic BaseModel (StrictBaseModel); LlmArgs is an alias to TorchLlmArgs."TorchLlmArgs subclasses BaseLlmArgs (which subclasses StrictBaseModel → pydantic.BaseModel); the alias
LlmArgs = TorchLlmArgsis defined in tensorrt_llm/llmapi/llm_args.py (BaseLlmArgs ~line 1097, alias ~line 2435).Likely an incorrect or invalid review comment.
115-133: Add typing import and resolve missing GenerationOutputAdd "from typing import List, Iterator" to the example and either define/export GenerationOutput or replace it with an existing return type — repo search shows GenerationOutput only appears in this docs file (not in code).
Location: docs/source/developer-guide/api-change.md lines 115–133-@set_api_status("beta") -def generate_with_streaming( - self, - prompts: List[str], - **kwargs -) -> Iterator[GenerationOutput]: +from typing import List, Iterator +@set_api_status("beta") +def generate_with_streaming( + self, + prompts: List[str], + **kwargs, +) -> Iterator[GenerationOutput]:
syuoni
reviewed
Sep 19, 2025
nv-guomingz
reviewed
Sep 19, 2025
nv-guomingz
reviewed
Sep 19, 2025
|
PR_Github #19327 [ run ] completed with state |
|
/bot run --stage-list "A10-Build_Docs" |
|
/bot kill |
👎 Promotion blocked, new vulnerability foundVulnerability report
|
1 similar comment
👎 Promotion blocked, new vulnerability foundVulnerability report
|
|
/bot run --stage-list "A10-Build_Docs" |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/source/developer-guide/api-change.md (5)
30-31: Fix broken/absolute link; use Sphinx cross‑ref.Use :doc: so it works in built docs.
-- See [API status documentation](/llm-api/reference.html) for complete details +- See :doc:`LLM API status <llm-api/reference>` for complete details
34-38: Clarify “automatically validated” = enforced by CI tests.State that api_stability tests run in CI to avoid ambiguity.
- Automatically validated to ensure consistency + - Automatically validated by api_stability tests (run in CI) to ensure consistency
66-71: Align “Field requirements” with the Pydantic change.Status isn’t a Field kwarg; track it in YAML (or json_schema_extra if you adopt the optional pattern).
-**Status**: Required for non-committed arguments (`prototype`, `beta`, etc.) +**Status**: Track in the YAML schema for non‑committed arguments (`prototype`, `beta`, etc.). Do not pass as a Field kwarg.
76-83: YAML comment should not claim to mirror “code” after removing status kwarg.- status: beta # Must match the status in code + status: beta # Source of truth for argument status
104-105: Avoid hard‑coding a GitHub URL in docs.Use a repo‑relative path or Sphinx role so links stay valid across branches/tags.
-- The actual implementation is in the `_TorchLLM` class ([llm.py](https://github.com/NVIDIA/TensorRT-LLM/blob/release/1.0/tensorrt_llm/llmapi/llm.py)) +- The actual implementation is in the `_TorchLLM` class (:file:`tensorrt_llm/llmapi/llm.py`)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/source/developer-guide/api-change.md(1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: venkywonka
PR: NVIDIA/TensorRT-LLM#6029
File: .github/pull_request_template.md:45-53
Timestamp: 2025-08-27T17:50:13.264Z
Learning: For PR templates in TensorRT-LLM, avoid suggesting changes that would increase developer overhead, such as converting plain bullets to mandatory checkboxes. The team prefers guidance-style bullets that don't require explicit interaction to reduce friction in the PR creation process.
📚 Learning: 2025-07-28T17:06:08.621Z
Learnt from: moraxu
PR: NVIDIA/TensorRT-LLM#6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-09-09T09:40:45.658Z
Learnt from: fredricz-20070104
PR: NVIDIA/TensorRT-LLM#7645
File: tests/integration/test_lists/qa/llm_function_core.txt:648-648
Timestamp: 2025-09-09T09:40:45.658Z
Learning: In TensorRT-LLM test lists, it's common and intentional for the same test to appear in multiple test list files when they serve different purposes (e.g., llm_function_core.txt for comprehensive core functionality testing and llm_function_core_sanity.txt for quick sanity checks). This duplication allows tests to be run in different testing contexts.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-01T15:14:45.673Z
Learnt from: yibinl-nvidia
PR: NVIDIA/TensorRT-LLM#6506
File: examples/models/core/mixtral/requirements.txt:3-3
Timestamp: 2025-08-01T15:14:45.673Z
Learning: In TensorRT-LLM, examples directory can have different dependency versions than the root requirements.txt file. Version conflicts between root and examples dependencies are acceptable because examples are designed to be standalone and self-contained.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-21T21:48:35.135Z
Learnt from: djns99
PR: NVIDIA/TensorRT-LLM#7104
File: cpp/tensorrt_llm/cutlass_extensions/include/cutlass_extensions/epilogue/fusion/sm90_visitor_scatter.hpp:399-417
Timestamp: 2025-08-21T21:48:35.135Z
Learning: CUTLASS extensions in TensorRT-LLM (located under cpp/tensorrt_llm/cutlass_extensions/) are designed to integrate with and extend functionality in the external CUTLASS repository. When analyzing these extensions, their consumers and functionality wiring may exist in the CUTLASS codebase rather than within TensorRT-LLM itself.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-11T20:09:24.389Z
Learnt from: achartier
PR: NVIDIA/TensorRT-LLM#6763
File: tests/integration/defs/triton_server/conftest.py:16-22
Timestamp: 2025-08-11T20:09:24.389Z
Learning: In the TensorRT-LLM test infrastructure, the team prefers simple, direct solutions (like hard-coding directory traversal counts) over more complex but robust approaches when dealing with stable directory structures. They accept the maintenance cost of updating tests if the layout changes.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-21T00:16:56.457Z
Learnt from: farshadghodsian
PR: NVIDIA/TensorRT-LLM#7101
File: docs/source/blogs/tech_blog/blog9_Deploying_GPT_OSS_on_TRTLLM.md:36-36
Timestamp: 2025-08-21T00:16:56.457Z
Learning: TensorRT-LLM container release tags in documentation should only reference published NGC container images. The README badge version may be ahead of the actual published container versions.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-06T13:58:07.506Z
Learnt from: galagam
PR: NVIDIA/TensorRT-LLM#6487
File: tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_trtllm_bench.py:1-12
Timestamp: 2025-08-06T13:58:07.506Z
Learning: In TensorRT-LLM, test files (files under tests/ directories) do not require NVIDIA copyright headers, unlike production source code files. Test files typically start directly with imports, docstrings, or code.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-21T02:39:12.009Z
Learnt from: djns99
PR: NVIDIA/TensorRT-LLM#7104
File: cpp/tensorrt_llm/kernels/cutlass_kernels/moe_gemm/moe_kernels.cu:1475-1480
Timestamp: 2025-08-21T02:39:12.009Z
Learning: The min latency mode functionality in TensorRT-LLM MOE kernels (cpp/tensorrt_llm/kernels/cutlass_kernels/moe_gemm/moe_kernels.cu) is deprecated and no longer being maintained/updated, as confirmed by djns99. Bug reports and optimization suggestions for the computeStridesTmaWarpSpecializedLowLatencyKernel and related min latency code paths should be deprioritized.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-14T15:38:01.771Z
Learnt from: MatthiasKohl
PR: NVIDIA/TensorRT-LLM#6904
File: cpp/tensorrt_llm/pybind/thop/bindings.cpp:55-57
Timestamp: 2025-08-14T15:38:01.771Z
Learning: In TensorRT-LLM Python bindings, tensor parameter collections like mla_tensor_params and spec_decoding_tensor_params are kept as required parameters without defaults to maintain API consistency, even when it might affect backward compatibility.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-15T06:46:53.813Z
Learnt from: eopXD
PR: NVIDIA/TensorRT-LLM#6767
File: cpp/tensorrt_llm/batch_manager/kvCacheManager.cpp:0-0
Timestamp: 2025-08-15T06:46:53.813Z
Learning: In the TensorRT-LLM KV cache manager, SWA (Sliding Window Attention) combined with beam search is currently in a broken/non-functional state and is planned for future rework. During preparatory refactoring phases, code related to SWA+beam search may intentionally remain in a non-working state until the broader rework is completed.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-14T15:43:23.107Z
Learnt from: MatthiasKohl
PR: NVIDIA/TensorRT-LLM#6904
File: tensorrt_llm/_torch/attention_backend/trtllm.py:259-262
Timestamp: 2025-08-14T15:43:23.107Z
Learning: In TensorRT-LLM's attention backend, tensor parameters in the plan() method are assigned directly without validation (dtype, device, contiguity checks). This maintains consistency across all tensor inputs and follows the pattern of trusting callers to provide correctly formatted tensors.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-09-16T09:30:09.716Z
Learnt from: tongyuantongyu
PR: NVIDIA/TensorRT-LLM#7763
File: cpp/tensorrt_llm/CMakeLists.txt:297-301
Timestamp: 2025-09-16T09:30:09.716Z
Learning: In the TensorRT-LLM project, NCCL libraries are loaded earlier by PyTorch libraries or the bindings library, so the main shared library doesn't need NCCL paths in its RPATH - the libraries will already be available in the process address space when needed.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-20T07:43:36.447Z
Learnt from: ChristinaZ
PR: NVIDIA/TensorRT-LLM#7068
File: cpp/tensorrt_llm/kernels/moeTopKFuncs.cuh:169-172
Timestamp: 2025-08-20T07:43:36.447Z
Learning: In TensorRT-LLM MOE kernels, when processing up to 128 experts across 32 threads, each thread handles at most 4 experts (N < 5 constraint), where N represents candidates per thread rather than total system capacity.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-26T09:37:10.463Z
Learnt from: jiaganc
PR: NVIDIA/TensorRT-LLM#7031
File: tensorrt_llm/bench/dataclasses/configuration.py:90-104
Timestamp: 2025-08-26T09:37:10.463Z
Learning: In TensorRT-LLM, the `get_pytorch_perf_config()` method returns `self.pytorch_config` which can contain default `cuda_graph_config` values, so `llm_args` may already have this config before the extra options processing.
Applied to files:
docs/source/developer-guide/api-change.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (5)
docs/source/developer-guide/api-change.md (5)
1-3: Intro, naming, and scope look good.“TensorRT LLM” usage and the LLM‑API focus are clear.
41-48: “Pydantic dataclass” likely inaccurate; soften wording.If this is a BaseModel (common), call it “Pydantic model” and avoid asserting aliasing details unless guaranteed.
-The LLM class accepts numerous configuration parameters ... managed through a Pydantic dataclass called `LlmArgs`. +The LLM class accepts numerous configuration parameters ... managed through a Pydantic model named `LlmArgs`. -`LlmArgs` is an alias for `TorchLlmArgs` (defined in `tensorrt_llm/llmapi/llm_args.py`) +Args models live in `tensorrt_llm/llmapi/llm_args.py` (e.g., `TorchLlmArgs`).Would you like me to scan the repo and confirm the exact class/type? I can provide a read‑only script.
56-64: Pydantic Field doesn’t support status=; move status to schema or json_schema_extra.Keep status in YAML (preferred) or use Annotated/json_schema_extra.
-garbage_collection_gen0_threshold: int = Field( +garbage_collection_gen0_threshold: int = Field( default=20000, description=( "Threshold for Python garbage collection of generation 0 objects. " "Lower values trigger more frequent garbage collection." ), - status="beta" # Required for non-committed arguments )Optionally embed metadata:
-from pydantic import Field +from typing import Annotated +from pydantic import Field -garbage_collection_gen0_threshold: int = Field( +garbage_collection_gen0_threshold: Annotated[int, Field( default=20000, description=( "Threshold for Python garbage collection of generation 0 objects. " "Lower values trigger more frequent garbage collection." ), -)] + json_schema_extra={"status": "beta"} +)]
104-106: Public methods are not auto‑exposed; require decorator/registration.Update to reflect @set_api_status/ApiStatusRegistry usage.
-- Public methods (not starting with `_`) are automatically exposed as APIs +- Methods are part of the public API only when decorated with `@set_api_status(...)` + or explicitly registered via `ApiStatusRegistry` (see llmapi/utils.py).
193-203: Paths verified — doc commands are correct. tests/unittest/api_stability/ and tests/unittest/api_stability/test_llm_api.py exist; the documented pytest commands will not 404. Also: similarly named integration tests exist at tests/integration/defs/accuracy/test_llm_api.py.
👎 Promotion blocked, new vulnerability foundVulnerability report
|
Superjomn
force-pushed
the
doc.add-developer-guide-api
branch
from
60976ae to
5b10ec7
Compare
|
/bot run |
👎 Promotion blocked, new vulnerability foundVulnerability report
|
syuoni
approved these changes
Sep 22, 2025
Superjomn
force-pushed
the
doc.add-developer-guide-api
branch
from
5b10ec7 to
7f3083d
Compare
chzblych
force-pushed
the
release/1.0
branch
from
d0519b4 to
af3ea37
Compare
|
/bot run --stage-list "A10-Build_Docs" |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/source/developer-guide/api-change.md (3)
25-31: Use @set_api_status/ApiStatusRegistry instead of “status in docstring”.Status is tracked via the decorator/registry and YAML schemas, not by parsing docstrings. Replace this bullet to direct contributors to use @set_api_status("prototype" | "beta" | "deprecated") and the schema files.
- - Marked with a `status` field in the docstring: - - `prototype` - Early experimental stage - - `beta` - More stable but still subject to change - - `deprecated` - Scheduled for removal + - Marked via `@set_api_status("<status>")` and reflected in the YAML schema: + - `prototype` - Early experimental stage + - `beta` - More stable but still subject to change + - `deprecated` - Scheduled for removal
34-38: Clarify “Automatically validated” = API stability tests in CI.Spell out that validation runs in CI via tests/unittest/api_stability to avoid ambiguity.
-- Automatically validated to ensure consistency +- Automatically validated in CI by tests under `tests/unittest/api_stability/`
70-71: Align “Status” guidance with decorator/YAML, not Field().Clarify that status is captured by @set_api_status and/or YAML, not via a Field kwarg.
-- **Status**: Required for non-committed arguments (`prototype`, `beta`, etc.) +- **Status**: Track via `@set_api_status("<status>")` for the owning API and mirror in the YAML schema (`references/*.yaml`).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/source/developer-guide/api-change.md(1 hunks)docs/source/index.rst(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/source/index.rst
🧰 Additional context used
🧠 Learnings (14)
📚 Learning: 2025-07-28T17:06:08.621Z
Learnt from: moraxu
PR: NVIDIA/TensorRT-LLM#6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-09-09T09:40:45.658Z
Learnt from: fredricz-20070104
PR: NVIDIA/TensorRT-LLM#7645
File: tests/integration/test_lists/qa/llm_function_core.txt:648-648
Timestamp: 2025-09-09T09:40:45.658Z
Learning: In TensorRT-LLM test lists, it's common and intentional for the same test to appear in multiple test list files when they serve different purposes (e.g., llm_function_core.txt for comprehensive core functionality testing and llm_function_core_sanity.txt for quick sanity checks). This duplication allows tests to be run in different testing contexts.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-01T15:14:45.673Z
Learnt from: yibinl-nvidia
PR: NVIDIA/TensorRT-LLM#6506
File: examples/models/core/mixtral/requirements.txt:3-3
Timestamp: 2025-08-01T15:14:45.673Z
Learning: In TensorRT-LLM, examples directory can have different dependency versions than the root requirements.txt file. Version conflicts between root and examples dependencies are acceptable because examples are designed to be standalone and self-contained.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-21T21:48:35.135Z
Learnt from: djns99
PR: NVIDIA/TensorRT-LLM#7104
File: cpp/tensorrt_llm/cutlass_extensions/include/cutlass_extensions/epilogue/fusion/sm90_visitor_scatter.hpp:399-417
Timestamp: 2025-08-21T21:48:35.135Z
Learning: CUTLASS extensions in TensorRT-LLM (located under cpp/tensorrt_llm/cutlass_extensions/) are designed to integrate with and extend functionality in the external CUTLASS repository. When analyzing these extensions, their consumers and functionality wiring may exist in the CUTLASS codebase rather than within TensorRT-LLM itself.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-11T20:09:24.389Z
Learnt from: achartier
PR: NVIDIA/TensorRT-LLM#6763
File: tests/integration/defs/triton_server/conftest.py:16-22
Timestamp: 2025-08-11T20:09:24.389Z
Learning: In the TensorRT-LLM test infrastructure, the team prefers simple, direct solutions (like hard-coding directory traversal counts) over more complex but robust approaches when dealing with stable directory structures. They accept the maintenance cost of updating tests if the layout changes.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-21T00:16:56.457Z
Learnt from: farshadghodsian
PR: NVIDIA/TensorRT-LLM#7101
File: docs/source/blogs/tech_blog/blog9_Deploying_GPT_OSS_on_TRTLLM.md:36-36
Timestamp: 2025-08-21T00:16:56.457Z
Learning: TensorRT-LLM container release tags in documentation should only reference published NGC container images. The README badge version may be ahead of the actual published container versions.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-06T13:58:07.506Z
Learnt from: galagam
PR: NVIDIA/TensorRT-LLM#6487
File: tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_trtllm_bench.py:1-12
Timestamp: 2025-08-06T13:58:07.506Z
Learning: In TensorRT-LLM, test files (files under tests/ directories) do not require NVIDIA copyright headers, unlike production source code files. Test files typically start directly with imports, docstrings, or code.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-21T02:39:12.009Z
Learnt from: djns99
PR: NVIDIA/TensorRT-LLM#7104
File: cpp/tensorrt_llm/kernels/cutlass_kernels/moe_gemm/moe_kernels.cu:1475-1480
Timestamp: 2025-08-21T02:39:12.009Z
Learning: The min latency mode functionality in TensorRT-LLM MOE kernels (cpp/tensorrt_llm/kernels/cutlass_kernels/moe_gemm/moe_kernels.cu) is deprecated and no longer being maintained/updated, as confirmed by djns99. Bug reports and optimization suggestions for the computeStridesTmaWarpSpecializedLowLatencyKernel and related min latency code paths should be deprioritized.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-14T15:38:01.771Z
Learnt from: MatthiasKohl
PR: NVIDIA/TensorRT-LLM#6904
File: cpp/tensorrt_llm/pybind/thop/bindings.cpp:55-57
Timestamp: 2025-08-14T15:38:01.771Z
Learning: In TensorRT-LLM Python bindings, tensor parameter collections like mla_tensor_params and spec_decoding_tensor_params are kept as required parameters without defaults to maintain API consistency, even when it might affect backward compatibility.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-15T06:46:53.813Z
Learnt from: eopXD
PR: NVIDIA/TensorRT-LLM#6767
File: cpp/tensorrt_llm/batch_manager/kvCacheManager.cpp:0-0
Timestamp: 2025-08-15T06:46:53.813Z
Learning: In the TensorRT-LLM KV cache manager, SWA (Sliding Window Attention) combined with beam search is currently in a broken/non-functional state and is planned for future rework. During preparatory refactoring phases, code related to SWA+beam search may intentionally remain in a non-working state until the broader rework is completed.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-14T15:43:23.107Z
Learnt from: MatthiasKohl
PR: NVIDIA/TensorRT-LLM#6904
File: tensorrt_llm/_torch/attention_backend/trtllm.py:259-262
Timestamp: 2025-08-14T15:43:23.107Z
Learning: In TensorRT-LLM's attention backend, tensor parameters in the plan() method are assigned directly without validation (dtype, device, contiguity checks). This maintains consistency across all tensor inputs and follows the pattern of trusting callers to provide correctly formatted tensors.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-09-16T09:30:09.716Z
Learnt from: tongyuantongyu
PR: NVIDIA/TensorRT-LLM#7763
File: cpp/tensorrt_llm/CMakeLists.txt:297-301
Timestamp: 2025-09-16T09:30:09.716Z
Learning: In the TensorRT-LLM project, NCCL libraries are loaded earlier by PyTorch libraries or the bindings library, so the main shared library doesn't need NCCL paths in its RPATH - the libraries will already be available in the process address space when needed.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-20T07:43:36.447Z
Learnt from: ChristinaZ
PR: NVIDIA/TensorRT-LLM#7068
File: cpp/tensorrt_llm/kernels/moeTopKFuncs.cuh:169-172
Timestamp: 2025-08-20T07:43:36.447Z
Learning: In TensorRT-LLM MOE kernels, when processing up to 128 experts across 32 threads, each thread handles at most 4 experts (N < 5 constraint), where N represents candidates per thread rather than total system capacity.
Applied to files:
docs/source/developer-guide/api-change.md
📚 Learning: 2025-08-26T09:37:10.463Z
Learnt from: jiaganc
PR: NVIDIA/TensorRT-LLM#7031
File: tensorrt_llm/bench/dataclasses/configuration.py:90-104
Timestamp: 2025-08-26T09:37:10.463Z
Learning: In TensorRT-LLM, the `get_pytorch_perf_config()` method returns `self.pytorch_config` which can contain default `cuda_graph_config` values, so `llm_args` may already have this config before the extra options processing.
Applied to files:
docs/source/developer-guide/api-change.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (8)
docs/source/developer-guide/api-change.md (8)
41-41: “Pydantic dataclass” phrasing might be inaccurate.If LlmArgs/TorchLlmArgs inherit from BaseModel (common in TRT‑LLM), call it a Pydantic model, not a dataclass.
-The LLM class accepts numerous configuration parameters ... managed through a Pydantic dataclass called `LlmArgs`. +The LLM class accepts numerous configuration parameters ... managed through a Pydantic model called `LlmArgs`.
56-64: Pydantic Field(status="beta") is invalid; use json_schema_extra or YAML-only.This will not parse in Pydantic. Prefer keeping status only in YAML; if you must embed, use Annotated/Field json_schema_extra.
Option A (preferred: keep status only in YAML):
-garbage_collection_gen0_threshold: int = Field( +garbage_collection_gen0_threshold: int = Field( default=20000, description=( "Threshold for Python garbage collection of generation 0 objects. " "Lower values trigger more frequent garbage collection." ), - status="beta" # Required for non-committed arguments )Option B (if embedding status in code is required):
-from pydantic import Field +from typing import Annotated +from pydantic import Field -garbage_collection_gen0_threshold: int = Field( +garbage_collection_gen0_threshold: Annotated[int, Field( default=20000, description=( "Threshold for Python garbage collection of generation 0 objects. " "Lower values trigger more frequent garbage collection." ), - status="beta" # Required for non-committed arguments -) + json_schema_extra={"status": "beta"} +)]
104-106: Public methods are NOT auto-exposed; explicit registration/decorator required.Update to state that API exposure requires @set_api_status or explicit registration with ApiStatusRegistry; tests enforce this.
-- Public methods (not starting with `_`) are automatically exposed as APIs +- Methods are exposed as public API only if decorated with `@set_api_status(...)` + or explicitly registered via `ApiStatusRegistry`. Undecorated/unstated methods + are not considered part of the public API surface.#!/bin/bash # Evidence gathering: decorator/registry usage across codebase rg -n 'set_api_status\(|ApiStatusRegistry' -C2 tensorrt_llm rg -n 'api_stability' -C1 tests/unittest
135-142: Committed APIs likely still require explicit registration; don’t advise “no decorator”.Either require
@set_api_status("committed")or note the allowlist/registry path; confirm the project’s rule and adjust text.-For committed APIs, no decorator is needed: +For committed APIs, register them explicitly (e.g., `@set_api_status("committed")` +or by adding them to the allowlist/registry used by API stability tests):
160-169: Same note for committed schema snippet: verify kwargs representation.Ensure the committed schema example mirrors the real format (omit kwargs if not explicitly modeled).
209-213: Promotion flow: don’t remove registration if it’s required.If APIs must remain registered, change the decorator to “committed” instead of removing it; otherwise, document the allowlist alternative.
-1. Remove the `@set_api_status("beta")` decorator from the method +1. Change `@set_api_status("beta")` to `@set_api_status("committed")` (or add to the committed allowlist/registry, per project policy)
46-47: Alias verified — no docs change needed.
LlmArgs is an alias of TorchLlmArgs in tensorrt_llm/llmapi/llm_args.py (alias at L2435; TorchLlmArgs class at L1980).
148-158: Confirm whether 'kwargs' is allowed in llm.yaml; update docs if not.Could not locate tests/unittest/api_stability/references/**/llm.yaml in the repo to verify conventions — confirm whether the schema permits a named
kwargs: dictparameter; if it does not, replace the example with explicit parameters matching the committed llm.yaml.
Location: docs/source/developer-guide/api-change.md (lines 148–158)
👎 Promotion blocked, new vulnerability foundVulnerability report
|
Signed-off-by: Yan Chunwei <[email protected]> fix comments Signed-off-by: Yan Chunwei <[email protected]>
Superjomn
force-pushed
the
doc.add-developer-guide-api
branch
from
7f3083d to
c941db3
Compare
|
/bot run --stage-list "A10-Build_Docs" |
|
PR_Github #19477 [ run ] triggered by Bot |
|
PR_Github #19477 [ run ] completed with state |
|
/bot skip --comment "only change the doc" |
|
PR_Github #19491 [ skip ] triggered by Bot |
|
PR_Github #19491 [ skip ] completed with state |
litaotju
approved these changes
Sep 22, 2025
dominicshanshan
pushed a commit
to dominicshanshan/TensorRT-LLM
that referenced
this pull request
Sep 23, 2025
Signed-off-by: Yan Chunwei <[email protected]>
dominicshanshan
pushed a commit
to dominicshanshan/TensorRT-LLM
that referenced
this pull request
Sep 23, 2025
Signed-off-by: Yan Chunwei <[email protected]> Signed-off-by: Wangshanshan <[email protected]>
dominicshanshan
pushed a commit
to dominicshanshan/TensorRT-LLM
that referenced
this pull request
Sep 23, 2025
Signed-off-by: Yan Chunwei <[email protected]> Signed-off-by: Wangshanshan <[email protected]>
yuanjingx87
pushed a commit
that referenced
this pull request
Sep 23, 2025
Signed-off-by: Yan Chunwei <[email protected]>
dominicshanshan
pushed a commit
to dominicshanshan/TensorRT-LLM
that referenced
this pull request
Sep 24, 2025
Signed-off-by: Yan Chunwei <[email protected]> Signed-off-by: Wangshanshan <[email protected]>
dominicshanshan
pushed a commit
to dominicshanshan/TensorRT-LLM
that referenced
this pull request
Sep 24, 2025
Signed-off-by: Yan Chunwei <[email protected]> Signed-off-by: Wangshanshan <[email protected]>
dominicshanshan
pushed a commit
to dominicshanshan/TensorRT-LLM
that referenced
this pull request
Sep 24, 2025
Signed-off-by: Yan Chunwei <[email protected]> Signed-off-by: Wangshanshan <[email protected]>
dominicshanshan
pushed a commit
to dominicshanshan/TensorRT-LLM
that referenced
this pull request
Sep 24, 2025
Signed-off-by: Yan Chunwei <[email protected]> Signed-off-by: Wangshanshan <[email protected]>
dominicshanshan
pushed a commit
to dominicshanshan/TensorRT-LLM
that referenced
this pull request
Sep 24, 2025
Signed-off-by: Yan Chunwei <[email protected]> Signed-off-by: Wangshanshan <[email protected]>
dominicshanshan
pushed a commit
to dominicshanshan/TensorRT-LLM
that referenced
this pull request
Sep 24, 2025
Signed-off-by: Yan Chunwei <[email protected]> Signed-off-by: Wangshanshan <[email protected]>
dominicshanshan
pushed a commit
to dominicshanshan/TensorRT-LLM
that referenced
this pull request
Sep 24, 2025
Signed-off-by: Yan Chunwei <[email protected]> Signed-off-by: Wangshanshan <[email protected]>
dominicshanshan
pushed a commit
to dominicshanshan/TensorRT-LLM
that referenced
this pull request
Sep 25, 2025
Signed-off-by: Yan Chunwei <[email protected]> Signed-off-by: Wangshanshan <[email protected]>
chzblych
pushed a commit
that referenced
this pull request
Sep 25, 2025
Signed-off-by: Yan Chunwei <[email protected]> Signed-off-by: Wangshanshan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Description
Test Coverage
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)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
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
/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...Provide a user friendly way for developers to interact with a Jenkins server.
Run
/bot [-h|--help]to print this help message.See details below for each supported subcommand.
run [--reuse-test (optional)pipeline-id --disable-fail-fast --skip-test --stage-list "A10-PyTorch-1, xxx" --gpu-type "A30, H100_PCIe" --test-backend "pytorch, cpp" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" --detailed-log --debug(experimental)]Launch build/test pipelines. All previously running jobs will be killed.
--reuse-test (optional)pipeline-id(OPTIONAL) : Allow the new pipeline to reuse build artifacts and skip successful test stages from a specified pipeline or the last pipeline if no pipeline-id is indicated. If the Git commit ID has changed, this option will be always ignored. The DEFAULT behavior of the bot is to reuse build artifacts and successful test results from the last pipeline.--disable-reuse-test(OPTIONAL) : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.--disable-fail-fast(OPTIONAL) : Disable fail fast on build/tests/infra failures.--skip-test(OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.--stage-list "A10-PyTorch-1, xxx"(OPTIONAL) : Only run the specified test stages. Examples: "A10-PyTorch-1, xxx". Note: Does NOT update GitHub check status.--gpu-type "A30, H100_PCIe"(OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.--test-backend "pytorch, cpp"(OPTIONAL) : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: "pytorch, cpp" (does not run test stages with tensorrt or triton backend). Note: Does NOT update GitHub pipeline status.--only-multi-gpu-test(OPTIONAL) : Only run the multi-GPU tests. Note: Does NOT update GitHub check status.--disable-multi-gpu-test(OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.--add-multi-gpu-test(OPTIONAL) : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline.--post-merge(OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx"(OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx".--detailed-log(OPTIONAL) : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.--debug(OPTIONAL) : Experimental feature. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in thestage-listparameter to access the appropriate container environment. Note: Does NOT update GitHub check status.For guidance on mapping tests to stage names, see
docs/source/reference/ci-overview.mdand the
scripts/test_to_stage_mapping.pyhelper.kill
killKill all running builds associated with pull request.
skip
skip --comment COMMENTSkip testing for latest commit on pull request.
--comment "Reason for skipping build/test"is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.reuse-pipeline
reuse-pipelineReuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.