Skip to content

[None][doc] Add Claude skill for multimodal model onboarding - #13842

Merged
yechank-nvidia merged 3 commits into
NVIDIA:mainfrom
yechank-nvidia:yechank/skill-trtllm-model-onboard-multimodal
May 21, 2026
Merged

[None][doc] Add Claude skill for multimodal model onboarding#13842
yechank-nvidia merged 3 commits into
NVIDIA:mainfrom
yechank-nvidia:yechank/skill-trtllm-model-onboard-multimodal

Conversation

@yechank-nvidia

@yechank-nvidia yechank-nvidia commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Adds .claude/skills/trtllm-model-onboard-multimodal/SKILL.md, an agent skill guiding contributors through onboarding HuggingFace multimodal models to the TensorRT-LLM PyTorch backend (tensorrt_llm/_torch/models/). Covers the aggregated and EPD-disaggregated request lifecycles, module reuse rules, three performance contracts (sync-free forward, async server-side preprocessing, shared-tensor broadcast), an onboarding phase walk-through keyed off TestModelingMultimodal, and a pre-PR checklist.

Summary by CodeRabbit

  • Documentation
    • Added comprehensive onboarding documentation for integrating HuggingFace multimodal models into TensorRT-LLM. Includes system architecture, performance requirements, implementation phases, and pre-release verification guidelines.

@yechank-nvidia yechank-nvidia self-assigned this May 7, 2026
@yechank-nvidia
yechank-nvidia marked this pull request as ready for review May 7, 2026 08:21
@yechank-nvidia
yechank-nvidia requested a review from 2ez4bz May 7, 2026 08:21
@coderabbitai

coderabbitai Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR introduces comprehensive onboarding documentation for integrating HuggingFace multimodal models (vision, audio, video + text) into the TensorRT-LLM PyTorch backend. The skill document details system architecture, design contracts, phased implementation steps, and pre-PR verification procedures.

Changes

Multimodal Model Onboarding Guide

