bug fix: dynamo integration - model download and instructions clarification#1420
Conversation
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
WalkthroughEnvironment and README files for Dynamo integration now export HuggingFace and Dynamo model/GPU/network environment variables; Dynamo startup scripts use a configurable WORKER_INIT_TIMEOUT_S (default increased 600→1800) and guidance switches from Changes
Sequence Diagram(s)(omitted — changes are configuration, documentation, and timeout parameterization without new multi-component control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (6)**/*.{md,mdx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*.{md,mdx,rst}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*.{py,js,ts,tsx,jsx,sh,yaml,yml,json,toml,md,mdx,rst}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*.{py,md,mdx,rst}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
examples/**/*⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (6)📚 Learning: 2026-01-05T15:46:49.676ZApplied to files:
📚 Learning: 2026-01-05T15:46:49.676ZApplied to files:
📚 Learning: 2026-01-05T15:46:49.676ZApplied to files:
📚 Learning: 2025-09-15T21:26:29.430ZApplied to files:
📚 Learning: 2026-01-05T15:46:49.676ZApplied to files:
📚 Learning: 2025-12-12T20:49:51.370ZApplied to files:
🪛 LanguageToolexternal/dynamo/README.md[style] ~218-~218: Consider a more concise word here. (IN_ORDER_TO_PREMIUM) ⏰ 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)
🔇 Additional comments (6)
✏️ Tip: You can disable this entire section by setting 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.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/dynamo_integration/.env.example (1)
1-33: Avoid shell‑special placeholder and lint warnings in the required exports.
export HF_TOKEN=<...>will be parsed as input redirection if someone sources this file before editing. Also, dotenv-linter flags quoted values here. Consider a safe placeholder and unquoted paths to keep lint clean.🛠️ Proposed fix
-export HF_TOKEN=<my_huggingface_read_token> +export HF_TOKEN=your_huggingface_read_token @@ -export DYNAMO_MODEL_DIR="/path/to/your/models/Llama-3.3-70B-Instruct" +export DYNAMO_MODEL_DIR=/path/to/your/models/Llama-3.3-70B-Instruct @@ -export DYNAMO_REPO_DIR="/path/to/NeMo-Agent-Toolkit/external/dynamo" +export DYNAMO_REPO_DIR=/path/to/NeMo-Agent-Toolkit/external/dynamo
🤖 Fix all issues with AI agents
In `@examples/dynamo_integration/.env.example`:
- Around line 36-53: Remove the unnecessary quotes around the environment
variable defaults to satisfy dotenv-linter: change lines defining
DYNAMO_GPU_DEVICES, DYNAMO_PREFILL_GPUS, DYNAMO_DECODE_GPUS, and DYNAMO_TP_SIZE
from quoted strings (e.g. DYNAMO_GPU_DEVICES="0,1,2,3") to unquoted values (e.g.
DYNAMO_GPU_DEVICES=0,1,2,3); alternatively, if quotes must stay for clarity, add
a dotenv-linter ignore comment for that variable, but preferred fix is to drop
the quotes for these symbols (DYNAMO_GPU_DEVICES, DYNAMO_PREFILL_GPUS,
DYNAMO_DECODE_GPUS, DYNAMO_TP_SIZE).
In `@examples/dynamo_integration/README.md`:
- Around line 90-95: The fenced code block under the "Quick Stop" section is
missing a language identifier; update the opening triple-backtick to "```bash"
so the snippet (containing the cd command and bash stop_dynamo.sh invocation) is
marked as Bash for proper syntax highlighting and accessibility.
- Line 73: Replace the typo in the README line that currently reads "# - 4x
NVIDIA GPUs (set WORKER_GPUS to teh available set of machines)" by changing
"teh" to "the" so it reads "# - 4x NVIDIA GPUs (set WORKER_GPUS to the
available set of machines)"; update the exact text in
examples/dynamo_integration/README.md where this string appears.
- Around line 54-65: Renumber the steps so they read 3, 4, 5, 6 (and update the
later "Run evaluation" step to 7), and fix the unquoted variable expansion in
the model download cd command: replace the unquoted cd $(dirname
$DYNAMO_MODEL_DIR) with a quoted form that handles spaces (use dirname on the
variable and quote both dirname result and the variable), leaving the other
commands (source .env, python scripts/download_agent_leaderboard_v2.py --domains
banking, and the huggingface-cli download line using --local-dir
$DYNAMO_MODEL_DIR) intact.
In `@external/dynamo/.env.example`:
- Line 41: The .env.example entry for DYNAMO_GPU_DEVICES currently uses
surrounding quotes which triggers dotenv-linter; remove the double quotes from
the value for DYNAMO_GPU_DEVICES (the variable name DYNAMO_GPU_DEVICES) so it
reads as an unquoted comma-separated list of devices to satisfy dotenv-linter
and dotenv parsing rules.
- Around line 16-33: Replace unsafe placeholder and quoted paths in the env
example: change HF_TOKEN from export HF_TOKEN=<my_huggingface_read_token> to a
safe literal placeholder (e.g., export
HF_TOKEN=your_huggingface_read_token_here) to avoid breaking source and lint
errors, and remove unnecessary quotes from DYNAMO_MODEL_DIR and DYNAMO_REPO_DIR
exports (use unquoted paths like export
DYNAMO_MODEL_DIR=/path/to/your/models/..., export
DYNAMO_REPO_DIR=/path/to/NeMo-Agent-Toolkit/external/dynamo) so dotenv-linter
won't flag them and users can safely source the file.
In `@external/dynamo/README.md`:
- Around line 205-216: The README uses an unquoted variable expansion in the
shell command `cd $(dirname $DYNAMO_MODEL_DIR)` which will break if
DYNAMO_MODEL_DIR contains spaces or special chars; update that command to quote
the expansion (use `cd "$(dirname "$DYNAMO_MODEL_DIR")"`), and ensure any
related directory creation comment/command uses the same quoting pattern for
`mkdir -p "$(dirname "$DYNAMO_MODEL_DIR")"` to be safe.
- Line 41: Fix the sentence grammar and replace the NAT acronym with the full
product name: change "Scope of the current integration is based around" to "The
scope of the current integration is based on" (or "focuses on"), and replace the
standalone "NAT" with "NeMo Agent Toolkit" in the README line that mentions
"Dynamo LLM" and the referenced module ../../src/nat/llm/dynamo_llm.py (leave
any backtick-enclosed code identifiers like `nat` or file paths unchanged).
In `@external/dynamo/start_dynamo_unified.sh`:
- Line 47: The wait_for_worker function still computes max_wait from
DYNAMO_WORKER_INIT_TIMEOUT_S with a 600s fallback so it can time out at 10
minutes; change wait_for_worker to use the already-computed
WORKER_INIT_TIMEOUT_S (which defaults to 1800) instead of directly reading
DYNAMO_WORKER_INIT_TIMEOUT_S, and ensure any places that set max_wait (including
the block around lines 276-283) derive from WORKER_INIT_TIMEOUT_S so the new
30-minute default is honored; reference the symbols WORKER_INIT_TIMEOUT_S,
wait_for_worker, DYNAMO_WORKER_INIT_TIMEOUT_S, and max_wait when making the
change.
🧹 Nitpick comments (1)
external/dynamo/README.md (1)
219-220: Consider more concise phrasing (optional).The phrase "in order to download" could be simplified to "to download" for more concise documentation.
✍️ Suggested refinement
-> [!NOTE] -> The Llama-3.3-70B-Instruct model requires approval from Meta. Request access at [huggingface.co/meta-llama/Llama-3.3-70B-Instruct](https://huggingface.co/meta-llama/Llama-3.3-70B-Instruct) before downloading. You will need to create a HuggingFace Access Token with read access in order to download the model. On the huggingface website visit: "Access Tokens" -> "+ Create access token" to generate a token starting with "hf_". Enter your token when prompted. Respond "n" when asked "Add token as git credential? (Y/n)". Set HF_HOME and HF_TOKEN in .env.. +> [!NOTE] +> The Llama-3.3-70B-Instruct model requires approval from Meta. Request access at [huggingface.co/meta-llama/Llama-3.3-70B-Instruct](https://huggingface.co/meta-llama/Llama-3.3-70B-Instruct) before downloading. You will need to create a HuggingFace Access Token with read access to download the model. On the huggingface website visit: "Access Tokens" -> "+ Create access token" to generate a token starting with "hf_". Enter your token when prompted. Respond "n" when asked "Add token as git credential? (Y/n)". Set HF_HOME and HF_TOKEN in .env.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/dynamo_integration/.env.exampleexamples/dynamo_integration/README.mdexternal/dynamo/.env.exampleexternal/dynamo/README.mdexternal/dynamo/start_dynamo_disagg.shexternal/dynamo/start_dynamo_unified.shexternal/dynamo/start_dynamo_unified_thompson_hints.sh
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{py,js,ts,tsx,jsx,sh,yaml,yml,json,toml,md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,js,ts,tsx,jsx,sh,yaml,yml,json,toml,md,mdx,rst}: Every file must start with the standard SPDX Apache-2.0 header
Confirm that copyright years are up-to-date whenever a file is changed
All source files must include the SPDX Apache-2.0 header template
Files:
external/dynamo/start_dynamo_disagg.shexamples/dynamo_integration/README.mdexternal/dynamo/start_dynamo_unified_thompson_hints.shexternal/dynamo/README.mdexternal/dynamo/start_dynamo_unified.sh
**/*
⚙️ 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 (except for return values of
None,
in that situation no return type hint is needed).
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.
- Documentation in Markdown files should not contain usage of a possessive 's with inanimate objects
(ex: "the system's performance" should be "the performance of the system").- Documentation in Markdown files should not use NAT as an acronym, always spell out NeMo Agent Toolkit.
The exception to this rule is when referring to package names or code identifiers that contain "nat", th...
Files:
external/dynamo/start_dynamo_disagg.shexamples/dynamo_integration/README.mdexternal/dynamo/start_dynamo_unified_thompson_hints.shexternal/dynamo/README.mdexternal/dynamo/start_dynamo_unified.sh
**/*.{md,mdx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{md,mdx}: Use 'NVIDIA NeMo Agent toolkit' for full name (first use), 'NeMo Agent toolkit' or 'the toolkit' for subsequent references, and 'Toolkit' (capital T) in titles/headings, 'toolkit' (lowercase t) in body text
Never use deprecated names: 'Agent Intelligence toolkit', 'aiqtoolkit', 'AgentIQ', 'AIQ', or 'aiq' in documentation; update any occurrences unless intentionally referring to deprecated versions or implementing compatibility layers
Files:
examples/dynamo_integration/README.mdexternal/dynamo/README.md
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{md,mdx,rst}: Documentation must be clear, comprehensive, and free of TODOs, FIXMEs, placeholder text, offensive or outdated terms, and spelling mistakes
Do not use words listed in 'ci/vale/styles/config/vocabularies/nat/reject.txt' in documentation
Words listed in 'ci/vale/styles/config/vocabularies/nat/accept.txt' are acceptable even if they appear to be spelling mistakes
Files:
examples/dynamo_integration/README.mdexternal/dynamo/README.md
**/*.{py,md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Version numbers are derived automatically by 'setuptools-scm'; never hard-code them in code or docs
Files:
examples/dynamo_integration/README.mdexternal/dynamo/README.md
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/dynamo_integration/README.md
🧠 Learnings (2)
📚 Learning: 2026-01-08T22:05:24.771Z
Learnt from: bbednarski9
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 1357
File: external/dynamo/start_dynamo_unified.sh:40-40
Timestamp: 2026-01-08T22:05:24.771Z
Learning: The container image nvcr.io/nvidia/ai-dynamo/sglang-runtime:0.7.1 is available and valid for use in the NVIDIA Dynamo SGLang runtime scripts.
Applied to files:
external/dynamo/start_dynamo_disagg.shexternal/dynamo/start_dynamo_unified_thompson_hints.sh
📚 Learning: 2026-01-05T15:46:49.676Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2026-01-05T15:46:49.676Z
Learning: Name test files 'test_*.py' and store them alongside the code in a 'tests/' folder
Applied to files:
examples/dynamo_integration/README.md
🪛 dotenv-linter (4.0.0)
external/dynamo/.env.example
[warning] 30-30: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 33-33: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 41-41: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
examples/dynamo_integration/.env.example
[warning] 30-30: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 33-33: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 41-41: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 LanguageTool
external/dynamo/README.md
[style] ~220-~220: Consider a more concise word here.
Context: ...ggingFace Access Token with read access in order to download the model. On the huggingface ...
(IN_ORDER_TO_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
examples/dynamo_integration/README.md
84-84: Blank line inside blockquote
(MD028, no-blanks-blockquote)
91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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 (15)
external/dynamo/start_dynamo_unified.sh (1)
486-488: LGTM: API readiness loop now uses the configurable timeout.external/dynamo/start_dynamo_disagg.sh (2)
44-44: LGTM: worker init timeout default bumped to 1800s.
605-606: LGTM: API availability wait now follows the shared timeout.external/dynamo/start_dynamo_unified_thompson_hints.sh (2)
49-49: LGTM: worker init timeout default updated.
611-612: LGTM: API availability wait now follows the shared timeout.examples/dynamo_integration/.env.example (3)
56-73: LGTM: network configuration guidance is clear.
76-85: LGTM: model configuration section looks good.
88-97: LGTM: disaggregated mode options are well documented.external/dynamo/README.md (4)
2-2: LGTM: Copyright year updated correctly.The copyright year has been properly updated to reflect 2025-2026, aligning with the coding guidelines requirement to keep copyright years current when files are modified.
178-178: LGTM: Hardware recommendations updated.The recommendation clarifies that B200 GPUs provide higher throughput, which aligns with the PR's goal to emphasize higher-throughput capabilities.
242-254: LGTM: Environment variable documentation added.The addition of
HF_HOMEandHF_TOKENenvironment variables properly documents the new env-driven workflow for model downloads, aligning with the PR's objective to standardize model download paths.
362-362: LGTM: Startup time guidance reflects extended timeouts.The updated guidance accurately reflects that startup may take 5-20 minutes for large models, which aligns with the PR's fix to extend the combined model load and API availability wait time to accommodate cold starts.
examples/dynamo_integration/README.md (3)
2-2: LGTM: Copyright year updated correctly.The copyright year has been properly updated to 2025-2026, following coding guidelines.
82-86: LGTM: Documentation blocks properly formatted.The WARNING and NOTE blocks are clearly separated and provide helpful context about model loading time and configuration options. The blank line between them (flagged by static analysis) is intentional for visual separation and readability.
183-185: LGTM: Test file references and outputs documentation updated.The updates correctly reflect the current file structure, with the new test file reference and clarification that outputs are generated at runtime.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@external/dynamo/README.md`:
- Line 212: The README sentence "llama models from requires approval from Meta.
See `NOTE` below." has bad grammar; update that line to a correct phrase such as
"llama models from Meta require approval. See `NOTE` below." (or "llama models
require approval. See `NOTE` below.") so it reads clearly; edit the string in
the README where that sentence appears.
♻️ Duplicate comments (4)
external/dynamo/.env.example (1)
18-18: Consider a safer placeholder format for HF_TOKEN.The placeholder
my_huggingface_read_tokencould be misinterpreted as a valid token format. A past review comment suggested using a more explicit placeholder likeyour_huggingface_read_token_hereto make it clearer that this value must be replaced.📝 Suggested placeholder improvement
-export HF_TOKEN=my_huggingface_read_token +export HF_TOKEN=your_huggingface_read_token_hereexternal/dynamo/README.md (2)
41-41: Replace NAT acronym with full product name per coding guidelines.The coding guidelines explicitly state: "Documentation should spell out NeMo Agent Toolkit instead of using the 'NAT' acronym (except when referring to code identifiers in backticks)." Additionally, the grammar could be improved.
📝 Proposed fix
-Dynamo is NVIDIA's high-performance LLM serving platform with KV cache optimization. Scope of the current integration is based around two core aspects. First, we have implemented a [Dynamo LLM](../../src/nat/llm/dynamo_llm.py) support for NeMo Agent toolkit inference on Dynamo runtimes. Second, we provide a set of startup scripts for NVIDIA Hopper and Blackwell GPU servers supporting NAT runtimes at scale. The following **Table** defines each script: +Dynamo is NVIDIA's high-performance LLM serving platform with KV cache optimization. The scope of the current integration focuses on two core aspects. First, we have implemented a [Dynamo LLM](../../src/nat/llm/dynamo_llm.py) support for NeMo Agent toolkit inference on Dynamo runtimes. Second, we provide a set of startup scripts for NVIDIA Hopper and Blackwell GPU servers supporting NeMo Agent Toolkit runtimes at scale. The following **Table** defines each script:Based on coding guidelines requiring full product name usage.
209-209: Quote variable expansion to handle paths with spaces.The unquoted variable expansion will fail if
DYNAMO_MODEL_DIRcontains spaces or special characters.🛡️ Proposed fix
-cd $(dirname $DYNAMO_MODEL_DIR) +cd "$(dirname "$DYNAMO_MODEL_DIR")"examples/dynamo_integration/README.md (1)
63-63: Quote variable expansion to handle paths with spaces.The command uses unquoted variable expansion which will fail if
DYNAMO_MODEL_DIRcontains spaces or special characters. This is a standard shell safety issue.🛡️ Proposed fix
-cd $(dirname $DYNAMO_MODEL_DIR) # parent directory of the intended model weights directory +cd "$(dirname "$DYNAMO_MODEL_DIR")" # parent directory of the intended model weights directory
🧹 Nitpick comments (2)
examples/dynamo_integration/README.md (2)
82-86: Consider removing blank line in blockquote for linter compliance.The blank line at line 84 inside the blockquote triggers markdownlint rule MD028. While this is a minor formatting issue, removing it would satisfy the linter.
🔧 Proposed fix
> [!WARNING] > The first load of model weights to `SGLang` workers can take significant time. - > [!NOTE] > To customize GPU workers and tensor parallelism, edit the configuration variables at the top of external/dynamo/start_dynamo_unified.sh.
278-278: Consistent placeholder usage across documentation.The
HF_TOKENplaceholder here matches the format in.env.example. If the placeholder format is updated in the env file, this should be updated accordingly for consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/dynamo_integration/.env.exampleexamples/dynamo_integration/README.mdexternal/dynamo/.env.exampleexternal/dynamo/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/dynamo_integration/.env.example
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{md,mdx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{md,mdx}: Use 'NVIDIA NeMo Agent toolkit' for full name (first use), 'NeMo Agent toolkit' or 'the toolkit' for subsequent references, and 'Toolkit' (capital T) in titles/headings, 'toolkit' (lowercase t) in body text
Never use deprecated names: 'Agent Intelligence toolkit', 'aiqtoolkit', 'AgentIQ', 'AIQ', or 'aiq' in documentation; update any occurrences unless intentionally referring to deprecated versions or implementing compatibility layers
Files:
examples/dynamo_integration/README.mdexternal/dynamo/README.md
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{md,mdx,rst}: Documentation must be clear, comprehensive, and free of TODOs, FIXMEs, placeholder text, offensive or outdated terms, and spelling mistakes
Do not use words listed in 'ci/vale/styles/config/vocabularies/nat/reject.txt' in documentation
Words listed in 'ci/vale/styles/config/vocabularies/nat/accept.txt' are acceptable even if they appear to be spelling mistakes
Files:
examples/dynamo_integration/README.mdexternal/dynamo/README.md
**/*.{py,js,ts,tsx,jsx,sh,yaml,yml,json,toml,md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,js,ts,tsx,jsx,sh,yaml,yml,json,toml,md,mdx,rst}: Every file must start with the standard SPDX Apache-2.0 header
Confirm that copyright years are up-to-date whenever a file is changed
All source files must include the SPDX Apache-2.0 header template
Files:
examples/dynamo_integration/README.mdexternal/dynamo/README.md
**/*.{py,md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Version numbers are derived automatically by 'setuptools-scm'; never hard-code them in code or docs
Files:
examples/dynamo_integration/README.mdexternal/dynamo/README.md
**/*
⚙️ 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 (except for return values of
None,
in that situation no return type hint is needed).
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.
- Documentation in Markdown files should not contain usage of a possessive 's with inanimate objects
(ex: "the system's performance" should be "the performance of the system").- Documentation in Markdown files should not use NAT as an acronym, always spell out NeMo Agent Toolkit.
The exception to this rule is when referring to package names or code identifiers that contain "nat", th...
Files:
examples/dynamo_integration/README.mdexternal/dynamo/README.md
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/dynamo_integration/README.md
🧠 Learnings (7)
📚 Learning: 2026-01-05T15:46:49.676Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2026-01-05T15:46:49.676Z
Learning: Name test files 'test_*.py' and store them alongside the code in a 'tests/' folder
Applied to files:
examples/dynamo_integration/README.md
📚 Learning: 2026-01-05T15:46:49.677Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2026-01-05T15:46:49.677Z
Learning: Applies to CHANGELOG.md : Add user-visible changes to 'CHANGELOG.md' under the appropriate section
Applied to files:
external/dynamo/README.md
📚 Learning: 2026-01-05T15:46:49.676Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2026-01-05T15:46:49.676Z
Learning: Use technical identifiers: 'nat' for API namespace and CLI tool, 'nvidia-nat' for package name, 'NAT_' prefix for environment variables, 'NeMo-Agent-Toolkit' for URLs and directory names
Applied to files:
external/dynamo/README.md
📚 Learning: 2026-01-05T15:46:49.676Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2026-01-05T15:46:49.676Z
Learning: Applies to **/*.{md,mdx} : Use 'NVIDIA NeMo Agent toolkit' for full name (first use), 'NeMo Agent toolkit' or 'the toolkit' for subsequent references, and 'Toolkit' (capital T) in titles/headings, 'toolkit' (lowercase t) in body text
Applied to files:
external/dynamo/README.md
📚 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:
external/dynamo/README.md
📚 Learning: 2026-01-05T15:46:49.676Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2026-01-05T15:46:49.676Z
Learning: Applies to **/*.{md,mdx} : Never use deprecated names: 'Agent Intelligence toolkit', 'aiqtoolkit', 'AgentIQ', 'AIQ', or 'aiq' in documentation; update any occurrences unless intentionally referring to deprecated versions or implementing compatibility layers
Applied to files:
external/dynamo/README.md
📚 Learning: 2025-12-12T20:49:51.370Z
Learnt from: zterek
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 1243
File: examples/risk_and_security/retail_agent/src/nat_retail_agent/configs/red-teaming.yml:1-98
Timestamp: 2025-12-12T20:49:51.370Z
Learning: In the NVIDIA/NeMo-Agent-Toolkit repository, most YAML configuration files use 2-space indentation rather than 4-space indentation. When reviewing YAML files, prefer consistency with the existing codebase style (2-space indentation) over the stated coding guideline (4-space indentation) until a repository-wide standardization is performed.
Applied to files:
external/dynamo/README.md
🪛 LanguageTool
external/dynamo/README.md
[style] ~220-~220: Consider a more concise word here.
Context: ...ggingFace Access Token with read access in order to download the model. On the huggingface ...
(IN_ORDER_TO_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
examples/dynamo_integration/README.md
84-84: Blank line inside blockquote
(MD028, no-blanks-blockquote)
⏰ 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 (5)
external/dynamo/.env.example (1)
20-30: LGTM: Clear documentation for model directory setup.The extensive example output showing the expected directory structure is helpful for users to validate their model installation.
examples/dynamo_integration/README.md (1)
54-79: LGTM: Well-structured setup instructions.The numbered steps are clear and include helpful comments about expected wait times and file locations. The previous numbering issues have been properly addressed.
external/dynamo/README.md (3)
45-47: LGTM: Clear table describing deployment modes.The updated descriptions for Unified and Unified + Thompson modes are clear and appropriately distinguish between development and production use cases.
219-220: LGTM: Comprehensive setup instructions.The NOTE block provides clear instructions for obtaining HuggingFace access, including specific steps for token generation and environment variable configuration.
362-362: Improved startup time guidance.The updated startup time estimate of 5-20 minutes (previously 5 minutes) better reflects cold-start scenarios and aligns with the PR objectives to address race conditions during model loading.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@examples/dynamo_integration/README.md`:
- Around line 84-88: There is an extra blank line separating two consecutive
blockquote callouts ("[!WARNING]" and "[!NOTE]") which triggers markdownlint
MD028; remove the blank line so the blockquotes are directly adjacent (or
alternatively insert a non-blockquote separator line if you intend them to be
distinct) to fix the lint error in the README content.
♻️ Duplicate comments (1)
examples/dynamo_integration/README.md (1)
63-66: Add inner quotes around variable in dirname call.Line 64 uses
$(dirname $DYNAMO_MODEL_DIR)but the variable should be quoted inside dirname for paths with spaces:$(dirname "$DYNAMO_MODEL_DIR").🛡️ Proposed fix
# 5. Download the model weights (requires HuggingFace account) -cd "$(dirname $DYNAMO_MODEL_DIR)" # parent directory of the intended model weights directory +cd "$(dirname "$DYNAMO_MODEL_DIR")" # parent directory of the intended model weights directory
🧹 Nitpick comments (2)
external/dynamo/README.md (2)
41-41: Grammar and naming convention addressed.The sentence now correctly uses "The scope of the current integration is based around" and spells out "NeMo Agent toolkit" instead of the NAT acronym, addressing the previous review feedback. However, consider changing "is based around" to "focuses on" for improved clarity.
✏️ Minor wording improvement
-Dynamo is NVIDIA's high-performance LLM serving platform with KV cache optimization. The scope of the current integration is based around two core aspects. First, we have implemented a [Dynamo LLM](../../src/nat/llm/dynamo_llm.py) support for NeMo Agent toolkit inference on Dynamo runtimes. Second, we provide a set of startup scripts for NVIDIA Hopper and Blackwell GPU servers supporting NeMo Agent toolkit runtimes at scale. The following **Table** defines each script: +Dynamo is NVIDIA's high-performance LLM serving platform with KV cache optimization. The scope of the current integration focuses on two core aspects. First, we have implemented a [Dynamo LLM](../../src/nat/llm/dynamo_llm.py) support for NeMo Agent toolkit inference on Dynamo runtimes. Second, we provide a set of startup scripts for NVIDIA Hopper and Blackwell GPU servers supporting NeMo Agent toolkit runtimes at scale. The following **Table** defines each script:
217-218: Consider more concise wording.Per static analysis, "in order to download" could be simplified to "to download".
✏️ Concise wording
-> [!NOTE] -> The Llama-3.3-70B-Instruct model requires approval from Meta. Request access at [huggingface.co/meta-llama/Llama-3.3-70B-Instruct](https://huggingface.co/meta-llama/Llama-3.3-70B-Instruct) before downloading. You will need to create a HuggingFace Access Token with read access in order to download the model. On the `HuggingFace` website visit: "Access Tokens" -> "+ Create access token" to generate a token starting with `hf_`. Enter your token when prompted. Respond "n" when asked "Add token as git credential? (Y/n)". Set HF_HOME and HF_TOKEN in .env.. +> [!NOTE] +> The Llama-3.3-70B-Instruct model requires approval from Meta. Request access at [huggingface.co/meta-llama/Llama-3.3-70B-Instruct](https://huggingface.co/meta-llama/Llama-3.3-70B-Instruct) before downloading. You will need to create a HuggingFace Access Token with read access to download the model. On the `HuggingFace` website visit: "Access Tokens" -> "+ Create access token" to generate a token starting with `hf_`. Enter your token when prompted. Respond "n" when asked "Add token as git credential? (Y/n)". Set HF_HOME and HF_TOKEN in .env.Also note the trailing double period
..at the end should be a single..
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/dynamo_integration/README.mdexternal/dynamo/README.mdexternal/dynamo/start_dynamo_disagg.shexternal/dynamo/start_dynamo_unified.shexternal/dynamo/start_dynamo_unified_thompson_hints.sh
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{py,js,ts,tsx,jsx,sh,yaml,yml,json,toml,md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,js,ts,tsx,jsx,sh,yaml,yml,json,toml,md,mdx,rst}: Every file must start with the standard SPDX Apache-2.0 header
Confirm that copyright years are up-to-date whenever a file is changed
All source files must include the SPDX Apache-2.0 header template
Files:
external/dynamo/start_dynamo_unified_thompson_hints.shexamples/dynamo_integration/README.mdexternal/dynamo/README.mdexternal/dynamo/start_dynamo_disagg.shexternal/dynamo/start_dynamo_unified.sh
**/*
⚙️ 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 (except for return values of
None,
in that situation no return type hint is needed).
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.
- Documentation in Markdown files should not contain usage of a possessive 's with inanimate objects
(ex: "the system's performance" should be "the performance of the system").- Documentation in Markdown files should not use NAT as an acronym, always spell out NeMo Agent Toolkit.
The exception to this rule is when referring to package names or code identifiers that contain "nat", th...
Files:
external/dynamo/start_dynamo_unified_thompson_hints.shexamples/dynamo_integration/README.mdexternal/dynamo/README.mdexternal/dynamo/start_dynamo_disagg.shexternal/dynamo/start_dynamo_unified.sh
**/*.{md,mdx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{md,mdx}: Use 'NVIDIA NeMo Agent toolkit' for full name (first use), 'NeMo Agent toolkit' or 'the toolkit' for subsequent references, and 'Toolkit' (capital T) in titles/headings, 'toolkit' (lowercase t) in body text
Never use deprecated names: 'Agent Intelligence toolkit', 'aiqtoolkit', 'AgentIQ', 'AIQ', or 'aiq' in documentation; update any occurrences unless intentionally referring to deprecated versions or implementing compatibility layers
Files:
examples/dynamo_integration/README.mdexternal/dynamo/README.md
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{md,mdx,rst}: Documentation must be clear, comprehensive, and free of TODOs, FIXMEs, placeholder text, offensive or outdated terms, and spelling mistakes
Do not use words listed in 'ci/vale/styles/config/vocabularies/nat/reject.txt' in documentation
Words listed in 'ci/vale/styles/config/vocabularies/nat/accept.txt' are acceptable even if they appear to be spelling mistakes
Files:
examples/dynamo_integration/README.mdexternal/dynamo/README.md
**/*.{py,md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Version numbers are derived automatically by 'setuptools-scm'; never hard-code them in code or docs
Files:
examples/dynamo_integration/README.mdexternal/dynamo/README.md
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/dynamo_integration/README.md
🧠 Learnings (7)
📚 Learning: 2026-01-08T22:05:24.771Z
Learnt from: bbednarski9
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 1357
File: external/dynamo/start_dynamo_unified.sh:40-40
Timestamp: 2026-01-08T22:05:24.771Z
Learning: The container image nvcr.io/nvidia/ai-dynamo/sglang-runtime:0.7.1 is available and valid for use in the NVIDIA Dynamo SGLang runtime scripts.
Applied to files:
external/dynamo/start_dynamo_unified_thompson_hints.shexternal/dynamo/start_dynamo_disagg.sh
📚 Learning: 2026-01-05T15:46:49.676Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2026-01-05T15:46:49.676Z
Learning: Name test files 'test_*.py' and store them alongside the code in a 'tests/' folder
Applied to files:
examples/dynamo_integration/README.md
📚 Learning: 2026-01-05T15:46:49.676Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2026-01-05T15:46:49.676Z
Learning: Use technical identifiers: 'nat' for API namespace and CLI tool, 'nvidia-nat' for package name, 'NAT_' prefix for environment variables, 'NeMo-Agent-Toolkit' for URLs and directory names
Applied to files:
external/dynamo/README.md
📚 Learning: 2026-01-05T15:46:49.676Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2026-01-05T15:46:49.676Z
Learning: Applies to **/*.{md,mdx} : Use 'NVIDIA NeMo Agent toolkit' for full name (first use), 'NeMo Agent toolkit' or 'the toolkit' for subsequent references, and 'Toolkit' (capital T) in titles/headings, 'toolkit' (lowercase t) in body text
Applied to files:
external/dynamo/README.md
📚 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:
external/dynamo/README.md
📚 Learning: 2026-01-05T15:46:49.676Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2026-01-05T15:46:49.676Z
Learning: Applies to **/*.{md,mdx} : Never use deprecated names: 'Agent Intelligence toolkit', 'aiqtoolkit', 'AgentIQ', 'AIQ', or 'aiq' in documentation; update any occurrences unless intentionally referring to deprecated versions or implementing compatibility layers
Applied to files:
external/dynamo/README.md
📚 Learning: 2025-12-12T20:49:51.370Z
Learnt from: zterek
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 1243
File: examples/risk_and_security/retail_agent/src/nat_retail_agent/configs/red-teaming.yml:1-98
Timestamp: 2025-12-12T20:49:51.370Z
Learning: In the NVIDIA/NeMo-Agent-Toolkit repository, most YAML configuration files use 2-space indentation rather than 4-space indentation. When reviewing YAML files, prefer consistency with the existing codebase style (2-space indentation) over the stated coding guideline (4-space indentation) until a repository-wide standardization is performed.
Applied to files:
external/dynamo/README.md
🪛 LanguageTool
external/dynamo/README.md
[style] ~218-~218: Consider a more concise word here.
Context: ...ggingFace Access Token with read access in order to download the model. On the `HuggingFace...
(IN_ORDER_TO_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
examples/dynamo_integration/README.md
86-86: Blank line inside blockquote
(MD028, no-blanks-blockquote)
⏰ 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 (13)
external/dynamo/start_dynamo_unified.sh (4)
47-47: LGTM! Timeout increase properly addresses cold-start scenarios.The increase from 600s (10 min) to 1800s (30 min) for
WORKER_INIT_TIMEOUT_Sis appropriate for large model loading on cold starts, as mentioned in the PR objectives.
280-281: Correctly usesWORKER_INIT_TIMEOUT_Sfor worker init timeout.The
wait_for_workerfunction now properly derivesmax_waitfromWORKER_INIT_TIMEOUT_S, aligning with the 30-minute default. This addresses the previous review comment about timeout alignment.
487-488: API availability timeout properly aligned with worker init timeout.The API readiness check now uses
WORKER_INIT_TIMEOUT_Sfor both the message andmax_attempts, ensuring consistent timeout behavior across all startup phases.
85-85: No changes needed — thehf downloadCLI command is available and correctly used.The newer Hugging Face CLI (
huggingface_hub) provides thehf downloadcommand as the documented approach for downloading models. The usage in lines 85 and 220 with the syntaxhf download meta-llama/Llama-3.3-70B-Instruct --local-dir ${DYNAMO_MODEL_DIR}is correct and supported.external/dynamo/start_dynamo_disagg.sh (3)
44-44: Changes consistent with unified script.The timeout increase and
hf downloadcommand updates are properly aligned with the changes instart_dynamo_unified.sh.Also applies to: 82-82, 216-216
412-413:wait_for_workers_parallelcorrectly uses dynamic timeout.The parallel worker wait function properly derives
max_waitfromWORKER_INIT_TIMEOUT_S, maintaining consistency with the 30-minute default across all scripts.
605-606: API availability check properly aligned.Consistent with the unified script, the API readiness check now uses the dynamic
WORKER_INIT_TIMEOUT_Svalue.external/dynamo/start_dynamo_unified_thompson_hints.sh (2)
49-49: Changes consistent across all startup scripts.The timeout and CLI command updates are properly aligned with the other startup scripts, ensuring consistent behavior across unified, disaggregated, and Thompson sampling modes.
Also applies to: 83-83, 243-243
311-312: Timeout handling properly aligned.Both the
wait_for_workerfunction and API availability check correctly useWORKER_INIT_TIMEOUT_Swith the 30-minute default.Also applies to: 611-612
external/dynamo/README.md (2)
205-214: Variable quoting fixed.The
cdcommand now properly quotes the variable expansion ascd "$(dirname "$DYNAMO_MODEL_DIR")", addressing the previous review comment about paths with spaces.
349-349: Startup time guidance updated appropriately.The updated range of 5-20 minutes reflects the variability in startup time depending on system cache state, which is helpful for user expectations.
examples/dynamo_integration/README.md (2)
92-97: Quick Stop section properly formatted.The code fence now includes the
bashlanguage identifier as requested in the previous review.
185-187: Test file reference and outputs directory updated.The test file rename from
test_prefix_utils.pytotest_tool_intent_buffer.pyand the outputs directory description update are consistent with the codebase changes.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@examples/dynamo_integration/README.md`:
- Around line 64-66: The shell commands use unquoted variables which can break
on paths with spaces or special characters; update the two instances to quote
the variable—wrap the inner variable inside dirname (change the cd command that
uses dirname $DYNAMO_MODEL_DIR) and quote the --local-dir argument passed to the
hf download command (the hf download ... --local-dir $DYNAMO_MODEL_DIR) so both
become safely quoted references to $DYNAMO_MODEL_DIR.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/dynamo_integration/README.md
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{md,mdx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{md,mdx}: Use 'NVIDIA NeMo Agent toolkit' for full name (first use), 'NeMo Agent toolkit' or 'the toolkit' for subsequent references, and 'Toolkit' (capital T) in titles/headings, 'toolkit' (lowercase t) in body text
Never use deprecated names: 'Agent Intelligence toolkit', 'aiqtoolkit', 'AgentIQ', 'AIQ', or 'aiq' in documentation; update any occurrences unless intentionally referring to deprecated versions or implementing compatibility layers
Files:
examples/dynamo_integration/README.md
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{md,mdx,rst}: Documentation must be clear, comprehensive, and free of TODOs, FIXMEs, placeholder text, offensive or outdated terms, and spelling mistakes
Do not use words listed in 'ci/vale/styles/config/vocabularies/nat/reject.txt' in documentation
Words listed in 'ci/vale/styles/config/vocabularies/nat/accept.txt' are acceptable even if they appear to be spelling mistakes
Files:
examples/dynamo_integration/README.md
**/*.{py,js,ts,tsx,jsx,sh,yaml,yml,json,toml,md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,js,ts,tsx,jsx,sh,yaml,yml,json,toml,md,mdx,rst}: Every file must start with the standard SPDX Apache-2.0 header
Confirm that copyright years are up-to-date whenever a file is changed
All source files must include the SPDX Apache-2.0 header template
Files:
examples/dynamo_integration/README.md
**/*.{py,md,mdx,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Version numbers are derived automatically by 'setuptools-scm'; never hard-code them in code or docs
Files:
examples/dynamo_integration/README.md
**/*
⚙️ 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 (except for return values of
None,
in that situation no return type hint is needed).
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.
- Documentation in Markdown files should not contain usage of a possessive 's with inanimate objects
(ex: "the system's performance" should be "the performance of the system").- Documentation in Markdown files should not use NAT as an acronym, always spell out NeMo Agent Toolkit.
The exception to this rule is when referring to package names or code identifiers that contain "nat", th...
Files:
examples/dynamo_integration/README.md
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/dynamo_integration/README.md
🧠 Learnings (1)
📚 Learning: 2026-01-05T15:46:49.676Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2026-01-05T15:46:49.676Z
Learning: Name test files 'test_*.py' and store them alongside the code in a 'tests/' folder
Applied to files:
examples/dynamo_integration/README.md
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
mnajafian-nv
left a comment
There was a problem hiding this comment.
The PR is fundamentally sound (fixes the timeout race condition and standardizes configuration), BUT it may introduces some issues that will break the user experience. For details please look into the inline comments.
Question about the hf download Command Availability: The PR changes from huggingface-cli download to hf download. While hf is the newer CLI, can we please verify: Is it available in the same package? Does it have the same syntax? Will this break for users with older huggingface_hub versions?
ccb7a5d to
47fe931
Compare
|
Note: switching from "huggingface-cli" to "hf" is backwards compatible and suppresses a warning message about deprecation. |
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
|
/merge |
…cation (NVIDIA#1420) Clarify the download location for huggingface open source models onto the GPU workstation. Corrected by standardization of the .env.example files between the dynamo example and the external dynamo components, as they will likely be run back-to-back. Also fixed a race condition in the startup scripts so that model load and api availability both run up to 30 minutes (longer than original 15 min to account for cold start scenarios). General improvements to docs. Closes NVBug 5756833 ## By Submitting this PR I confirm: - I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/NeMo-Agent-Toolkit/blob/develop/docs/source/resources/contributing/index.md). - We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license. - Any contribution which contains commits that are not Signed-Off will not be accepted. - When the PR is ready for review, new or existing tests cover these changes. - When the PR is ready for review, the documentation is up to date with these changes. ## Summary by CodeRabbit * **Documentation** * Updated quick-start and setup to use a sourced .env (explicit HF_HOME/HF_TOKEN), clarified model-weight download step, reordered startup flow, added warnings about initial model load time, updated evaluation/artifact guidance, and replaced huggingface-cli examples with hf. * **Chores** * Exposed many configuration options as exported environment variables (model/repo paths, GPU workers, tensor-parallel settings, ports, etc.) and increased worker initialization timeout defaults for large-model readiness. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> Authors: - Bryan Bednarski (https://github.com/bbednarski9) Approvers: - https://github.com/mnajafian-nv URL: NVIDIA#1420 Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
Description
Clarify the download location for huggingface open source models onto the GPU workstation. Corrected by standardization of the .env.example files between the dynamo example and the external dynamo components, as they will likely be run back-to-back. Also fixed a race condition in the startup scripts so that model load and api availability both run up to 30 minutes (longer than original 15 min to account for cold start scenarios). General improvements to docs.
Closes
NVBug 5756833
By Submitting this PR I confirm:
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.