-
Notifications
You must be signed in to change notification settings - Fork 416
docs: renumbering the getting started notebooks #1149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Bryan Bednarski <[email protected]>
Signed-off-by: Bryan Bednarski <[email protected]>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Warning Rate limit exceeded@bbednarski9 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 5 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughReordered and renamed example notebooks and README listings; updated tests to reference new notebook names and expanded workflows. Added a pyproject.toml rename helper and integrated it into the MCP setup install flow with added waits; updated multiple notebooks to use renamed example package directories and next-step links. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as tests/test_notebooks_e2e.py
participant NB as Notebook kernel
participant FS as Filesystem
participant TOML as tomli / tomllib
participant Writer as tomli_w
participant PIP as pip
Test->>NB: execute notebook (parameterized)
Note right of NB: cells run sequentially
NB->>FS: check destination for pyproject.toml
alt pyproject exists
NB->>TOML: parse pyproject.toml
TOML-->>NB: return project.name (or none)
NB->>NB: rename_package_in_pyproject(..., "_notebook")
NB->>Writer: write updated pyproject.toml
NB->>PIP: pip install renamed package
else no pyproject
NB->>PIP: pip install from path
end
Note right of NB: installation/serve steps include explicit time.sleep(10) waits
NB-->>Test: report execution status/result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
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. Comment |
There was a problem hiding this 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
🧹 Nitpick comments (2)
examples/README.md (1)
125-125: Replace “NAT Optimize” with compliant naming in README.Per docs rules, avoid “NAT” in documentation/README. Suggest: “NeMo Agent toolkit Optimize CLI (nat optimize)”.
Apply this diff:
-7. [Optimizing Model Selection, Parameters, and Prompts](notebooks/7_optimize_model_selection.ipynb) - Use NAT Optimize to compare models, parameters, and prompt variations +7. [Optimizing Model Selection, Parameters, and Prompts](notebooks/7_optimize_model_selection.ipynb) - Use the NeMo Agent toolkit Optimize CLI (nat optimize) to compare models, parameters, and prompt variationsAs per coding guidelines.
examples/notebooks/README.md (1)
30-30: Avoid “NAT” in README; use full product name.Replace “NAT Optimize” with “NeMo Agent toolkit Optimize CLI (nat optimize)”.
-6. [Observability, Evaluation, and Profiling](6_observability_evaluation_and_profiling.ipynb) - Instrumenting with observability, evaluation and profiling tools -7. [Optimizing Model Selection, Parameters, and Prompts](7_optimize_model_selection.ipynb) - Use NAT Optimize to compare models, parameters, and prompt variations +6. [Observability, Evaluation, and Profiling](6_observability_evaluation_and_profiling.ipynb) - Instrumenting with observability, evaluation and profiling tools +7. [Optimizing Model Selection, Parameters, and Prompts](7_optimize_model_selection.ipynb) - Use the NeMo Agent toolkit Optimize CLI (nat optimize) to compare models, parameters, and prompt variationsAs per coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
examples/README.md(1 hunks)examples/notebooks/2_bringing_your_own_agent.ipynb(1 hunks)examples/notebooks/3_adding_tools_to_agents.ipynb(1 hunks)examples/notebooks/4_mcp_setup_and_integration.ipynb(1 hunks)examples/notebooks/5_multi_agent_orchestration.ipynb(1 hunks)examples/notebooks/6_observability_evaluation_and_profiling.ipynb(2 hunks)examples/notebooks/7_optimize_model_selection.ipynb(0 hunks)examples/notebooks/README.md(1 hunks)examples/notebooks/tests/test_notebooks_e2e.py(4 hunks)
💤 Files with no reviewable changes (1)
- examples/notebooks/7_optimize_model_selection.ipynb
🧰 Additional context used
📓 Path-based instructions (9)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/notebooks/3_adding_tools_to_agents.ipynbexamples/README.mdexamples/notebooks/README.mdexamples/notebooks/2_bringing_your_own_agent.ipynbexamples/notebooks/tests/test_notebooks_e2e.pyexamples/notebooks/4_mcp_setup_and_integration.ipynbexamples/notebooks/6_observability_evaluation_and_profiling.ipynbexamples/notebooks/5_multi_agent_orchestration.ipynb
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/notebooks/3_adding_tools_to_agents.ipynbexamples/README.mdexamples/notebooks/README.mdexamples/notebooks/2_bringing_your_own_agent.ipynbexamples/notebooks/tests/test_notebooks_e2e.pyexamples/notebooks/4_mcp_setup_and_integration.ipynbexamples/notebooks/6_observability_evaluation_and_profiling.ipynbexamples/notebooks/5_multi_agent_orchestration.ipynb
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/notebooks/3_adding_tools_to_agents.ipynbexamples/README.mdexamples/notebooks/README.mdexamples/notebooks/2_bringing_your_own_agent.ipynbexamples/notebooks/tests/test_notebooks_e2e.pyexamples/notebooks/4_mcp_setup_and_integration.ipynbexamples/notebooks/6_observability_evaluation_and_profiling.ipynbexamples/notebooks/5_multi_agent_orchestration.ipynb
{docs/source/**/*.md,**/README.@(md|ipynb)}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{docs/source/**/*.md,**/README.@(md|ipynb)}: Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Never use “NAT”/“nat” abbreviations in documentation
Documentation must be clear/comprehensive; avoid TODOs/FIXMEs/placeholders; avoid offensive/outdated terms; ensure spelling is correct
Files:
examples/README.mdexamples/notebooks/README.md
examples/*/README.@(md|ipynb)
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Each example must include a README.md or README.ipynb
Files:
examples/notebooks/README.md
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/test_*.py,examples/*/tests/test_*.py,packages/*/tests/test_*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Name test files as test_*.py and store them in a tests/ folder
Files:
examples/notebooks/tests/test_notebooks_e2e.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Applied to files:
examples/README.mdexamples/notebooks/README.mdexamples/notebooks/2_bringing_your_own_agent.ipynbexamples/notebooks/4_mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Applied to files:
examples/notebooks/2_bringing_your_own_agent.ipynb
🔇 Additional comments (6)
examples/notebooks/3_adding_tools_to_agents.ipynb (2)
1237-1238: LGTM: Next step link updated correctly.Link to
4_mcp_setup_and_integration.ipynbmatches the new sequence.
1-13: Review comment requires verification due to conflicting guidance.The coding guidelines require SPDX headers for all
.ipynbfiles. However, retrieved learnings indicate that Python code in.ipynbfiles withinexamples/notebooks/(specifically code created via%%writefilecommands) is exempt from requiring Apache headers because these are demo examples.Unclear points:
- Whether this exception applies to the notebook file itself or only to generated Python code within it
- Whether other example notebooks contain SPDX header cells (couldn't verify existing patterns due to tool limitations)
- Whether this is truly an all-notebooks or notebook-specific requirement
Recommendation: Confirm with the repository maintainers whether the retrieved exception for example notebook files extends to notebook-wide header cells, or if only the generated code is exempt. If the notebook file itself requires headers, the review comment is correct; if exempt per the learning, the comment should be closed.
examples/notebooks/2_bringing_your_own_agent.ipynb (1)
858-861: LGTM: Next step link points to 3_adding_tools_to_agents.ipynb.Consistent with the new ordering.
examples/notebooks/4_mcp_setup_and_integration.ipynb (1)
1218-1221: LGTM: Next step chain updated to 5_multi_agent_orchestration.ipynb.Sequencing is consistent with README and other notebooks.
examples/notebooks/5_multi_agent_orchestration.ipynb (1)
1883-1886: LGTM: Next step link updated to 6_observability_evaluation_and_profiling.ipynb.Matches the renumbered flow.
examples/notebooks/tests/test_notebooks_e2e.py (1)
93-96: Directory cleanup handles notebook artifacts correctly.Good call adding the directory branch with
shutil.rmtree; this keeps the fixture resilient when notebooks leave behind folders likesimple_calculator/.
Signed-off-by: Bryan Bednarski <[email protected]>
There was a problem hiding this 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
📒 Files selected for processing (1)
examples/notebooks/6_observability_evaluation_and_profiling.ipynb(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/notebooks/6_observability_evaluation_and_profiling.ipynb
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/notebooks/6_observability_evaluation_and_profiling.ipynb
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/notebooks/6_observability_evaluation_and_profiling.ipynb
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
|
What if we removed the number prefix for the names so we don't need to do this in the future? We can still list an order in the READMEs |
Signed-off-by: Bryan Bednarski <[email protected]>
Signed-off-by: Bryan Bednarski <[email protected]>
There was a problem hiding this 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
🧹 Nitpick comments (2)
examples/notebooks/mcp_setup_and_integration.ipynb (2)
269-270: Consider validating uniqueness of renamed package.The function appends a fixed suffix but doesn't verify the resulting name is actually unique. If a package with the suffix already exists, this could still cause conflicts.
Consider adding a uniqueness check or using a timestamp/random suffix:
import time new_name = f"{original_name}{suffix}_{int(time.time())}"
510-519: Hardcoded sleep delays are fragile.Multiple 10-second
time.sleep(10)calls are scattered throughout for server startup. This approach is brittle—servers may take longer in some environments or be ready sooner in others.Consider implementing a retry loop that polls for server readiness:
import time import urllib.request def wait_for_server(url, max_wait=30, check_interval=2): """Poll until server responds or timeout.""" elapsed = 0 while elapsed < max_wait: try: urllib.request.urlopen(url, timeout=1) print(f"✓ Server ready at {url}") return True except: time.sleep(check_interval) elapsed += check_interval print(f"✗ Server not ready after {max_wait}s") return False # Usage: wait_for_server("http://localhost:9901/mcp")Also applies to: 587-596, 648-656, 977-985, 1098-1107, 1150-1158
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
examples/README.md(1 hunks)examples/notebooks/README.md(1 hunks)examples/notebooks/adding_tools_to_agents.ipynb(1 hunks)examples/notebooks/bringing_your_own_agent.ipynb(1 hunks)examples/notebooks/mcp_setup_and_integration.ipynb(13 hunks)examples/notebooks/multi_agent_orchestration.ipynb(1 hunks)examples/notebooks/observability_evaluation_and_profiling.ipynb(2 hunks)examples/notebooks/optimize_model_selection.ipynb(0 hunks)examples/notebooks/tests/test_notebooks_e2e.py(5 hunks)
💤 Files with no reviewable changes (1)
- examples/notebooks/optimize_model_selection.ipynb
✅ Files skipped from review due to trivial changes (2)
- examples/notebooks/observability_evaluation_and_profiling.ipynb
- examples/notebooks/bringing_your_own_agent.ipynb
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/notebooks/README.md
- examples/README.md
🧰 Additional context used
📓 Path-based instructions (7)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/notebooks/multi_agent_orchestration.ipynbexamples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/mcp_setup_and_integration.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/notebooks/multi_agent_orchestration.ipynbexamples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/mcp_setup_and_integration.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/notebooks/multi_agent_orchestration.ipynbexamples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/mcp_setup_and_integration.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/test_*.py,examples/*/tests/test_*.py,packages/*/tests/test_*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Name test files as test_*.py and store them in a tests/ folder
Files:
examples/notebooks/tests/test_notebooks_e2e.py
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to examples/*/pyproject.toml : Examples containing Python code should have a pyproject.toml
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {tests/test_*.py,examples/*/tests/test_*.py,packages/*/tests/test_*.py} : Name test files as test_*.py and store them in a tests/ folder
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {tests/**/*.py,examples/*/tests/**/*.py} : Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Applied to files:
examples/notebooks/adding_tools_to_agents.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to examples/*/pyproject.toml : Examples containing Python code should have a pyproject.toml
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to packages/*/pyproject.toml : Each package under packages/ must contain a pyproject.toml
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {**/pyproject.toml,uv.lock} : New dependencies must be added to both pyproject.toml (alphabetically) and uv.lock via ‘uv pip install <pkg> --sync’
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
🪛 Ruff (0.14.3)
examples/notebooks/tests/test_notebooks_e2e.py
123-123: subprocess call: check for execution of untrusted input
(S603)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI Pipeline / Check
🔇 Additional comments (7)
examples/notebooks/multi_agent_orchestration.ipynb (1)
1885-1885: LGTM - Clear forward reference to next notebook.The updated reference to
6_observability_evaluation_and_profiling.ipynbaligns well with the notebook renumbering described in the PR objectives.examples/notebooks/adding_tools_to_agents.ipynb (1)
1237-1237: LGTM - Clear progression to MCP notebook.The reference to
4_mcp_setup_and_integration.ipynbprovides good continuity in the notebook series.examples/notebooks/mcp_setup_and_integration.ipynb (2)
414-415: Non-editable install appears to be a workaround.The comment "remove editable mode so that nat workflow delete works" suggests this change addresses a symptom rather than the root cause. This may create maintainability issues if the underlying problem with editable installs isn't documented or tracked.
Is there a known issue with
nat workflow deleteand editable installs? If so, consider:
- Filing an issue to track the root cause
- Adding a more detailed comment explaining the limitation
- Documenting this behavior in user-facing docs
1325-1325: LGTM - Enhanced closing narrative.The updated conclusion provides better context and forward-looking guidance for the notebook series.
examples/notebooks/tests/test_notebooks_e2e.py (3)
18-18: LGTM - Environment variable propagation supports local install mode.The addition of
NAT_NOTEBOOK_INSTALL_MODE="local"and its propagation to the subprocess environment aligns well with the install flow changes inmcp_setup_and_integration.ipynb.Note: The ruff S603 warning about untrusted input is a false positive here—the environment variables are controlled by the test framework.
Also applies to: 118-123
136-146: LGTM - Test parametrization updated for renumbered notebooks.The test IDs and notebook filenames have been properly updated to reflect the removal of numeric prefixes from the notebook files, maintaining clear test coverage across the notebook series.
138-140: Package name verification confirmed — test expectation is correct.The
rename_package_in_pyprojectfunction defined in the notebook (Cell 17 ofmcp_setup_and_integration.ipynb) uses the default suffix"_notebook". Applied to the original package name"nat_simple_calculator", it produces"nat_simple_calculator_notebook", which matches the test expectation at line 138.
Signed-off-by: Bryan Bednarski <[email protected]>
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/notebooks/adding_tools_to_agents.ipynb (1)
1-12: Missing required SPDX Apache-2.0 header.Per the coding guidelines, all source files including
.ipynbfiles must include the SPDX Apache-2.0 header. For Jupyter notebooks, this should be added as the first markdown cell in the notebook with the standard header template and copyright year 2025.As per coding guidelines.
Add a markdown cell at the beginning of the notebook with content similar to:
<!-- SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. -->examples/notebooks/multi_agent_orchestration.ipynb (1)
1-10: Add SPDX Apache-2.0 license header.This notebook is missing the required SPDX Apache-2.0 header. Per the coding guidelines, all source files (including
.ipynbfiles) must start with the standard SPDX Apache-2.0 header template, and copyright years should be current when a file is changed.Add a markdown cell at the beginning of the notebook with the SPDX header (copy the format from other notebooks in the repository).
Based on coding guidelines.
♻️ Duplicate comments (1)
examples/notebooks/observability_evaluation_and_profiling.ipynb (1)
50-50: Fix capitalization inconsistency between TOC and section header.The table of contents entry at Line 50 uses "Next Steps" (title case), but the section header at Line 1891 uses "Next steps" (sentence case). For consistency with other section headers in the notebook (e.g., "Setup", "Creating a Tool-Calling Workflow"), use title case.
Apply this diff to make the header consistent:
- "# 6.0) Next steps\n", + "# 6.0) Next Steps\n",Also applies to: 1885-1894
🧹 Nitpick comments (2)
examples/notebooks/mcp_setup_and_integration.ipynb (2)
236-284: Well-implemented helper function with good error handling.The
rename_package_in_pyprojectfunction properly handles:
- Python version differences (tomllib vs tomli)
- Missing dependencies with graceful fallback
- File I/O errors
- Package name extraction and modification
Optional: Consider minor docstring formatting improvement for consistency:
def rename_package_in_pyproject(pyproject_path: Path, suffix: str = "_notebook") -> str: """ Rename the package in pyproject.toml to avoid conflicts with existing installations. - + Args: pyproject_path: Path to the pyproject.toml file suffix: Suffix to append to the package name - + Returns: The new package name, or None if renaming failed """
514-524: Consider replacing fixed delays with health checks.Multiple cells use
time.sleep(10)to wait for background servers to start. This approach:
- May be insufficient on slower systems (leading to flaky execution)
- Wastes time on faster systems
- Makes the notebook less robust
Consider replacing with a polling loop that checks server readiness:
import time import requests def wait_for_server(url: str, timeout: int = 30, poll_interval: float = 0.5): """Wait for server to become available.""" start_time = time.time() while time.time() - start_time < timeout: try: response = requests.get(url, timeout=1) if response.status_code == 200: print(f"✓ Server ready at {url}") return True except (requests.ConnectionError, requests.Timeout): pass time.sleep(poll_interval) raise TimeoutError(f"Server at {url} did not become ready within {timeout}s") # Usage wait_for_server("http://localhost:9901/debug/tools/list")However, if keeping fixed delays for notebook simplicity, the current implementation is acceptable.
Also applies to: 591-600, 652-661, 981-990, 1102-1111, 1154-1163
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/notebooks/adding_tools_to_agents.ipynb(1 hunks)examples/notebooks/bringing_your_own_agent.ipynb(1 hunks)examples/notebooks/getting_started_with_nat.ipynb(1 hunks)examples/notebooks/mcp_setup_and_integration.ipynb(15 hunks)examples/notebooks/multi_agent_orchestration.ipynb(1 hunks)examples/notebooks/observability_evaluation_and_profiling.ipynb(2 hunks)examples/notebooks/tests/test_notebooks_e2e.py(5 hunks)
✅ Files skipped from review due to trivial changes (2)
- examples/notebooks/getting_started_with_nat.ipynb
- examples/notebooks/bringing_your_own_agent.ipynb
🧰 Additional context used
📓 Path-based instructions (7)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/notebooks/multi_agent_orchestration.ipynbexamples/notebooks/mcp_setup_and_integration.ipynbexamples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/observability_evaluation_and_profiling.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/notebooks/multi_agent_orchestration.ipynbexamples/notebooks/mcp_setup_and_integration.ipynbexamples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/observability_evaluation_and_profiling.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/notebooks/multi_agent_orchestration.ipynbexamples/notebooks/mcp_setup_and_integration.ipynbexamples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/observability_evaluation_and_profiling.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/test_*.py,examples/*/tests/test_*.py,packages/*/tests/test_*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Name test files as test_*.py and store them in a tests/ folder
Files:
examples/notebooks/tests/test_notebooks_e2e.py
🧠 Learnings (5)
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {**/pyproject.toml,uv.lock} : New dependencies must be added to both pyproject.toml (alphabetically) and uv.lock via ‘uv pip install <pkg> --sync’
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to examples/*/pyproject.toml : Examples containing Python code should have a pyproject.toml
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to packages/*/pyproject.toml : Each package under packages/ must contain a pyproject.toml
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-08-22T20:18:28.041Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/cursor-rules.mdc:0-0
Timestamp: 2025-08-22T20:18:28.041Z
Learning: Applies to .cursor/rules/**/*.mdc : Use consistent project terminology in descriptions (e.g., NAT workflows, NAT CLI commands)
Applied to files:
examples/notebooks/observability_evaluation_and_profiling.ipynb
🧬 Code graph analysis (1)
examples/notebooks/tests/test_notebooks_e2e.py (1)
packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_base.py (1)
env(501-502)
🪛 Ruff (0.14.3)
examples/notebooks/tests/test_notebooks_e2e.py
123-123: subprocess call: check for execution of untrusted input
(S603)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI Pipeline / Check
🔇 Additional comments (8)
examples/notebooks/adding_tools_to_agents.ipynb (1)
1234-1238: LGTM - Clear and concise next steps.The updated text effectively guides users to the next notebook in the series with a clear description of what will be covered. The reference to MCP servers and clients provides good context for the progression of the tutorial.
examples/notebooks/multi_agent_orchestration.ipynb (1)
1882-1886: LGTM!The Next Steps section clearly references the specific next notebook and provides a concise description of what will be covered. The text is well-written and maintains consistency with the documentation style.
examples/notebooks/mcp_setup_and_integration.ipynb (3)
199-208: LGTM! Past review concern addressed.The installation now includes both
tomliandtomli-w, which correctly handles the package renaming functionality across different Python versions. Therename_package_in_pyprojectfunction usestomllibfor Python 3.11+ with a fallback totomlifor older versions.Based on learnings
306-308: LGTM! Good automation support.The environment variable
NAT_NOTEBOOK_INSTALL_MODEallows tests to bypass interactive prompts while maintaining user-friendly behavior for manual execution. This integrates well with the test suite changes intest_notebooks_e2e.py.
1329-1329: LGTM! Clear next steps guidance.The updated text provides good context for what comes next in the notebook series, explaining the transition from individual agent tools to multi-agent orchestration.
examples/notebooks/tests/test_notebooks_e2e.py (3)
18-31: LGTM! Workflow list updated correctly.The additions to
_ALL_WORKFLOWSalign with:
- New MCP notebook creating
mcp_dev_workflowandsimple_calculator_notebook(with_notebooksuffix to avoid conflicts)- Existing optimization notebook creating
tmp_workflowThe
import osis properly used at Line 118 for environment variable handling.
34-60: LGTM! File list properly updated.The comments reflect the notebook renumbering (removing numeric prefixes), and the file paths correctly map to outputs from various notebooks. The commented line 59 for
simple_calculator/is appropriate since workflow deletion handles that directory cleanup.
118-123: LGTM! Environment variable propagation correctly implemented.The code properly:
- Copies the current environment to preserve existing variables
- Sets
NAT_NOTEBOOK_INSTALL_MODE="develop"for automated testing- Passes the modified environment to the subprocess
This integrates cleanly with the notebook's
install_example_codefunction, which checks this variable before prompting for user input.
…ow cleanup Signed-off-by: Bryan Bednarski <[email protected]>
There was a problem hiding this 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
📒 Files selected for processing (1)
examples/notebooks/mcp_setup_and_integration.ipynb(18 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/notebooks/mcp_setup_and_integration.ipynb
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/notebooks/mcp_setup_and_integration.ipynb
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/notebooks/mcp_setup_and_integration.ipynb
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {**/pyproject.toml,uv.lock} : New dependencies must be added to both pyproject.toml (alphabetically) and uv.lock via ‘uv pip install <pkg> --sync’
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to examples/*/pyproject.toml : Examples containing Python code should have a pyproject.toml
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to packages/*/pyproject.toml : Each package under packages/ must contain a pyproject.toml
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI Pipeline / Check
Signed-off-by: Bryan Bednarski <[email protected]>
There was a problem hiding this 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
♻️ Duplicate comments (1)
examples/notebooks/tests/test_notebooks_e2e.py (1)
28-30: Package name inconsistency persists.Line 30 defines the workflow as
"simple_calculator_notebook", but line 138 expects"nat_simple_calculator_notebook"with the"nat_"prefix. Based on past review analysis, the notebook creates a package namedsimple_calculator_notebook(without the prefix).Update line 138 to match the workflow name in line 30:
- pytest.param("mcp_setup_and_integration.ipynb", ["nat_simple_calculator_notebook"], + pytest.param("mcp_setup_and_integration.ipynb", ["simple_calculator_notebook"],Also applies to: 138-140
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/notebooks/tests/test_notebooks_e2e.py(5 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/test_*.py,examples/*/tests/test_*.py,packages/*/tests/test_*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Name test files as test_*.py and store them in a tests/ folder
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/notebooks/tests/test_notebooks_e2e.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/notebooks/tests/test_notebooks_e2e.py
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/notebooks/tests/test_notebooks_e2e.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
🪛 Ruff (0.14.3)
examples/notebooks/tests/test_notebooks_e2e.py
123-123: subprocess call: check for execution of untrusted input
(S603)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI Pipeline / Check
🔇 Additional comments (2)
examples/notebooks/tests/test_notebooks_e2e.py (2)
59-59: Clarify commentedsimple_calculator/directory entry.The
simple_calculator/directory is commented out in_OTHER_FILES. If themcp_setup_and_integration.ipynbnotebook creates this directory, it should be cleaned up by the test. If it's intentionally excluded for safety reasons (to avoid deleting developer data), please add a comment explaining why.Additionally, update the comment reference from
4_mcp_setup_and_integrationtomcp_setup_and_integrationto match the new naming:- # "simple_calculator/", # Example code directory created by 4_mcp_setup_and_integration + # "simple_calculator/", # Example code directory created by mcp_setup_and_integration
118-123: Environment variable setup looks good.The addition of
NAT_NOTEBOOK_INSTALL_MODE="develop"and passing it via theenvparameter correctly addresses previous review feedback about making the install mode more flexible based on the test environment.Note: The static analysis warning (S603) about untrusted subprocess input is a false positive here, as
cmdis constructed entirely from controlled inputs and the notebook path.
Signed-off-by: Bryan Bednarski <[email protected]>
Signed-off-by: Bryan Bednarski <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
examples/notebooks/mcp_setup_and_integration.ipynb (1)
236-284: Fix return type annotation - function returnsOptional[str], notstr.The
rename_package_in_pyprojectfunction is annotated to returnstr(line 236), but it returnsNoneon multiple error paths (lines 256, 267, 284). This contradicts the docstring which states "Returns: The new package name, or None if renaming failed" and breaks type checking.This issue was previously flagged in past review comments but remains unresolved.
Apply this fix:
+from typing import Optional + -def rename_package_in_pyproject(pyproject_path: Path, suffix: str = "_notebook") -> str: +def rename_package_in_pyproject(pyproject_path: Path, suffix: str = "_notebook") -> Optional[str]:Note: The
Optionalimport should be added at the top of the function definition within the cell, not as a module-level import (since this is notebook cell code).
🧹 Nitpick comments (1)
examples/notebooks/tests/test_notebooks_e2e.py (1)
18-18: Remove unusedosimport.The
osmodule is imported on line 18 but doesn't appear to be used anywhere in the visible code. The commented-out code at lines 121-122 would use it, but those lines are currently disabled.-import osIf the commented code at lines 121-122 will be uncommented in the future, consider adding a comment explaining why the import is retained.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/notebooks/adding_tools_to_agents.ipynb(16 hunks)examples/notebooks/mcp_setup_and_integration.ipynb(18 hunks)examples/notebooks/multi_agent_orchestration.ipynb(18 hunks)examples/notebooks/observability_evaluation_and_profiling.ipynb(22 hunks)examples/notebooks/tests/test_notebooks_e2e.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/notebooks/observability_evaluation_and_profiling.ipynb
🧰 Additional context used
📓 Path-based instructions (7)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/multi_agent_orchestration.ipynbexamples/notebooks/mcp_setup_and_integration.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/multi_agent_orchestration.ipynbexamples/notebooks/mcp_setup_and_integration.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/multi_agent_orchestration.ipynbexamples/notebooks/mcp_setup_and_integration.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/test_*.py,examples/*/tests/test_*.py,packages/*/tests/test_*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Name test files as test_*.py and store them in a tests/ folder
Files:
examples/notebooks/tests/test_notebooks_e2e.py
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Applied to files:
examples/notebooks/adding_tools_to_agents.ipynb
📚 Learning: 2025-09-15T21:26:29.430Z
Learnt from: saglave
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 726
File: examples/frameworks/adk_demo/src/nat_adk_demo/weather_update_tool.py:0-0
Timestamp: 2025-09-15T21:26:29.430Z
Learning: In NAT's ADK integration, the docstring of registered functions serves as the tool description sent to the LLM, not standard Python function documentation. The docstring should describe the logical tool interface (parameters the LLM will provide) rather than the NAT framework wrapper parameters like tool_config and builder.
Applied to files:
examples/notebooks/adding_tools_to_agents.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {**/pyproject.toml,uv.lock} : New dependencies must be added to both pyproject.toml (alphabetically) and uv.lock via ‘uv pip install <pkg> --sync’
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to examples/*/pyproject.toml : Examples containing Python code should have a pyproject.toml
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to packages/*/pyproject.toml : Each package under packages/ must contain a pyproject.toml
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Applied to files:
examples/notebooks/mcp_setup_and_integration.ipynb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI Pipeline / Check
🔇 Additional comments (6)
examples/notebooks/multi_agent_orchestration.ipynb (1)
625-1885: LGTM! Systematic renaming to nb5 variant is consistent.The notebook has been systematically updated to use
retail_sales_agent_nb5instead ofretail_sales_agentthroughout all workflow creation, file paths, configuration references, and execution commands. The next steps reference has also been appropriately updated to point toobservability_evaluation_and_profiling.ipynb. This aligns with the broader PR's notebook reorganization strategy.examples/notebooks/mcp_setup_and_integration.ipynb (2)
306-311: Good defensive handling for notebook environments without stdin.The
StdinNotImplementedErrorexception handling with a default to 'local' is a good pattern for notebook environments where stdin is not available (e.g., during automated testing). This prevents failures in non-interactive contexts.
356-365: Verify new_name is used consistently in success messages.The code checks
if new_name:before printing the renamed package name (lines 362-365, 420-423). However, whennew_nameisNone, the code still prints a generic success message without the package name. Consider whether this is the intended behavior or if installation should be skipped when renaming fails.Should the installation proceed when
rename_package_in_pyprojectreturnsNone? If package renaming is critical for avoiding conflicts (as the function docstring suggests), consider either:
- Warning the user more prominently when renaming fails, or
- Skipping installation when renaming fails to prevent conflicts
Also applies to: 412-423
examples/notebooks/adding_tools_to_agents.ipynb (1)
615-1237: LGTM! Systematic renaming to nb3 variant is consistent.The notebook has been systematically updated to use
retail_sales_agent_nb3instead ofretail_sales_agentthroughout all workflow creation, file paths, configuration references, and execution commands. The next steps reference has also been appropriately updated to point tomcp_setup_and_integration.ipynb. This aligns with the broader PR's notebook reorganization and numbering scheme.examples/notebooks/tests/test_notebooks_e2e.py (2)
121-122: Clarify intent of commented environment variable code.The
NAT_NOTEBOOK_INSTALL_MODEenvironment variable setup is commented out without explanation. If this code is intended for future use, add a comment explaining why it's commented. If it's no longer needed, remove it entirely.# env = os.environ.copy() # env["NAT_NOTEBOOK_INSTALL_MODE"] = "local"Is this code planned for future use? If not, it should be removed for clarity.
28-33: LGTM! Test coverage properly expanded for renumbered notebooks.The test suite has been appropriately updated to cover the new notebook naming scheme (nb3, nb5, nb6 variants) and the new
mcp_setup_and_integration.ipynbnotebook. The expected packages list correctly reflects what each notebook installs:
retail_sales_agent_nb3for adding_tools_to_agentsretail_sales_agent_nb5for multi_agent_orchestrationretail_sales_agent_nb6for observability_evaluation_and_profilingmcp_dev_workflowandnat_simple_calculator_notebookfor mcp_setup_and_integrationAlso applies to: 140-149
Signed-off-by: Bryan Bednarski <[email protected]>
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/notebooks/adding_tools_to_agents.ipynb (1)
1-12: Add the required SPDX Apache-2.0 header.This notebook is missing the SPDX Apache-2.0 header that is required for all source files, including
.ipynbfiles. Please add the standard header at the beginning of the notebook.As per coding guidelines.
♻️ Duplicate comments (1)
examples/notebooks/tests/test_notebooks_e2e.py (1)
140-142: Critical: Fix package name prefix - regression from previous review.Line 140 uses
"nat_simple_calculator_notebook"but should be"simple_calculator_notebook"(without the"nat_"prefix) to match the package name defined in_ALL_WORKFLOWS(line 32) and created by the notebook'sinstall_example_code()function. This issue was flagged and supposedly fixed in a previous review but has regressed.Apply this diff to fix the package name:
- pytest.param("mcp_setup_and_integration.ipynb", ["mcp_dev_workflow", "nat_simple_calculator_notebook"], + pytest.param("mcp_setup_and_integration.ipynb", ["mcp_dev_workflow", "simple_calculator_notebook"],
🧹 Nitpick comments (1)
examples/notebooks/tests/test_notebooks_e2e.py (1)
120-122: Consider removing commented-out dead code.These commented-out lines serve no purpose in the current implementation. If they're needed for future reference, consider documenting the reason or moving them to a separate issue/comment.
Apply this diff to remove the dead code:
- # env = os.environ.copy() - # env["NAT_NOTEBOOK_INSTALL_MODE"] = "local" -
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/notebooks/adding_tools_to_agents.ipynb(21 hunks)examples/notebooks/tests/test_notebooks_e2e.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/notebooks/adding_tools_to_agents.ipynbexamples/notebooks/tests/test_notebooks_e2e.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/test_*.py,examples/*/tests/test_*.py,packages/*/tests/test_*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Name test files as test_*.py and store them in a tests/ folder
Files:
examples/notebooks/tests/test_notebooks_e2e.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Applied to files:
examples/notebooks/adding_tools_to_agents.ipynb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI Pipeline / Check
🔇 Additional comments (4)
examples/notebooks/adding_tools_to_agents.ipynb (2)
643-1654: LGTM: Consistent renaming to nb3.The notebook has been systematically updated to use
retail_sales_agent_nb3throughout all workflow commands, file paths, and configuration references. The naming is consistent and aligns with the PR's objective of renumbering the getting started notebooks.
1668-1672: LGTM: Next steps reference updated correctly.The reference to the next notebook (
mcp_setup_and_integration.ipynb) is appropriate and aligns with the renumbering effort. Using descriptive names without numeric prefixes is a good approach for maintainability.examples/notebooks/tests/test_notebooks_e2e.py (2)
21-33: LGTM! Workflow list expansion aligns with test expectations.The expanded list correctly includes all workflows created by the renumbered notebooks.
159-162: LGTM! Test correctly updated for renamed notebook.The notebook filename is correctly updated to remove the numeric prefix while maintaining the expected packages.
Signed-off-by: Bryan Bednarski <[email protected]>
Signed-off-by: Bryan Bednarski <[email protected]>
|
/ok to test e84451f |
There was a problem hiding this 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
🧹 Nitpick comments (1)
examples/notebooks/tests/test_notebooks_e2e.py (1)
120-122: Remove or document the commented-out environment variable code.The commented-out
NAT_NOTEBOOK_INSTALL_MODEenvironment variable setup lacks explanation. If this is intended for future use, add a TODO comment explaining why it's preserved. Otherwise, remove it to reduce code clutter.Apply this diff to remove the commented code:
- # env = os.environ.copy() - # env["NAT_NOTEBOOK_INSTALL_MODE"] = "local" -Or add a TODO if it's needed later:
+ # TODO: Add support for controlling install mode via environment variable # env = os.environ.copy() # env["NAT_NOTEBOOK_INSTALL_MODE"] = "local" -
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/notebooks/tests/test_notebooks_e2e.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{tests/test_*.py,examples/*/tests/test_*.py,packages/*/tests/test_*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Name test files as test_*.py and store them in a tests/ folder
Files:
examples/notebooks/tests/test_notebooks_e2e.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/notebooks/tests/test_notebooks_e2e.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/notebooks/tests/test_notebooks_e2e.py
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/notebooks/tests/test_notebooks_e2e.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Applied to files:
examples/notebooks/tests/test_notebooks_e2e.py
📚 Learning: 2025-08-22T20:18:28.041Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/cursor-rules.mdc:0-0
Timestamp: 2025-08-22T20:18:28.041Z
Learning: Applies to .cursor/rules/**/*.mdc : Use consistent project terminology in descriptions (e.g., NAT workflows, NAT CLI commands)
Applied to files:
examples/notebooks/tests/test_notebooks_e2e.py
🔇 Additional comments (3)
examples/notebooks/tests/test_notebooks_e2e.py (3)
27-32: Verify the workflow naming suffix pattern.The retail_sales_agent workflow now has three suffixed variants (nb3, nb5, nb6) corresponding to different notebooks. While this isolation prevents test conflicts, the non-sequential pattern (nb3, nb5, nb6 rather than nb1, nb2, nb3) may be confusing.
Consider whether these suffixes should:
- Reflect the old notebook numbers (as they currently do), which may become outdated
- Use sequential suffixes (nb1, nb2, nb3)
- Use more descriptive names based on the notebook content (e.g., retail_sales_agent_tools, retail_sales_agent_multi, retail_sales_agent_observability)
37-62: LGTM! Comment updates align with notebook renumbering.The comments have been properly updated to reference notebooks without numeric prefixes, maintaining consistency with the renumbering changes.
138-162: LGTM! Test parametrization properly updated for renamed notebooks.The test parametrization has been correctly updated to reflect the notebook renumbering, with appropriate expected packages, timeouts, and test IDs. The structure is clean and maintainable.
Signed-off-by: Bryan Bednarski <[email protected]>
Signed-off-by: Bryan Bednarski <[email protected]>
|
/merge |


Description
This PR renumbers the getting started notebooks to a more intuitive order now that we have seven of them. The mcp notebook (fourth) also needed to be added to the unit tests. READMEs also needed to represent this change.
Closes
By Submitting this PR I confirm:
Summary by CodeRabbit
Documentation
Tests