Skip to content

[None][docs] Remove ghost --topr from RocketKV example command - #18576

Open
imitater-dou wants to merge 1 commit into
NVIDIA:mainfrom
imitater-dou:docs/fix-rocketkv-remove-ghost-topr
Open

[None][docs] Remove ghost --topr from RocketKV example command#18576
imitater-dou wants to merge 1 commit into
NVIDIA:mainfrom
imitater-dou:docs/fix-rocketkv-remove-ghost-topr

Conversation

@imitater-dou

@imitater-dou imitater-dou commented Sep 2, 2026

Copy link
Copy Markdown

Summary

The RocketKV example command in examples/sparse_attention/RocketKV.md documents --topr 128, but examples/llm-api/llm_sparse_attention.py only exposes --topk (not --topr). Copy-pasting the documented command fails with an unrecognized-arguments error.

RocketSparseAttentionConfig.topr remains a valid LLM API field (default 128); the Python API snippet and Configuration Arguments section are unchanged. The example script relies on that API default when constructing RocketSparseAttentionConfig.

Repro (on main @ d7d79c3)

# Documented command includes --topr:
rg -n -- '--topr' examples/sparse_attention/RocketKV.md
# -> Example Command block lists --topr 128

# Script argparse has --topk only:
rg -n "add_argument\\('--topk'|add_argument\\('--topr'" examples/llm-api/llm_sparse_attention.py
# -> only --topk; no --topr

# API still has topr default 128:
rg -n 'topr:' tensorrt_llm/llmapi/llm_args.py
# -> RocketSparseAttentionConfig.topr default=128

Change

  • Drop --topr 128 from the Example Command in RocketKV.md.

Test plan

  • Confirm Example Command flags match llm_sparse_attention.py argparse
  • Confirm API example / Configuration Arguments still document topr

Dev Engineer Review

  • Removed the unsupported --topr 128 flag from the RocketKV example command.
  • The command now matches the script’s exposed --topk option.
  • The Python API example and configuration documentation remain unchanged.
  • No test changes.

QA Engineer Review

No test changes.

examples/llm-api/llm_sparse_attention.py exposes --topk but not --topr;
RocketSparseAttentionConfig.topr still defaults to 128 in the API.

Signed-off-by: ADou <ikun3.1415927@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 8fdba5db-0966-4fe5-98fc-a719d015c261

📥 Commits

Reviewing files that changed from the base of the PR and between d7d79c3 and 234a287.

📒 Files selected for processing (1)
  • examples/sparse_attention/RocketKV.md
💤 Files with no reviewable changes (1)
  • examples/sparse_attention/RocketKV.md

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


Walkthrough

The RocketKV example command removes the --topr 128 option. All other command-line options remain unchanged.

Changes

RocketKV command update

Layer / File(s) Summary
Update RocketKV example command
examples/sparse_attention/RocketKV.md
The example command no longer passes --topr 128.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 234a2

This removes an invalid command-line flag from the RocketKV example without changing runtime or API behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the documentation change: removing the unsupported --topr option from the RocketKV example command.
Description check ✅ Passed The description explains the issue, affected files, root cause, exact change, and test plan. It is sufficiently complete for this documentation-only change, although it does not reproduce the full rep…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
Full details: Description check

Explanation

The description explains the issue, affected files, root cause, exact change, and test plan. It is sufficiently complete for this documentation-only change, although it does not reproduce the full repository checklist.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant