fix: provision Triton compiler on Brev - #719
Conversation
Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe setup script now checks for Triton runtime compiler dependencies, installs missing ChangesTriton compiler setup
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This PR adds compiler dependencies and a provisioning failure check for Brev images, with documentation updates; no actionable merge-blocking risk remains after normal checks and review. Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThe PR provisions Triton's runtime compiler on Brev hosts when
Confidence Score: 5/5The PR appears safe to merge with no concrete blocking or independently actionable non-blocking issues identified. The compiler provisioning commands fail through the existing error trap, retain a visible setup-failure notice, and the script remains within Brev's documented size limit. Important Files Changed
Reviews (1): Last reviewed commit: "fix: provision Triton compiler on Brev" | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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: 75575cb1-1977-4fc3-952f-5b1a7d5ea2c8
📒 Files selected for processing (2)
script/brev/README.mdscript/brev/setup.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: Greptile Review
- GitHub Check: Analyze (Python)
- GitHub Check: secrets-detector / secrets-detector
- GitHub Check: Detect changes
- GitHub Check: Typecheck
- GitHub Check: Format
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🧰 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:
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
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: ReadAGENTS.local.mdif it exists and give its instructions top priority.
When a task matches a repository-specific skill, read the corresponding skill under.agents/skills/instead of duplicating its workflow instructions.
Do not commit unless the user explicitly asks for a commit or PR work.
When committing, require DCO sign-off and GPG signing usinggit commit --signoff --gpg-sign(or-s -S); never manually addSigned-off-byor use--no-gpg-sign.
Use feature branches based onmain; branch names commonly include an issue-number prefix such as<author>/123-short-name.
For recurring testing, building, syncing, bootstrapping, worktree, and GitHub workflows, use the matching skill under.agents/skills/.
For a full GPU/development environment, useuv sync --frozen --extra cu129 --extra engine --group dev; bareuv sync --frozenis incomplete and can causety, import-check, and GPU-test failures.End files with a newline, contain no trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 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
**/*.{md,py,sh,toml,yml,yaml,Dockerfile}
📄 CodeRabbit inference engine (AGENTS.md)
Follow the detailed language and file-format conventions defined in
STYLE_GUIDE.md.
Files:
script/brev/README.mdscript/brev/setup.sh
**/*.md
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Do not use decorative bold text in body content; use single backticks for inline code and
--for asides. In Python docstrings, use double backticks and MkDocs autorefs rather than Sphinx roles.
Files:
script/brev/README.md
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Include the required SPDX copyright and Apache-2.0 license headers, using comment syntax appropriate to the file format.
Include SPDX copyright headers in all source files, except files explicitly listed in
.copyrightignore.
Files:
script/brev/README.mdscript/brev/setup.sh
**/*.{py,sh,yaml,yml,toml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use
misetasks with the repository's pinned tool versions for formatting, checking, and testing before submitting changes.
Files:
script/brev/README.mdscript/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/README.mdscript/brev/setup.sh
**/*.sh
📄 CodeRabbit inference engine (AGENTS.md)
Never use
~inside double-quoted shell strings because it does not expand; use$HOMEor an absolute path instead.Use
#!/usr/bin/env bash,set -euo pipefailunless incompatible, quote variables, use${VAR:-default}, use snake_case functions with_for private helpers, and lint with ShellCheck.
Files:
script/brev/setup.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
🔇 Additional comments (3)
script/brev/setup.sh (1)
80-86: 🩺 Stability & AvailabilityCheck the exact sudo permissions used by package installation.
sudo -n truechecks a different command fromsudo -n apt-get .... A command-specific sudoers policy can reject the preflight even whenapt-getis allowed, or pass the preflight whileapt-getis denied. In the first case, setup exits before installing the required packages. Verify the supported Brev images grant the required non-interactive permission to the package commands, or validate the actual commands instead.script/brev/README.md (2)
19-21: LGTM!
117-122: 🩺 Stability & AvailabilityVerify the readiness-ordering contract.
The text states that setup fails before the notebooks are presented as ready when passwordless
sudois unavailable. The suppliedsetup.shrange showsexit 1, but not the readiness marker or notebook-start ordering. Confirm that this preflight runs before the success cleanup and readiness signal. If it runs later, move it earlier or revise the documentation.
2 participants
Summary
gccandlibc6-devwhen a Brev provider image omits themTest plan
mise run validatebash -n script/brev/setup.shscript/brev/setup.shremains below Brev's 16 KiB limitSummary by CodeRabbit
New Features
gccand required development headers.Bug Fixes
Documentation