Skip to content

[https://nvbugs/6442074][fix] Unbreak main: DSparkWorker forward rename + test stub _forward_impl#16579

Merged
chzblych merged 2 commits into
NVIDIA:mainfrom
dongfengy:fix-main-specworker-guard
Jul 20, 2026
Merged

[https://nvbugs/6442074][fix] Unbreak main: DSparkWorker forward rename + test stub _forward_impl#16579
chzblych merged 2 commits into
NVIDIA:mainfrom
dongfengy:fix-main-specworker-guard

Conversation

@dongfengy

@dongfengy dongfengy commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Main is broken since d973974 (the SpecWorkerBase.__init_subclass__ guard from #16382) crossed in flight with two later additions:

  • dspark.py defines forward → import-time TypeError for import tensorrt_llm (via speculative/utils.py module-level import)
  • test_rejection_buffers_guard.py's _Worker stub lacks _forward_implTypeError at instantiation (3 tests)

This PR carries both fixes. The dspark rename is cherry-picked from #16577 — full credit @brnguyen2. Swept current main: these are the only two violations. Neither fix can pass CI alone (each PR's CI merge still contains the other breakage), hence one combined PR.

Test plan

  • Sweep of current main for all SpecWorkerBase subclasses / forward overrides — only these two sites
  • Local H100 validation: import tensorrt_llm + dspark import chain OK; test_rejection_buffers_guard.py 16/16; test_force_accepted_tokens.py 22/22

Links

brnguyen2 and others added 2 commits July 19, 2026 21:59
…d_impl

d973974 added __init_subclass__ to SpecWorkerBase requiring subclasses
to implement _forward_impl instead of overriding forward directly, but did
not update DSparkWorker. Rename the method to satisfy the contract.

Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
…tub a _forward_impl

d973974 made SpecWorkerBase._forward_impl abstract; the _Worker stub
in test_rejection_buffers_guard.py (added after that PR branched) only
stubs max_draft_len, so instantiating it now raises TypeError. Same fix
as _StubSpecWorker in test_force_accepted_tokens.py.

Swept current main for other gaps: dspark.py (fixed by this PR) and
this stub are the only two SpecWorkerBase violations.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Dongfeng Yu <dongfengy@nvidia.com>
@dongfengy
dongfengy requested a review from a team as a code owner July 19, 2026 22:09
@dongfengy

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Renames DSparkWorker.forward(...) to _forward_impl(...) without changing its method body, and adds _forward_impl(...) to the rejection-buffer guard test worker stub.

Changes

DSpark dispatch alignment

Layer / File(s) Summary
Worker implementation entry point
tensorrt_llm/_torch/speculative/dspark.py, tests/unittest/_torch/speculative/test_rejection_buffers_guard.py
DSparkWorker now defines _forward_impl(...); the test worker stub implements the same method and raises NotImplementedError.

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

Possibly related PRs

Suggested reviewers: junyixu-nv

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 matches the PR scope and follows the required ticket/type pattern.
Description check ✅ Passed The description explains the bug, fix, and testing, though it uses custom headings and omits the checklist checkbox.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #60199 [ run ] triggered by Bot. Commit: 0dbe52f Link to invocation

@dongfengy

Copy link
Copy Markdown
Collaborator Author

Local validation on H100 (viking-prod-235) at this PR's head 0dbe52f5, full source build:

  1. Import chain restored: import tensorrt_llm + from tensorrt_llm._torch.speculative.utils import get_spec_worker + from tensorrt_llm._torch.speculative.dspark import DSparkWorker — all OK (this is the path that currently throws TypeError on main).
  2. The 3 CI-failing tests fixed: tests/unittest/_torch/speculative/test_rejection_buffers_guard.py16/16 passed.
  3. Regression: test_force_accepted_tokens.py22/22 passed.

Given main is import-broken for everyone, appreciate a fast review — @brnguyen2's dspark rename rides in here with authorship preserved.

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #60199 [ run ] completed with state FAILURE. Commit: 0dbe52f
/LLM/main/L0_MergeRequest_PR pipeline #48569 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

@chzblych

Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #60218 [ run ] triggered by Bot. Commit: 0dbe52f Link to invocation

@chzblych

Copy link
Copy Markdown
Collaborator

/bot skip --comment "Skip the rerun pipeline and merge the PR to unblock main. The only failing test is perf/test_perf_sanity.py::test_e2e[disagg_upload-gen_only-b200_deepseek-r1-fp4_8k1k_con1536_ctx1_dep4_gen1_dep8_eplb0_mtp1_ccb-NIXL], which is a known failure and is waived in #16582"

@zhaoyangwang-nvidia zhaoyangwang-nvidia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please get this PR merged ASAP

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #60220 [ skip ] triggered by Bot. Commit: 0dbe52f Link to invocation

@chzblych
chzblych enabled auto-merge (squash) July 20, 2026 02:36
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #60218 [ run ] completed with state ABORTED. Commit: 0dbe52f

Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #60220 [ skip ] completed with state SUCCESS. Commit: 0dbe52f
Skipping testing for commit 0dbe52f

Link to invocation

@chzblych
chzblych merged commit fa54a19 into NVIDIA:main Jul 20, 2026
17 of 19 checks passed
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.

6 participants