Skip to content

[https://nvbugs/6478715][fix] Renamed the worker's forward() to _forward_impl() so the base…#16730

Open
chenfeiz0326 wants to merge 1 commit into
NVIDIA:mainfrom
chenfeiz0326:repair-bot-bug6478715
Open

[https://nvbugs/6478715][fix] Renamed the worker's forward() to _forward_impl() so the base…#16730
chenfeiz0326 wants to merge 1 commit into
NVIDIA:mainfrom
chenfeiz0326:repair-bot-bug6478715

Conversation

@chenfeiz0326

@chenfeiz0326 chenfeiz0326 commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Root cause: MTPEagleDynamicTreeWorker overrode SpecWorkerBase.forward, which the init_subclass guard (nvbugs/6442074) forbids, raising TypeError at import time and collecting 0 tests.
  • Fix: Renamed the worker's forward() to _forward_impl() so the base SpecWorkerBase.forward wrapper drives it and retains its attn-metadata cleanup guarantee; no behavioral change. Staged ONLY the source file to avoid the LFS-binary pollution from prior rejected commits.
  • Automated fix generated by repair-bot

Test plan

  • Verify fix on the same GPU type as the original failure
  • Check for regressions in related tests

Links

Dev Engineer Review

  • Renamed MTPEagleDynamicTreeWorker.forward() to _forward_impl() so SpecWorkerBase can retain control of the guarded forward() wrapper and perform attention-metadata cleanup.
  • No behavioral, configuration, or test-list changes were made.

QA Engineer Review

No test changes.

…rward_impl

MTPEagleDynamicTreeWorker overrode SpecWorkerBase.forward, which the
__init_subclass__ guard (nvbugs/6442074) forbids, raising TypeError at
import time and breaking test collection. Rename to _forward_impl so the
base SpecWorkerBase.forward wrapper drives it and retains its
attn-metadata cleanup guarantee.

Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 22, 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: b75e806b-c90d-482c-8076-230e6fa76978

📥 Commits

Reviewing files that changed from the base of the PR and between 9cd6a49 and ac73231.

📒 Files selected for processing (1)
  • tensorrt_llm/_torch/speculative/mtp_dynamic_tree.py

Walkthrough

The MTP dynamic tree worker’s top-level method is renamed from forward to _forward_impl; its parameters and implementation remain unchanged.

Changes

MTP dynamic tree worker

Layer / File(s) Summary
Rename worker entry point
tensorrt_llm/_torch/speculative/mtp_dynamic_tree.py
Renames MTPEagleDynamicTreeWorker.forward(...) to _forward_impl(...) without changing the method signature or body.

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

Possibly related PRs

Suggested reviewers: lori-ren

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description has Summary/Test plan/Links, but it omits the required Description and PR Checklist sections from the template. Add a Description section explaining the issue and fix, plus a completed PR Checklist; keep the Test Coverage section aligned with the template.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the change and includes the required ticket/type prefix.
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.
✨ 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