Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[None][fix] Properly raise errors from multimodal loading#12331

Merged
2ez4bz merged 1 commit intoNVIDIA:mainNVIDIA/TensorRT-LLM:mainfrom
2ez4bz:dev-mm-load-raise2ez4bz/TensorRT-LLM:dev-mm-load-raiseCopy head branch name to clipboard
Mar 19, 2026
Merged

[None][fix] Properly raise errors from multimodal loading#12331
2ez4bz merged 1 commit intoNVIDIA:mainNVIDIA/TensorRT-LLM:mainfrom
2ez4bz:dev-mm-load-raise2ez4bz/TensorRT-LLM:dev-mm-load-raiseCopy head branch name to clipboard

Conversation

@2ez4bz
Copy link
Copy Markdown
Collaborator

@2ez4bz 2ez4bz commented Mar 18, 2026 •
edited by coderabbitai Bot
Loading

Summary by CodeRabbit

  • Refactor
    • Streamlined asynchronous data loading and error handling patterns for improved code efficiency and maintainability.

Description

  • Why?

Previously, errors from loading multimodal data (e.g. retrieving an image from the web) would be suppressed, leading to confusing error messages.

  • What?

This commit fixes that.

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

  • Update tava architecture diagram if there is a significant design change in PR.

  • 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

To see a list of available CI bot commands, please comment /bot help.

@2ez4bz 2ez4bz requested a review from a team as a code owner March 18, 2026 22:26
@2ez4bz 2ez4bz requested a review from hchings March 18, 2026 22:26
@2ez4bz
Copy link
Copy Markdown
Collaborator Author

2ez4bz commented Mar 18, 2026

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

tensorrt-cicd commented Mar 18, 2026

PR_Github #39508 [ run ] triggered by Bot. Commit: 8e14811 Link to invocation

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8243a806-1cf0-48c8-97d9-eb72a3e41f1d

📥 Commits

Reviewing files that changed from the base of the PR and between 3e0ae7f and 8e14811.

📒 Files selected for processing (1)
  • tensorrt_llm/serve/chat_utils.py
📝 Walkthrough

Walkthrough

The change refactors async data handling in chat utilities by removing internal async wrapper functions and instead returning coroutines directly from async loaders for image, video, and audio processing. Error handling via try/except blocks is removed, delegating responsibility to underlying async operations or higher-level orchestration.

Changes

Cohort / File(s) Summary
Async Wrapper Refactoring
tensorrt_llm/serve/chat_utils.py		 Replaced per-part inline async wrappers (load_image_async, decode_image_embeds_async, load_video_async, load_audio_async) with direct calls to underlying async loaders/decoders. Now returns coroutines instead of awaited results, removes broad try/except error handling, and defers data retrieval to existing async orchestration pathway.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description explains the issue (errors being suppressed) and solution (properly raising errors), but the Test Coverage section is completely empty without even a placeholder explanation. Fill in the Test Coverage section with specific test cases or files that validate the error handling changes in multimodal loading.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: removing error suppression from multimodal loading to properly raise errors, which directly aligns with the code modifications.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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

Tip

CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.

OpenGrep is compatible with Semgrep configurations. Add an opengrep.yml or semgrep.yml configuration file to your project to enable OpenGrep analysis.

@2ez4bz
[None][fix] Properly raise errors from multimodal loading
5c4a7e9 
* Why?

Previously, errors from loading multimodal data (e.g. retrieving an
image from the web) would be suppressed, leading to confusing error
messages.

* What?

This commit fixes that.

Signed-off-by: William Zhang <133824995+2ez4bz@users.noreply.github.com>
@2ez4bz 2ez4bz force-pushed the dev-mm-load-raise branch from 8e14811 to 5c4a7e9 Compare March 18, 2026 22:50
@2ez4bz
Copy link
Copy Markdown
Collaborator Author

2ez4bz commented Mar 18, 2026

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

tensorrt-cicd commented Mar 18, 2026

PR_Github #39513 [ run ] triggered by Bot. Commit: 5c4a7e9 Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

tensorrt-cicd commented Mar 19, 2026

PR_Github #39513 [ run ] completed with state SUCCESS. Commit: 5c4a7e9
/LLM/main/L0_MergeRequest_PR pipeline #30734 completed with status: 'SUCCESS'

CI Report

Link to invocation

@2ez4bz 2ez4bz merged commit ed0f14e into NVIDIA:main Mar 19, 2026
5 checks passed
longcheng-nv pushed a commit to longcheng-nv/TensorRT-LLM that referenced this pull request Mar 31, 2026
@2ez4bz @longcheng-nv
[None][fix] Properly raise errors from multimodal loading (NVIDIA#12331)
862519f 
* Why?

Previously, errors from loading multimodal data (e.g. retrieving an
image from the web) would be suppressed, leading to confusing error
messages.

* What?

This commit fixes that.

Signed-off-by: William Zhang <133824995+2ez4bz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pcastonguay pcastonguay pcastonguay approved these changes

@hchings hchings Awaiting requested review from hchings hchings is a code owner automatically assigned from NVIDIA/trt-llm-llmapi-devs

Assignees

@2ez4bz 2ez4bz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@2ez4bz @tensorrt-cicd @pcastonguay
Morty Proxy This is a proxified and sanitized view of the page, visit original site.