Layer / File(s) Summary
Metadata and Overview
.claude/skills/trtllm-model-onboard-multimodal/SKILL.md
YAML front-matter defines skill name, multimodal onboarding description, and documentation metadata.
System Architecture
.claude/skills/trtllm-model-onboard-multimodal/SKILL.md
Documents aggregated execution pipeline from OpenAI server through async media loading, preprocessing, handle-based broadcast, per-iteration GPU staging, and multimodal embedding fusion; includes optional EPD-disaggregated path with encoder-only worker outputs.
Design Requirements
.claude/skills/trtllm-model-onboard-multimodal/SKILL.md
Lists template models by modality, enforces reuse-before-write rule for TRT-LLM module classes (linear, embedding, attention, MLP, RoPE), defines three performance contracts (zero sync in forward, CPU-side async preprocessing, handle-based shared-tensor transport).
Implementation Phases
.claude/skills/trtllm-model-onboard-multimodal/SKILL.md
Provides step-by-step guidance: wrapper skeleton showing module composition and mRoPE integration, input processor CPU-side mrope config and mm token id rewriting, HF weight mapper prefix surgery, and test structure via multimodal modeling base harness.
Verification and Checklist
.claude/skills/trtllm-model-onboard-multimodal/SKILL.md
Details documentation updates (support matrix), serve verification for both aggregated and EPD flows, and comprehensive pre-PR checklist covering decorator ordering, forward signature, GPU device paths, module reuse prohibitions, performance audit, and tests/docs confirmation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description explains what was added and its purpose, but lacks some required template sections like Test Coverage and a completed PR Checklist. Add Test Coverage section explaining how this documentation change is verified, and complete the PR Checklist with all applicable items marked for review before merging.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding a Claude skill documentation file for multimodal model onboarding guidance.
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.

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

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

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/skills/trtllm-model-onboard-multimodal/SKILL.md:
- Line 31: The fenced code block that begins with "[1] API event loop (async)   
tensorrt_llm/serve/openai_server.py" is missing a language identifier, which
triggers markdownlint MD040; update the opening fence to include a language
(e.g., add "text" so it becomes ```text) to silence the linter and preserve
formatting for the block containing the lines like "logits → sampling
(PyExecutor) → tokens". Ensure only the fence line is changed and the block
content remains identical.
🪄 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: Enterprise

Run ID: d8c480c6-992d-4092-ab00-6425199a2ccb

📥 Commits

Reviewing files that changed from the base of the PR and between cbfb02a and e6b8d40.

📒 Files selected for processing (1)
  • .claude/skills/trtllm-model-onboard-multimodal/SKILL.md

Comment thread .claude/skills/trtllm-model-onboard-multimodal/SKILL.md Outdated
Comment thread .claude/skills/trtllm-model-onboard-multimodal/SKILL.md Outdated
Comment thread .claude/skills/trtllm-model-onboard-multimodal/SKILL.md Outdated
Comment thread .claude/skills/trtllm-model-onboard-multimodal/SKILL.md Outdated
Comment thread .claude/skills/trtllm-model-onboard-multimodal/SKILL.md Outdated
Comment thread .claude/skills/trtllm-model-onboard-multimodal/SKILL.md Outdated
Comment thread .claude/skills/trtllm-model-onboard-multimodal/SKILL.md
Comment thread .claude/skills/trtllm-model-onboard-multimodal/SKILL.md
Comment thread .claude/skills/trtllm-model-onboard-multimodal/SKILL.md Outdated
Comment thread .claude/skills/trtllm-model-onboard-multimodal/SKILL.md
Comment thread .claude/skills/trtllm-model-onboard-multimodal/SKILL.md
yechank-nvidia added a commit to yechank-nvidia/TensorRT-LLM that referenced this pull request May 8, 2026
Iterates on the trtllm-model-onboard-multimodal SKILL.md per reviewer
feedback on PR NVIDIA#13842:

- Tighten frontmatter description to a trigger-only summary; move
  implementation detail into the body.
- "vision tower" -> "multimodal encoder(s)" throughout where the term
  is modality-agnostic (audio / video as well).
- Trim system-map diagrams: drop file paths and private-method names
  that belong to debugging/perf rather than onboarding; restore the
  contributor-facing public APIs (find_input_mm_embeds,
  prepare_mrope_config, get_multimodal_embeddings cache hits, mrope
  CPU staging) and add explicit Key invariants.
- Clarify "blocking" vs to_thread wording so it no longer contradicts
  the diagram's async/threaded staging.
- Define what a shared-tensor "handle" actually is (small dict +
  IPC/shm key) instead of the jargon "handle-shaped".
- Mark the get_text_with_mm_placeholders / expand_prompt_token_ids_for_mm
  fast path as optional pending the planned refactor.
- Add Phase 4 host-memory guidance from PR NVIDIA#13283 (lazy encoder
  construction outside MetaInitMode + weights.mark_consumed for
  incremental mmap shard release).
- Drop the duplicated compose-from rule under the dedicated Reuse
  section; expand the encoder-length / placeholder-count invariant
  with the chunked-prefill cache failure mode.
- Expand "Be parsimonious" guidance for TestModelingMultimodal
  scenarios so CI runtime is bounded.
- Add quickstart_multimodal.py as the first line of defense before
  trtllm-serve, plus the chunked-prefill log grep verification step.
- Clarify AutoModelForCausalLM is TRT-LLM's, not transformers'.
- Compute modules table: drop entries that are obvious or stale-prone,
  keep only non-obvious wiring (Linear TP/quant kwargs, Attention
  dual use for LLM/encoder, MLP/GatedMLP fused gate/up, mRoPE vs
  RotaryEmbedding split, MoE pointer to MOE_DEVELOPER_GUIDE.md).
- "Port to TRT-LLM modules" is required (not preferred); spell out
  the two reasons (perf + transformers version coupling) and ban HF
  module/computation imports outright with one Qwen2-VL exception.
- Add Contract 4: batch the multimodal encoder across requests in
  one forward (concat pixel_values + per-image seq_lens; nsys audit
  shows one wide block, not N narrow ones).
- Replace Phase 2 model-wrapper template with the LlavaNext /
  Gemma3VL pattern (single class, super().__init__, text_config-based
  AutoModel.from_config, post_config, vocab_size_padded /
  infer_max_seq_len). Note Base/non-Base split (Qwen2VL/Qwen3VL) as
  a multi-variant implementation detail, not the default.

Net change: +212/-160. File grows from 386 to 438 lines as new
content (Contract 4, host-memory guidance, default Phase 2 template,
quickstart verification, two-reason port argument) outweighs trims.

Signed-off-by: yechank <161688079+yechank-nvidia@users.noreply.github.com>
@yechank-nvidia yechank-nvidia added the Multimodal Label for issues & PRs regarding Multimodal related objects label May 12, 2026
Adds .claude/skills/trtllm-model-onboard-multimodal/SKILL.md, an agent skill
guiding contributors through onboarding HuggingFace multimodal models to the
TensorRT-LLM PyTorch backend (tensorrt_llm/_torch/models/). Covers the
aggregated and EPD-disaggregated request lifecycles, module reuse rules,
three performance contracts (sync-free forward, async server-side
preprocessing, shared-tensor broadcast), an onboarding phase walk-through
keyed off TestModelingMultimodal, and a pre-PR checklist.

Signed-off-by: yechank <161688079+yechank-nvidia@users.noreply.github.com>
Iterates on the trtllm-model-onboard-multimodal SKILL.md per reviewer
feedback on PR NVIDIA#13842:

- Tighten frontmatter description to a trigger-only summary; move
  implementation detail into the body.
- "vision tower" -> "multimodal encoder(s)" throughout where the term
  is modality-agnostic (audio / video as well).
- Trim system-map diagrams: drop file paths and private-method names
  that belong to debugging/perf rather than onboarding; restore the
  contributor-facing public APIs (find_input_mm_embeds,
  prepare_mrope_config, get_multimodal_embeddings cache hits, mrope
  CPU staging) and add explicit Key invariants.
- Clarify "blocking" vs to_thread wording so it no longer contradicts
  the diagram's async/threaded staging.
- Define what a shared-tensor "handle" actually is (small dict +
  IPC/shm key) instead of the jargon "handle-shaped".
- Mark the get_text_with_mm_placeholders / expand_prompt_token_ids_for_mm
  fast path as optional pending the planned refactor.
- Add Phase 4 host-memory guidance from PR NVIDIA#13283 (lazy encoder
  construction outside MetaInitMode + weights.mark_consumed for
  incremental mmap shard release).
- Drop the duplicated compose-from rule under the dedicated Reuse
  section; expand the encoder-length / placeholder-count invariant
  with the chunked-prefill cache failure mode.
- Expand "Be parsimonious" guidance for TestModelingMultimodal
  scenarios so CI runtime is bounded.
- Add quickstart_multimodal.py as the first line of defense before
  trtllm-serve, plus the chunked-prefill log grep verification step.
- Clarify AutoModelForCausalLM is TRT-LLM's, not transformers'.
- Compute modules table: drop entries that are obvious or stale-prone,
  keep only non-obvious wiring (Linear TP/quant kwargs, Attention
  dual use for LLM/encoder, MLP/GatedMLP fused gate/up, mRoPE vs
  RotaryEmbedding split, MoE pointer to MOE_DEVELOPER_GUIDE.md).
- "Port to TRT-LLM modules" is required (not preferred); spell out
  the two reasons (perf + transformers version coupling) and ban HF
  module/computation imports outright with one Qwen2-VL exception.
- Add Contract 4: batch the multimodal encoder across requests in
  one forward (concat pixel_values + per-image seq_lens; nsys audit
  shows one wide block, not N narrow ones).
- Replace Phase 2 model-wrapper template with the LlavaNext /
  Gemma3VL pattern (single class, super().__init__, text_config-based
  AutoModel.from_config, post_config, vocab_size_padded /
  infer_max_seq_len). Note Base/non-Base split (Qwen2VL/Qwen3VL) as
  a multi-variant implementation detail, not the default.

Net change: +212/-160. File grows from 386 to 438 lines as new
content (Contract 4, host-memory guidance, default Phase 2 template,
quickstart verification, two-reason port argument) outweighs trims.

Signed-off-by: yechank <161688079+yechank-nvidia@users.noreply.github.com>
@yechank-nvidia
yechank-nvidia force-pushed the yechank/skill-trtllm-model-onboard-multimodal branch from fcf479b to 6c1da15 Compare May 20, 2026 06:36
Signed-off-by: Michal Guzek <mguzek@nvidia.com>
@yechank-nvidia

Copy link
Copy Markdown
Collaborator Author

/bot skip --comment "Claude skill update for TRT-LLM multimodal"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #49543 [ skip ] triggered by Bot. Commit: 0173d5b Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #49543 [ skip ] completed with state SUCCESS. Commit: 0173d5b
Skipping testing for commit 0173d5b

Link to invocation

@yechank-nvidia
yechank-nvidia merged commit 67b0654 into NVIDIA:main May 21, 2026
7 checks passed
xxi-nv pushed a commit to xxi-nv/TensorRT-LLM that referenced this pull request May 22, 2026
…13842)

Signed-off-by: yechank <161688079+yechank-nvidia@users.noreply.github.com>
Signed-off-by: Michal Guzek <mguzek@nvidia.com>
Co-authored-by: Michal Guzek <mguzek@nvidia.com>
bmarimuthu-nv pushed a commit to nv-auto-deploy/TensorRT-LLM that referenced this pull request May 28, 2026
…13842)

Signed-off-by: yechank <161688079+yechank-nvidia@users.noreply.github.com>
Signed-off-by: Michal Guzek <mguzek@nvidia.com>
Co-authored-by: Michal Guzek <mguzek@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Multimodal Label for issues & PRs regarding Multimodal related objects

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants