feat: add Brev Launchable provisioning script - #687
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughChangesThe PR adds Brev launch documentation and an idempotent provisioning script. It installs the pinned runtime, stages tutorials, configures environment variables and a Jupyter kernel, runs smoke checks, and publishes a welcome guide. Brev provisioning hardening
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryAdds a one-click Brev GPU environment for evaluating NeMo Safe Synthesizer.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the current retry guards repair incomplete notebook support and tutorial extraction, while the pinned uv artifact is checked against its published checksum before installation. Important Files Changed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
script/brev/setup.sh (1)
76-82: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider downloading the uv installer before executing it.
Piping
curlstraight intosh(flagged by static analysis, CWE-494) means a compromised/MITM'dastral.shresponse executes immediately with no chance to inspect it. astral's own docs offercurl ... | lessas an inspection step before running, and the URL is already version-pinned here, so splitting download from execution is a low-cost hardening step consistent with the path instructions' call to review dependency/tooling scripts for supply-chain risk.🔒 Proposed fix: download then execute
- curl -LsSf "https://astral.sh/uv/${UV_VERSION}/install.sh" \ - | env UV_INSTALL_DIR="${BIN_DIR}" INSTALLER_NO_MODIFY_PATH=1 sh + uv_installer="$(mktemp)" + curl -LsSf "https://astral.sh/uv/${UV_VERSION}/install.sh" -o "${uv_installer}" + env UV_INSTALL_DIR="${BIN_DIR}" INSTALLER_NO_MODIFY_PATH=1 sh "${uv_installer}" + rm -f "${uv_installer}"Sources: Path instructions, Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2aa96fa5-3e92-4414-81ac-4979df077987
📒 Files selected for processing (2)
script/brev/README.mdscript/brev/setup.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Greptile Review
- GitHub Check: Analyze (Python)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{md,markdown,py}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings
Files:
script/brev/README.md
**/*.{md,markdown}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown}: Bold is acceptable only in markdown tables where it's the conventional way to mark header-like cells in the body
Use##headers to segment markdown sections instead of bold text
Use--(em-dash) instead of-(hyphen) for asides in markdown
Files:
script/brev/README.md
**/*.md
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Do not use decorative bold in Markdown body text, list items, or docstrings; use single backticks for code identifiers, paths, and commands.
Files:
script/brev/README.md
**/*
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
**/*: Every source file requires the SPDX copyright and license header appropriate to its file format.
End files with a newline, remove trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.
**/*: All contributions must use verified Git commits and DCO sign-off; unsigned or unsigned-off commits cannot be merged.
Branches other thanmainmust follow<author>/<description>, optionally including an issue ID or type; branch names must use lowercase alphanumeric characters and hyphens.
Commits merged tomainmust follow Conventional Commits, using a valid lowercase type and a description of at most 100 characters.
Files:
script/brev/README.mdscript/brev/setup.sh
⚙️ CodeRabbit configuration file
**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.
- Refactor suggestion: use for local maintainability problems introduced
by the diff when they have clear future cost, such as duplicated setup,
unclear boundaries, over-mocking, avoidable complexity, or opaque test
helpers.- Nitpick: avoid in chill mode. Do not emit formatting, import-order,
wording, or style-only comments unless automated tools cannot catch the
issue and it affects maintainability.Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.
- Major: incorrect generation/training/evaluation behavior, broken
CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
cleanup and process-isolation bugs likely to fail CI or production
runs.- Minor: localized bugs, missing focused tests for changed behavior, or
bad test patterns that weaken regression coverage.- Trivial: small cleanup with no behavior impact. Usually suppress in
chill mode.- Info: context only. Avoid unless it helps reviewers understand risk.
Safe-Synthesizer-specific review focus: - Data ...
Files:
script/brev/README.mdscript/brev/setup.sh
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Python, shell, YAML, YML, and Markdown source files require SPDX copyright headers.
Files:
script/brev/README.mdscript/brev/setup.sh
**/*.{py,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use the repository's documented Python and Markdown style conventions and validate changes with the pinned
miseformatting and checking tasks.
Files:
script/brev/README.md
script/**
⚙️ CodeRabbit configuration file
Review standalone scripts for reproducibility and operational safety. Check argument validation, quoting, repo-root detection, environment variables, generated artifacts, external commands, GPU/cluster assumptions, and whether the script should be wired through Makefile or documented in README/docs.
Files:
script/brev/README.mdscript/brev/setup.sh
**/*.{sh,bash}
📄 CodeRabbit inference engine (AGENTS.md)
Never use
~inside double-quoted strings in shell scripts -- use$HOMEor an absolute path instead
Files:
script/brev/setup.sh
**/*.{py,sh}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use the repository's pinned
misetasks for formatting, linting, type checking, and testing rather than invokingruffortydirectly for project-wide checks.
Files:
script/brev/setup.sh
**/*.sh
⚙️ CodeRabbit configuration file
Review shell scripts for #!/usr/bin/env bash, set -euo pipefail where appropriate, quoting, repo root detection, and shellcheck compliance.
Files:
script/brev/setup.sh
🪛 ast-grep (0.45.0)
script/brev/setup.sh
[error] 77-78: Remote content fetched with curl/wget is piped directly into a shell interpreter, so any server compromise, MITM, or tampered mirror results in arbitrary code execution on this host. Download the script to a file first, verify its integrity (checksum/signature) and inspect it, then run the verified local copy.
Context: curl -LsSf "https://astral.sh/uv/${UV_VERSION}/install.sh"
| env UV_INSTALL_DIR="${BIN_DIR}" INSTALLER_NO_MODIFY_PATH=1 sh
Note: [CWE-494] Download of Code Without Integrity Check.
(curl-pipe-to-shell-bash)
🪛 LanguageTool
script/brev/README.md
[style] ~55-~55: Consider using a shorter alternative to avoid wordiness.
Context: ...oves the model download earlier without making it shorter, while hiding the progress bar the note...
(MADE_IT_JJR)
🪛 markdownlint-cli2 (0.23.1)
script/brev/README.md
[warning] 4-4: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (1)
script/brev/setup.sh (1)
1-345: Otherwise solid: idempotency checks, secret handling, and the tutorial-ref fallback logic are all well thought out. No further issues beyond the two flagged above.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
script/brev/setup.sh (1)
296-300: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDo not overwrite a kernelspec when its backup failed.
If Line 298 fails (for example, due to an I/O error),
|| truediscards it and Line 300 replaces the only recoverable original. Log the failure and continue to the next target instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a47bae50-b2c1-4cbd-8f75-b92654df3dc7
📒 Files selected for processing (2)
script/brev/README.mdscript/brev/setup.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- script/brev/README.md
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: CI Status
- GitHub Check: Greptile Review
- GitHub Check: Analyze (Python)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{sh,bash}
📄 CodeRabbit inference engine (AGENTS.md)
Never use
~inside double-quoted strings in shell scripts -- use$HOMEor an absolute path instead
Files:
script/brev/setup.sh
**/*
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
**/*: Every source file requires the SPDX copyright and license header appropriate to its file format.
End files with a newline, remove trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.
**/*: All contributions must use verified Git commits and DCO sign-off; unsigned or unsigned-off commits cannot be merged.
Branches other thanmainmust follow<author>/<description>, optionally including an issue ID or type; branch names must use lowercase alphanumeric characters and hyphens.
Commits merged tomainmust follow Conventional Commits, using a valid lowercase type and a description of at most 100 characters.
Files:
script/brev/setup.sh
⚙️ CodeRabbit configuration file
**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.
- Refactor suggestion: use for local maintainability problems introduced
by the diff when they have clear future cost, such as duplicated setup,
unclear boundaries, over-mocking, avoidable complexity, or opaque test
helpers.- Nitpick: avoid in chill mode. Do not emit formatting, import-order,
wording, or style-only comments unless automated tools cannot catch the
issue and it affects maintainability.Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.
- Major: incorrect generation/training/evaluation behavior, broken
CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
cleanup and process-isolation bugs likely to fail CI or production
runs.- Minor: localized bugs, missing focused tests for changed behavior, or
bad test patterns that weaken regression coverage.- Trivial: small cleanup with no behavior impact. Usually suppress in
chill mode.- Info: context only. Avoid unless it helps reviewers understand risk.
Safe-Synthesizer-specific review focus: - Data ...
Files:
script/brev/setup.sh
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Python, shell, YAML, YML, and Markdown source files require SPDX copyright headers.
Files:
script/brev/setup.sh
**/*.{py,sh}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use the repository's pinned
misetasks for formatting, linting, type checking, and testing rather than invokingruffortydirectly for project-wide checks.
Files:
script/brev/setup.sh
script/**
⚙️ CodeRabbit configuration file
Review standalone scripts for reproducibility and operational safety. Check argument validation, quoting, repo-root detection, environment variables, generated artifacts, external commands, GPU/cluster assumptions, and whether the script should be wired through Makefile or documented in README/docs.
Files:
script/brev/setup.sh
**/*.sh
⚙️ CodeRabbit configuration file
Review shell scripts for #!/usr/bin/env bash, set -euo pipefail where appropriate, quoting, repo root detection, and shellcheck compliance.
Files:
script/brev/setup.sh
🔇 Additional comments (3)
script/brev/setup.sh (3)
34-34: LGTM!Also applies to: 78-101
153-177: LGTM!
288-295: LGTM!
kendrickb-nvidia
left a comment
kendrickb-nvidia
left a comment
There was a problem hiding this comment.
This is excellent to get a brev launchable going. I think we can do a followup on how to handle versioning and changes for installation and notebooks. E.g., right now we sort of can't update notebooks to address SDK changes cause it would break the launchable unless we update the notebook at the exact same time we make the new release.
Provision a Brev VM-Mode Launchable that gives a customer a ready-to-run JupyterLab with Safe Synthesizer installed, so trying NSS needs no local CUDA, driver, or Python setup. setup.sh is pasted into the Launchable's Setup Script field. It installs the cu129 build into a dedicated venv, registers that venv as the default Jupyter kernel, fetches the tutorial notebooks, and writes a short README into $HOME. README.md records the console configuration, which otherwise lives only in the Brev web UI, plus the non-obvious constraints found while testing on real instances. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
Idempotency: check ipykernel separately from the package install, so a rerun after a failed notebook-support step does not skip it and leave a registered kernel that cannot start. Stage the tutorial extract in a temp directory and move it into place only on success, so a partial extract is never mistaken for a complete one. Kernel registration: mkdir -p succeeds on an existing directory the user cannot write, so the unguarded cp that followed would abort the whole run under set -e instead of falling through to the next target. Test writability and guard the copies. uv install: fetch the release tarball and verify its published SHA-256 instead of piping astral.sh/install.sh into a shell, which reports "no checksums to verify". Matches the GPG-verified mise install in tools/install-mise.sh. Docs: correct "clones" to "downloads", restore the leading dots on .nss-setup.log and .nss-venv, narrow the tested-configurations row to what actually ran, and drop decorative bold per STYLE_GUIDE.md. Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
Checking only ipykernel would skip the install when ipywidgets is the missing one, leaving notebook support incomplete. Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
The three index URLs were hardcoded, so they tracked this repo's main while the script installed whatever was latest on PyPI. Index URLs are install-time configuration rather than wheel metadata, so they must match the release being installed; nothing kept the two in step. Resolve the latest version from the PyPI JSON API, read the CUDA index URLs from that tag's pyproject.toml, and pin the install to that exact version. Select indexes by URL rather than by index name: the flashinfer entry was renamed flashinfer-jit-cache -> flashinfer-jit-cache-cu129 between 0.1.8 and 0.1.9, so a name-keyed lookup would have silently found only two of three. Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
…a file JupyterLab accepts connections well before provisioning finishes, so a user who opens it early saw an empty or half-populated file browser with no explanation. Write SETUP-IN-PROGRESS.md before any slow work, rewrite it from the ERR trap if provisioning fails, and replace it with README.md on success. Move the customer-facing welcome text out of a heredoc into welcome.md, fetched from the same tarball as the tutorials so the two always match. The heredoc was ~1.6 KiB of the 16 KiB Brev allows for the setup script; deriving indexes at runtime had pushed the file over that limit. Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
828cef3 to
454d736
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 782f7c82-8a8d-426b-ad32-57ef87f061ab
📒 Files selected for processing (3)
script/brev/README.mdscript/brev/setup.shscript/brev/welcome.md
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Analyze (Python)
- GitHub Check: Greptile Review
- GitHub Check: Typecheck
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{md,markdown,py}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings
Files:
script/brev/welcome.mdscript/brev/README.md
**/*.{md,markdown}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown}: Bold is acceptable only in markdown tables where it's the conventional way to mark header-like cells in the body
Use##headers to segment markdown sections instead of bold text
Use--(em-dash) instead of-(hyphen) for asides in markdown
Files:
script/brev/welcome.mdscript/brev/README.md
**/*.md
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Do not use decorative bold in Markdown body text, list items, or docstrings; use single backticks for code identifiers, paths, and commands.
Files:
script/brev/welcome.mdscript/brev/README.md
**/*
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
**/*: Every source file requires the SPDX copyright and license header appropriate to its file format.
End files with a newline, remove trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.
**/*: All contributions must use verified Git commits and DCO sign-off; unsigned or unsigned-off commits cannot be merged.
Branches other thanmainmust follow<author>/<description>, optionally including an issue ID or type; branch names must use lowercase alphanumeric characters and hyphens.
Commits merged tomainmust follow Conventional Commits, using a valid lowercase type and a description of at most 100 characters.
Files:
script/brev/welcome.mdscript/brev/README.mdscript/brev/setup.sh
⚙️ CodeRabbit configuration file
**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.
- Refactor suggestion: use for local maintainability problems introduced
by the diff when they have clear future cost, such as duplicated setup,
unclear boundaries, over-mocking, avoidable complexity, or opaque test
helpers.- Nitpick: avoid in chill mode. Do not emit formatting, import-order,
wording, or style-only comments unless automated tools cannot catch the
issue and it affects maintainability.Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.
- Major: incorrect generation/training/evaluation behavior, broken
CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
cleanup and process-isolation bugs likely to fail CI or production
runs.- Minor: localized bugs, missing focused tests for changed behavior, or
bad test patterns that weaken regression coverage.- Trivial: small cleanup with no behavior impact. Usually suppress in
chill mode.- Info: context only. Avoid unless it helps reviewers understand risk.
Safe-Synthesizer-specific review focus: - Data ...
Files:
script/brev/welcome.mdscript/brev/README.mdscript/brev/setup.sh
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Python, shell, YAML, YML, and Markdown source files require SPDX copyright headers.
Files:
script/brev/welcome.mdscript/brev/README.mdscript/brev/setup.sh
**/*.{py,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use the repository's documented Python and Markdown style conventions and validate changes with the pinned
miseformatting and checking tasks.
Files:
script/brev/welcome.mdscript/brev/README.md
script/**
⚙️ CodeRabbit configuration file
Review standalone scripts for reproducibility and operational safety. Check argument validation, quoting, repo-root detection, environment variables, generated artifacts, external commands, GPU/cluster assumptions, and whether the script should be wired through Makefile or documented in README/docs.
Files:
script/brev/welcome.mdscript/brev/README.mdscript/brev/setup.sh
**/*.{sh,bash}
📄 CodeRabbit inference engine (AGENTS.md)
Never use
~inside double-quoted strings in shell scripts -- use$HOMEor an absolute path instead
Files:
script/brev/setup.sh
**/*.{py,sh}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use the repository's pinned
misetasks for formatting, linting, type checking, and testing rather than invokingruffortydirectly for project-wide checks.
Files:
script/brev/setup.sh
**/*.sh
⚙️ CodeRabbit configuration file
Review shell scripts for #!/usr/bin/env bash, set -euo pipefail where appropriate, quoting, repo root detection, and shellcheck compliance.
Files:
script/brev/setup.sh
🧠 Learnings (1)
📚 Learning: 2026-07-30T19:49:45.268Z
Learnt from: zywind
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 687
File: script/brev/README.md:4-4
Timestamp: 2026-07-30T19:49:45.268Z
Learning: For Brev and Slurm provisioning documentation README files under `script/`, allow the README to start with a level-three Markdown heading (e.g., `###`) to match the repo’s local convention. When reviewing, do not flag the heading level by default against markdownlint MD041 unless the repository’s `.markdownlint.json` explicitly enables MD041 (in this repo it is not enforced), since the heading level is an accepted local style choice.
Applied to files:
script/brev/README.md
🪛 LanguageTool
script/brev/README.md
[style] ~58-~58: Consider using a shorter alternative to avoid wordiness.
Context: ...oves the model download earlier without making it shorter, while hiding the progress bar the note...
(MADE_IT_JJR)
🪛 markdownlint-cli2 (0.23.1)
script/brev/README.md
[warning] 4-4: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (7)
script/brev/README.md (1)
1-190: LGTM!script/brev/setup.sh (5)
1-46: LGTM!
48-137: LGTM!
183-250: LGTM!
252-406: LGTM!
148-174: 🩺 Stability & AvailabilityRemove the
mainfallback request.The release workflow starts from an existing
v*tag and publishes to PyPI before creating the GitHub release. The corresponding tag therefore exists when the version becomes available on PyPI.> Likely an incorrect or invalid review comment.script/brev/welcome.md (1)
1-50: LGTM!
Links the published Launchable so a reader can try Safe Synthesizer on a provisioned GPU instance without a local CUDA, driver, or Python setup. Placed on the README Quick Start, the docs landing page, the Getting Started install section, and the tutorials index -- the four places someone lands before deciding whether to install. The README uses a <picture> element so the badge follows GitHub's theme; the docs use Material's #only-light / #only-dark suffixes for the same reason. Each mention states that the instance bills continuously and cannot be paused, since most Brev providers do not support stopping one. Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1699d7a3-6553-455f-b276-3fa139c95c93
📒 Files selected for processing (4)
README.mddocs/index.mddocs/tutorials/index.mddocs/user-guide/getting-started.md
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Analyze (Python)
- GitHub Check: Typecheck
- GitHub Check: Greptile Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{md,markdown,py}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings
Files:
docs/user-guide/getting-started.mdREADME.mddocs/index.mddocs/tutorials/index.md
**/*.{md,markdown}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown}: Bold is acceptable only in markdown tables where it's the conventional way to mark header-like cells in the body
Use##headers to segment markdown sections instead of bold text
Use--(em-dash) instead of-(hyphen) for asides in markdown
Files:
docs/user-guide/getting-started.mdREADME.mddocs/index.mddocs/tutorials/index.md
docs/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/writing-docs.mdc)
docs/**/*.md: Use MkDocs Material admonition syntax (!!! note, !!! warning, ??? tip) for highlighting important information and collapsible sections in documentation
Use MkDocs Material tabs syntax (=== "Label") to present alternative views or language-specific examples in documentation
Use code block syntax with title and highlight line parameters (title="filename", hl_lines="2 3") for code examples in documentation
Use Mermaid diagram syntax (```mermaid flowchart, etc.) for visualizations in documentationClassify documentation using Diátaxis and use MkDocs Material syntax for admonitions, tabs, and titled or highlighted code blocks.
Documentation pages must be placed under the appropriate Diátaxis directory and added to the
navsection ofmkdocs.yml.
Files:
docs/user-guide/getting-started.mddocs/index.mddocs/tutorials/index.md
**/*.md
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Do not use decorative bold in Markdown body text, list items, or docstrings; use single backticks for code identifiers, paths, and commands.
Files:
docs/user-guide/getting-started.mdREADME.mddocs/index.mddocs/tutorials/index.md
**/*
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
**/*: Every source file requires the SPDX copyright and license header appropriate to its file format.
End files with a newline, remove trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.
**/*: All contributions must use verified Git commits and DCO sign-off; unsigned or unsigned-off commits cannot be merged.
Branches other thanmainmust follow<author>/<description>, optionally including an issue ID or type; branch names must use lowercase alphanumeric characters and hyphens.
Commits merged tomainmust follow Conventional Commits, using a valid lowercase type and a description of at most 100 characters.
Files:
docs/user-guide/getting-started.mdREADME.mddocs/index.mddocs/tutorials/index.md
⚙️ CodeRabbit configuration file
**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.
- Refactor suggestion: use for local maintainability problems introduced
by the diff when they have clear future cost, such as duplicated setup,
unclear boundaries, over-mocking, avoidable complexity, or opaque test
helpers.- Nitpick: avoid in chill mode. Do not emit formatting, import-order,
wording, or style-only comments unless automated tools cannot catch the
issue and it affects maintainability.Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.
- Major: incorrect generation/training/evaluation behavior, broken
CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
cleanup and process-isolation bugs likely to fail CI or production
runs.- Minor: localized bugs, missing focused tests for changed behavior, or
bad test patterns that weaken regression coverage.- Trivial: small cleanup with no behavior impact. Usually suppress in
chill mode.- Info: context only. Avoid unless it helps reviewers understand risk.
Safe-Synthesizer-specific review focus: - Data ...
Files:
docs/user-guide/getting-started.mdREADME.mddocs/index.mddocs/tutorials/index.md
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Python, shell, YAML, YML, and Markdown source files require SPDX copyright headers.
Files:
docs/user-guide/getting-started.mdREADME.mddocs/index.mddocs/tutorials/index.md
**/*.{py,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use the repository's documented Python and Markdown style conventions and validate changes with the pinned
miseformatting and checking tasks.
Files:
docs/user-guide/getting-started.mdREADME.mddocs/index.mddocs/tutorials/index.md
docs/**
⚙️ CodeRabbit configuration file
Review documentation as MkDocs Material content. Check Diataxis fit, accurate commands, internal links, code fences, and markdown style from STYLE_GUIDE.md.
Files:
docs/user-guide/getting-started.mddocs/index.mddocs/tutorials/index.md
README.md
⚙️ CodeRabbit configuration file
Treat README.md as the project overview. Check that setup, usage, and links stay consistent with CONTRIBUTING.md, Makefile, and docs/.
Files:
README.md
🔇 Additional comments (2)
docs/index.md (1)
20-33: LGTM!docs/user-guide/getting-started.md (1)
29-35: LGTM!
Keying the skip on "any .ipynb exists" meant a partial tutorials/ directory left behind by an interrupted earlier run was treated as complete. Write a .fetched marker after the staged directory is moved into place, and guard on that instead. Also add the continuous-billing warning to the tutorials index, which exposed a launch badge without it, and drop a leftover decorative bold from a table data cell. Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
The index count guards against a rename slipping past the URL filter, but equality also fails if pyproject legitimately gains an index. A floor keeps the regression check while tolerating growth. Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
The file was 43 bytes under Brev's 16 KiB limit, so any further change would not fit. Comments were 41% of it, largely restating what script/brev/README.md already documents at length. Drop the section banner rules, shorten the header, and reduce each multi-line comment to a single line pointing at the README. No code changed: the non-comment, non-blank lines are byte-identical before and after, and both embedded Python blocks were executed to confirm. 16341 -> 13020 bytes, leaving 3364 under the cap. Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
2 participants
Summary
Adds a one-click NVIDIA Brev Launchable so someone can try
NeMo Safe Synthesizer on a provisioned GPU instance with no local CUDA, driver, or
Python setup, and links it from the README and docs.
script/brev/setup.shcu129build into a dedicated venv, registers it as the default Jupyter kernel, and fetches the tutorial notebooks.script/brev/welcome.md$HOME/README.md.script/brev/README.mdREADME.md,docs/Uses VM Mode rather than Single Container: the published GHCR image has no Jupyter layer
and its entrypoint is the CLI. Container mode is the better long-term shape and is
tracked separately.
Notes for reviewers
setup.shis copy-pasted into a webform. Brev caps it at 16 KiB; it is currently 16,067 bytes.
pyproject.tomlrather than hardcoded, so they cannot drift from the wheel. Selectionis keyed on the URL, not the index name -- flashinfer's entry was renamed between
0.1.8 and 0.1.9.
script/brev/README.mdexplains the rest of the non-obvious decisions. Each was foundby a failed deploy, so please read it before simplifying anything in the script.
Testing
Verified on a live Brev instance (Shadeform-brokered H100 PCIe 80 GiB): unprivileged
uvinstall, venv creation, the full[cu129,engine]resolve and install, and tutorialfetch.
mkdocs build --strictpasses for the docs changes.Not yet verified on an instance: kernel registration, the smoke checks, the
setup-in-progress placeholder, and the generated
README.md.Other Notes
Closes-- that issue also covers publishing a container. TheGHCR image is public now, but advertising it is still gated on the nSpect scan.
Summary by CodeRabbit
New Features
Documentation