Skip to content

Respect AutoDeploy trust_remote_code#12751

Open
jmecom wants to merge 1 commit intoNVIDIA:mainfrom
jmecom:codex/fix-autodeploy-trust-remote-code
Open

Respect AutoDeploy trust_remote_code#12751
jmecom wants to merge 1 commit intoNVIDIA:mainfrom
jmecom:codex/fix-autodeploy-trust-remote-code

Conversation

@jmecom
Copy link
Copy Markdown

@jmecom jmecom commented Apr 3, 2026

Summary

  • thread trust_remote_code from AutoDeploy LlmArgs into the model factory
  • use that flag for tokenizer init, AutoConfig.from_pretrained, from_config, and from_pretrained instead of forcing True
  • add regression tests covering the false case and ensuring the factory-level flag stays authoritative

Why

AutoDeploy accepted trust_remote_code=False at the public API, but the Hugging Face model factory ignored that choice and hardcoded trust_remote_code=True in several load paths. That meant an operator could explicitly disable remote code execution and still end up loading model-defined Python during AutoDeploy initialization.

This change makes AutoDeploy respect the callers trust boundary consistently across config, tokenizer, and model loading.

Summary by CodeRabbit

  • Refactor

    • Improved parameter propagation throughout the model factory pipeline to ensure configuration settings are consistently applied across tokenizer initialization, model configuration, and model loading stages.
  • Tests

    • Added unit tests validating configuration parameter handling and propagation through model factory operations.

@jmecom jmecom changed the title [codex] Respect AutoDeploy trust_remote_code Respect AutoDeploy trust_remote_code Apr 3, 2026
Signed-off-by: Jordan Mecom <jm@squareup.com>
@jmecom jmecom force-pushed the codex/fix-autodeploy-trust-remote-code branch from 7097602 to f31b179 Compare April 3, 2026 21:58
@jmecom jmecom marked this pull request as ready for review April 3, 2026 22:01
@jmecom jmecom requested a review from a team as a code owner April 3, 2026 22:01
@jmecom jmecom requested a review from nzmora-nvidia April 3, 2026 22:01
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 3, 2026

📝 Walkthrough

Walkthrough

The changes propagate a trust_remote_code configuration parameter through the model factory pipeline. Previously hardcoded as True in HuggingFace operations, it is now configurable via LlmArgs and consistently threaded through tokenizer initialization, config loading, and model instantiation. Comprehensive tests validate the propagation.

Changes

Cohort / File(s) Summary
Configuration Propagation
tensorrt_llm/_torch/auto_deploy/llm_args.py, tensorrt_llm/_torch/auto_deploy/models/factory.py, tensorrt_llm/_torch/auto_deploy/models/hf.py
Introduced trust_remote_code as a configurable parameter through the factory pipeline. Replaced hardcoded trust_remote_code=True defaults in HuggingFace operations (tokenizer creation, config loading, model instantiation) with self.trust_remote_code values.
Test Coverage
tests/unittest/auto_deploy/singlegpu/models/test_hf.py
Added 73 lines of unit tests validating trust_remote_code propagation from LlmArgs.create_factory() through AutoModelForCausalLMFactory operations, including tokenizer initialization, config loading, and model instantiation paths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description clearly explains what was changed (threading trust_remote_code through the factory), why it matters (security: respecting trust boundaries), and includes test coverage details. However, it does not follow the required template format with ticket/type prefix, Description, Test Coverage, and PR Checklist sections. Update the description to follow the repository template: add [ticket/issue][type] prefix to title, organize content under Description and Test Coverage sections, and include the PR Checklist confirmation.
Docstring Coverage ⚠️ Warning Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Respect AutoDeploy trust_remote_code' is clear, concise, and directly summarizes the main change—threading the trust_remote_code flag through AutoDeploy's model factory to ensure it respects caller-specified trust boundaries.

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

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

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tensorrt_llm/_torch/auto_deploy/llm_args.py (1)

354-369: ⚠️ Potential issue | 🟠 Major

Forward trust_remote_code to internal factories in EagleOneModelFactory.

The issue is confirmed: EagleOneModelFactory accepts trust_remote_code (via **kwargs passed to the parent class) but does not forward it to the internal AutoModelForCausalLMFactory (line 295) and EagleDrafterFactory (line 305). Both factories support this parameter, but will default to False, silently ignoring the user's setting when model_factory="eagle_one_model" is specified.

Pass trust_remote_code=self.trust_remote_code to both factory instantiations:

self.target_factory = AutoModelForCausalLMFactory(
    ...
    trust_remote_code=self.trust_remote_code,
)

self.draft_factory = EagleDrafterFactory(
    ...
    trust_remote_code=self.trust_remote_code,
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tensorrt_llm/_torch/auto_deploy/llm_args.py` around lines 354 - 369,
EagleOneModelFactory is not forwarding the trust_remote_code flag to its
internal factories, causing AutoModelForCausalLMFactory and EagleDrafterFactory
to default to False; update the EagleOneModelFactory constructor where
self.target_factory and self.draft_factory are created (references:
EagleOneModelFactory, AutoModelForCausalLMFactory, EagleDrafterFactory) to pass
trust_remote_code=self.trust_remote_code into both factory instantiations so the
user's trust_remote_code setting is honored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@tensorrt_llm/_torch/auto_deploy/llm_args.py`:
- Around line 354-369: EagleOneModelFactory is not forwarding the
trust_remote_code flag to its internal factories, causing
AutoModelForCausalLMFactory and EagleDrafterFactory to default to False; update
the EagleOneModelFactory constructor where self.target_factory and
self.draft_factory are created (references: EagleOneModelFactory,
AutoModelForCausalLMFactory, EagleDrafterFactory) to pass
trust_remote_code=self.trust_remote_code into both factory instantiations so the
user's trust_remote_code setting is honored.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b42f1866-1069-4092-9dfe-5e07293498de

📥 Commits

Reviewing files that changed from the base of the PR and between 7ee9e8b and f31b179.

📒 Files selected for processing (4)
  • tensorrt_llm/_torch/auto_deploy/llm_args.py
  • tensorrt_llm/_torch/auto_deploy/models/factory.py
  • tensorrt_llm/_torch/auto_deploy/models/hf.py
  • tests/unittest/auto_deploy/singlegpu/models/test_hf.py

@svc-trtllm-gh-bot svc-trtllm-gh-bot added the Community want to contribute PRs initiated from Community label Apr 3, 2026
@lucaslie lucaslie requested review from lucaslie and removed request for nzmora-nvidia April 7, 2026 21:10
@lucaslie
Copy link
Copy Markdown
Member

lucaslie commented Apr 7, 2026

/bot run

model_kwargs=self.model_kwargs,
tokenizer=None if self.tokenizer is None else str(self.tokenizer),
tokenizer_kwargs=self.tokenizer_kwargs,
trust_remote_code=self.trust_remote_code,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Our current default behavior is True for remote code trust. This will change the default to False. I suggest we overwrite the default of the LlmArgs from False to true for the AutoDeploy LlmArgs in particular.

Otherwise, you would have to go through all the failures and existing deployments and make sure to add trust_remote_code is True.

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #42186 [ run ] triggered by Bot. Commit: f31b179 Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #42186 [ run ] completed with state FAILURE. Commit: f31b179
/LLM/main/L0_MergeRequest_PR pipeline #33011 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

Link to invocation

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

Labels

Community want to contribute PRs initiated from Community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants