Skip to content

Conversation

@Kipok
Copy link
Collaborator

@Kipok Kipok commented Jan 27, 2026

Summary by CodeRabbit

  • Chores
    • Updated package dependencies: constrained huggingface_hub to version <1 and disabled optional packages to improve compatibility.

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

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 27, 2026

Important Files Changed

Filename Overview
nemo_skills/inference/eval/bfcl.py Modified BFCL_REQUIREMENTS list to disable sentence-transformers and pin huggingface_hub version

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

📝 Walkthrough

Walkthrough

Updated the BFCL_REQUIREMENTS constant in nemo_skills/inference/eval/bfcl.py by pinning "huggingface_hub" to versions <1, removing "qwen-agent" and "sentence-transformers>=2.7.0" from active requirements (now commented), and adding inline explanatory comments.

Changes

Cohort / File(s) Summary
Dependency constraints update
nemo_skills/inference/eval/bfcl.py
Modified BFCL_REQUIREMENTS: replace "huggingface_hub" with "huggingface_hub<1"; comment out "qwen-agent" and "sentence-transformers>=2.7.0"; added inline comments. Review for any code paths relying on the removed packages.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: modifying BFCL_REQUIREMENTS to speed up installation by pinning huggingface_hub to a lower version and removing/disabling optional dependencies.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

@gwarmstrong
Copy link
Collaborator

Looks okay in principle, but let's run it with the integration tests?

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@Kipok
Copy link
Collaborator Author

Kipok commented Jan 29, 2026

I also ran slurm tests (rebasing on the #1192 to fix bfcl) and both gpu and slurm tests work fine

@gwarmstrong gwarmstrong enabled auto-merge (squash) January 29, 2026 16:15
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@gwarmstrong gwarmstrong merged commit 48933b2 into main Jan 29, 2026
5 checks passed
@gwarmstrong gwarmstrong deleted the igitman/bfcl-req-fixes branch January 29, 2026 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants