Skip to content

Conversation

tgasser-nv
Copy link
Collaborator

@tgasser-nv tgasser-nv commented Sep 5, 2025

Description

Related Issue(s)

Checklist

  • I've read the CONTRIBUTING guidelines.
  • I've updated the documentation if applicable.
  • I've added tests if applicable.
  • @mentions of the person or team responsible for reviewing proposed changes.

Copy link
Contributor

github-actions bot commented Sep 5, 2025

Documentation preview

https://nvidia.github.io/NeMo-Guardrails/review/pr-1376

@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.62%. Comparing base (92fe37c) to head (6c13a32).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1376   +/-   ##
========================================
  Coverage    71.62%   71.62%           
========================================
  Files          171      171           
  Lines        17021    17021           
========================================
  Hits         12192    12192           
  Misses        4829     4829           
Flag Coverage Δ
python 71.62% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tgasser-nv tgasser-nv self-assigned this Sep 5, 2025
Copy link
Collaborator

@Pouyanpi Pouyanpi left a comment

Choose a reason for hiding this comment

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

Thank you Tim, it looks good 👍🏻

Regarding the reqs installation command:

pip install nemoguardrails

would it be better to use nemoguardrails >= 0.16.0 ?

something we must tell VDR to be aware of if they are going to re-run the notebooks as we have not released it yet.

Also something very minor:

in 2_tracing_with_jaeger.ipynb notebook I see in cell 10:

CONFIG_MODELS: Dict[str, str] but should be List[Dict[str, str]]

Copy link
Collaborator

@cparisien cparisien left a comment

Choose a reason for hiding this comment

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

This is a nice improvement, thanks Tim. I'm approving, but we should figure out what's causing the bad output from Llama 3.1 8B. Stop token maybe?

@tgasser-nv tgasser-nv merged commit 877785a into develop Sep 5, 2025
19 checks passed
@tgasser-nv tgasser-nv deleted the docs/tracing-notebook-vdr-feedback branch September 5, 2025 15:03
tgasser-nv added a commit that referenced this pull request Sep 22, 2025
* Updated notebook 1 with VDR feedback

* Updated notebook 2 (Jaeger) with VDR comments

* Replace Llama 3.1 8B with Llama 4 scout as main model

* Correct type in cell 10 notebook 2
tgasser-nv added a commit that referenced this pull request Sep 22, 2025
* Updated notebook 1 with VDR feedback

* Updated notebook 2 (Jaeger) with VDR comments

* Replace Llama 3.1 8B with Llama 4 scout as main model

* Correct type in cell 10 notebook 2
Pouyanpi pushed a commit that referenced this pull request Oct 1, 2025
* Updated notebook 1 with VDR feedback

* Updated notebook 2 (Jaeger) with VDR comments

* Replace Llama 3.1 8B with Llama 4 scout as main model

* Correct type in cell 10 notebook 2
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.

4 participants