Skip to content

Fix fail to run Nemo Skills when need trust remote code#1142

Closed
fzyzcjy wants to merge 1 commit intoNVIDIA-NeMo:mainfrom
fzyzcjy:patch-1
Closed

Fix fail to run Nemo Skills when need trust remote code#1142
fzyzcjy wants to merge 1 commit intoNVIDIA-NeMo:mainfrom
fzyzcjy:patch-1

Conversation

@fzyzcjy
Copy link
Contributor

@fzyzcjy fzyzcjy commented Dec 25, 2025

I think Nemo-Skills is run by users only internally w/ control, thus not make it configurable. Please ping me if this is needed.

Summary by CodeRabbit

  • Bug Fixes
    • Improved tokenizer initialization to enable remote code execution support when loading tokenizers from string identifiers.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: fzyzcjy <5236035+fzyzcjy@users.noreply.github.com>
@fzyzcjy fzyzcjy changed the title Fix tokenizer error when need trust remote code Fix fail to run Nemo Skills when need trust remote code Dec 25, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 25, 2025

📝 Walkthrough

Walkthrough

A tokenizer string parameter now loads with trust_remote_code=True enabled, allowing remote code execution during tokenizer initialization. This change applies only when the tokenizer is specified as a string path; other tokenizer loading paths remain unaffected.

Changes

Cohort / File(s) Summary
Tokenizer Loading Configuration
nemo_skills/prompt/utils.py
Added trust_remote_code=True parameter to AutoTokenizer.from_pretrained() call when tokenizer is provided as a string, enabling remote code execution for model-hosted tokenizers.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly relates to the main change: fixing an issue where Nemo Skills fails to run when the tokenizer requires remote code trust, which aligns with the implementation of trust_remote_code=True in the tokenizer loading logic.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
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.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e85e995 and 1e085ad.

📒 Files selected for processing (1)
  • nemo_skills/prompt/utils.py

@Kipok
Copy link
Collaborator

Kipok commented Dec 27, 2025

thanks @fzyzcjy! Re-created here #1146 from a branch to be able to run ci and also added this parameter in a few more places

@Kipok
Copy link
Collaborator

Kipok commented Dec 27, 2025

merged in #1146

@Kipok Kipok closed this Dec 27, 2025
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.

2 participants