build: generate CUDA dependency metadata from one source - #655
Conversation
github-actions
Bot
added
area:dev-ex
|
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:
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 (5)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
🧰 Additional context used📓 Path-based instructions (11).agents/skills/**📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
Files:
.agents/skills/*/SKILL.md📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
cuda_deps.toml📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*📄 CodeRabbit inference engine (AGENTS.md)
Files:
⚙️ CodeRabbit configuration file
Files:
**/*.toml📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{md,markdown,py}📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
Files:
**/*.{py,pyi}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{py,sh,yaml,yml,md}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{py,md}📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Files:
tools/**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📓 Common learnings🔇 Additional comments (3)
WalkthroughThe PR adds ChangesCUDA Metadata Generation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| return spec.as_pepstr(self) | ||
| raise TypeError(f"Unsupported dependency entry {dependency!r}") | ||
|
|
||
| def applies(self, dependency: DependencyEntry) -> bool: |
There was a problem hiding this comment.
Resolved at current head. render() returns for both supported entry types and raises TypeError otherwise; applies() returns explicitly from every match arm. mise run check passes.
coderabbitai
Bot
added
feature
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR introduces
Confidence Score: 5/5Safe to merge; the generator is well-tested, idempotent, and resolves prior-thread concerns. Two minor documentation/config observations do not affect runtime correctness. The generation, splicing, marker-stripping, and check-mode logic are all covered by an extensive test suite including idempotency and no-op write guard tests. The flashinfer multi-variant name-collision concern raised in a prior review thread is resolved by the {extra} template. The two observations flagged here are documentation drift and a ty negation pattern whose effectiveness is uncertain but has no impact on the generated CUDA metadata or lock file. Files Needing Attention: The !./tools/gen_cuda_deps.py negation in the ty exclude list in pyproject.toml warrants a quick smoke-check to confirm ty actually type-checks that file as intended. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @dataclass(frozen=True) | ||
| class NvidiaCudaLibrarySpec(StrictModel): | ||
| """NVIDIA CUDA package source routing metadata.""" | ||
|
|
||
| name: str = Field(description="NVIDIA CUDA library package stem without the nvidia- prefix.") | ||
| nvidia_package_suffix: str | None = Field( | ||
| default=None, | ||
| description="Optional package suffix override. Defaults to the CUDA variant suffix.", | ||
| ) | ||
| index: str | None = Field( | ||
| default=None, | ||
| description="Optional literal uv index name or template. Defaults to the variant PyTorch index.", | ||
| ) |
There was a problem hiding this comment.
@dataclass(frozen=True) applied to a Pydantic BaseModel subclass
NvidiaCudaLibrarySpec combines Python's standard @dataclass(frozen=True) with Pydantic's BaseModel, which is non-standard. Because Pydantic v2 generates __init__ directly into each model class's __dict__, the @dataclass decorator skips its own __init__ — but it still writes __setattr__/__delattr__ that raise FrozenInstanceError, bypassing Pydantic's own immutability machinery. The correct approach for frozen Pydantic models is model_config = ConfigDict(frozen=True), which is already available through StrictModel's configuration. This combination may break silently across Pydantic minor releases if that internal __init__ placement changes.
There was a problem hiding this comment.
Resolved. NvidiaCudaLibrarySpec now uses Pydantic’s frozen model configuration without @dataclass.
| def _update_pyproject( | ||
| pyproject_path: Path, | ||
| check: bool, | ||
| generated: CudaPyprojectFragment, | ||
| ) -> GenerationResult: | ||
| current = pyproject_path.read_text(encoding="utf-8") | ||
| updated = apply_cuda_fragment_to_pyproject(current, generated) | ||
| if check: | ||
| return _check_pyproject(pyproject_path, current, updated) | ||
| pyproject_path.write_text(updated, encoding="utf-8") | ||
| return GenerationResult( | ||
| status=GenStatus.ok, | ||
| message=f"Updated generated CUDA dependency sections in {pyproject_path}", | ||
| ) |
There was a problem hiding this comment.
_update_pyproject unconditionally calls pyproject_path.write_text(updated, ...) and returns "Updated generated CUDA dependency sections" even when current == updated. This means running the generator when the file is already up-to-date still modifies the mtime, can create a spurious git diff, and produces a misleading success message. A pre-check if current == updated: return GenerationResult(status=GenStatus.ok, message="... already up to date") before the write would prevent this.
There was a problem hiding this comment.
Resolved. _update_pyproject() returns without writing when the generated content is already current. Coverage now also proves stale --check mode leaves the file unchanged.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b4bffca5-fd31-4a5a-8bfe-d35cea816d23
📒 Files selected for processing (7)
.agents/skills/uv-build/SKILL.md.mise/tasks/quality.tomlCONTRIBUTING.mdcuda_deps.tomlpyproject.tomltests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Unit Tests (3.13)
- GitHub Check: Unit Tests (3.12)
- GitHub Check: Unit Tests (3.11)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{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:
CONTRIBUTING.mdtests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
**/*.{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:
CONTRIBUTING.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.
Use the repository's MkDocs Material Markdown conventions, including supported admonitions, content tabs, fenced code blocks, Mermaid diagrams, task lists, footnotes, definition lists, and emoji where appropriate.
Files:
CONTRIBUTING.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.
**/*: Never move a published release tag; if release code changes, create and validate the next release candidate instead.
A stable release tag must point to the exact tested commit SHA, not a latermaincommit.
Before publishing a release, verify the wheel, package import, CLI, dependency set, GitHub release, PyPI artifacts, and immutable container image tag.
All contributions must include a DCOSigned-off-bytrailer, and commits must also have a verified cryptographic signature.
Commits merged tomainmust follow Conventional Commits syntax with a lowercase valid type, optional scope, description of at most 100 characters, and!for breaking changes.
Before submitting a pull request, all existing tests must pass; new features require tests and bug fixes require regression tests.
Use the repository'smisetasks for development, testing, formatting, checking, documentation, and release operations instead of deprecated Makefile task commands.
Files:
CONTRIBUTING.mdcuda_deps.tomlpyproject.tomltests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
⚙️ 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:
CONTRIBUTING.mdcuda_deps.tomlpyproject.tomltests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,sh,yaml,yml,md}: All source files with.py,.sh,.yaml,.yml, or.mdextensions must include SPDX copyright headers.
Use the repository's pinnedmisetasks and formatting checks rather than relying on locally installed tool versions.
Files:
CONTRIBUTING.mdtests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
.agents/skills/**
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
Store skills in canonical location
.agents/skills/with each skill containing a SKILL.md file and optional references/
Files:
.agents/skills/uv-build/SKILL.md
**/*.toml
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Use spaces around
=, comment dependency pins inline, and follow the prescribedpyproject.tomlsection order.
Files:
cuda_deps.tomlpyproject.toml
cuda_deps.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Changes to
cuda_deps.tomlrequire regenerated CUDA metadata and lockfile validation viamise run lock-check.
Files:
cuda_deps.toml
.mise/tasks/**/*.toml
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Keep declarative Mise tasks under
.mise/tasks/, provide descriptions for public tasks, and add usage metadata where arguments need validation or help.
Files:
.mise/tasks/quality.toml
.mise/tasks/*
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Keep shared shell helpers in
.mise/tasks/_lib.shand make that file non-executable.
Files:
.mise/tasks/quality.toml
pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
Configure package metadata, dependencies, extras (cpu/cu129/engine), and uv configuration in
pyproject.tomlKeep generated CUDA metadata and the
uvlockfile synchronized when changingpyproject.tomlorcuda_deps.toml; validate drift withmise run lock-check.
Files:
pyproject.toml
⚙️ CodeRabbit configuration file
Treat pyproject.toml as high-risk. Check package metadata, uv indexes, dependency groups, optional extras, Python version bounds, hatch config, ty config, script entry points, dependency consistency, and whether changes require regenerating uv.lock.
Files:
pyproject.toml
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Place durable implementation guidance in function and class docstrings for public contracts and source comments for local invariants
Target Python 3.11–3.13 with modern syntax (X | Y,list[str],Self). Python 3.14+ is not supported
**/*.py: Use American English spelling in Python code, documentation, and messages.
UseField(description=...)for every Pydantic model field.
Use assignment-styleField()by default; useAnnotatedonly for additional metadata such as validators, constrained aliases, or discriminated unions.
Use@dataclass(frozen=True)for immutable value objects and validators; use mutable dataclasses only for builders, accumulators, and pipeline state.
Usefield(default_factory=list)instead of mutable list defaults.
UseStrEnumfor string-valued configuration or serialization enums and plainEnumfor internal constants.
Obtain loggers withobservability.get_logger(__name__); do not calllogging.getLogger()orstructlog.get_logger()directly.
Use.runtime,.user, and.systemcategory loggers appropriately.
Do not useprint()for operational library output; use the approved logger,click.echo(), orsys.stdout.write()where appropriate.
Useextra={}for machine-queryable logging data and f-strings only for human-readable context.
Raise errors from the custom Safe Synthesizer error hierarchy, using the documented dual inheritance for user and internal errors.
Keep shared package code compatible with Python 3.11; do not use Python 3.12-only syntax such as PEP 695 type statements or bracketed generic parameters.
PreferX | Y, built-in collection generics, andSelfoverOptional,Union, and legacy typing collections.
Use collection ABCs for function arguments and concrete collection types for return values.
UseProtocolfor structural subtyping and avoidAnywhenobject, generics, or protocols are suitable.
UseTYPE_CHECKINGguards for heavy imports such as pandas, torch, and transformers.
...
Files:
tests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
**/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use the
unitmarker instead of the deprecatedunit_testmarker for test identification
Files:
tests/test_gen_cuda_deps.py
tests/**
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
tests/**: Mirrorsrc/directory structure intests/directory for test organization
Auto-mark tests by directory:tests/e2e/→e2e,tests/smoke/→smoke, otherwise default tounitMirror source code directory structure in tests directory (e.g.,
tests/training/,tests/generation/parallel to source structure)
Files:
tests/test_gen_cuda_deps.py
tests/**/*.py
📄 CodeRabbit inference engine (tests/TESTING.md)
tests/**/*.py: Auto-mark tests based on file path: tests under/e2e/gete2emarker, tests under/smoke/getsmokemarker, all others getunitmarker (only if no category marker already present)
Every test should have exactly one category marker:unit,smoke, ore2e
Usepytest.mark.requires_gpumodifier on tests that need CUDA hardware
Usepytest.mark.vllmon tests using vLLM generation backend and ensure each vLLM test file runs in its own process for GPU memory isolation
Usepytest.mark.slowon long-running tests
Usepytest.mark.smollm2for SmolLM2 Hub download tests to enable process isolation
Usepytest.mark.noautouseto skip autouse fixtures for specific tests
Useload_test_dataset(filename)helper to load test datasets fromtests/stub_datasets/as HuggingFaceDatasetobjects
Useload_test_dataframe(filename)helper to load test data files fromtests/stub_datasets/as pandas DataFrames
Convert pandas columns to nullable dtypes (pd.Int64Dtype(),pd.BooleanDtype()) before assigningnp.nanvalues
Usefake.seed_instance(seed)andrandom.seed(seed)together for Faker-based test data reproducibility
When sharing methods across multiple test files, define them inconftest.pyand import them using relative imports (e.g.,from .conftest import train_with_sdk); note that importing from other test files liketests/cli/helpers.pydoes not work
Usefixture_mock_processororfixture_mock_processor_without_valid_recordsfor mocking ParsedResponse objects withvalid_records,invalid_records,errors, andprompt_numberfields
Usepytest.importorskipto gate tests on optional dependencies that require specific extras (e.g.,sentence_transformers,vllm)
Run vLLM tests with separate pytest invocations (one per file) using-n 0(single process) for GPU memory isolation, or use staged mise tasks for CI visibility
Print statements are allowed in tests (ruffT201is suppressed fortests/directory) and should...
Files:
tests/test_gen_cuda_deps.py
⚙️ CodeRabbit configuration file
Review tests against tests/TESTING.md. Check marker usage, fixture naming, tmp_path usage, determinism, and GPU/vLLM process-isolation requirements. Flag slop tests that only check that code runs, assert result is not None when stronger invariants exist, over-mock internal implementation details, patch around the bug instead of reproducing it, or add broad snapshot/golden churn without a clear contract. Flag change detector tests that fail on harmless refactors, formatting, record ordering, incidental wording, or private implementation details without demonstrating a behavior regression. Prefer existing fixtures or focused new fixtures for repeated setup; keep tests DRY when reasonable without making the behavior under test opaque. print() is allowed in tests.
Files:
tests/test_gen_cuda_deps.py
tests/test_*.py
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Name test files
test_*.py, classesTest*, and functionstest_<module>_<expected_behavior>.
Files:
tests/test_gen_cuda_deps.py
tools/**
⚙️ CodeRabbit configuration file
Review tools as developer and CI infrastructure. Check that scripts use uv or Makefile wrappers instead of ad hoc python/pip commands, preserve read-only behavior for check targets, fail with clear messages, avoid hidden network or filesystem side effects, and stay consistent with STYLE_GUIDE.md and CONTRIBUTING.md. Tooling may use print() when it is a standalone script or intentional CLI output.
Files:
tools/gen_cuda_deps.py
🧠 Learnings (1)
📚 Learning: 2026-05-27T22:20:37.354Z
Learnt from: kendrickb-nvidia
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 520
File: tests/generation/test_vllm_backend.py:556-587
Timestamp: 2026-05-27T22:20:37.354Z
Learning: In NVIDIA-NeMo/Safe-Synthesizer, `tests/conftest.py`’s `pytest_collection_modifyitems` hook applies pytest category markers automatically based on each test file’s path: tests under `/e2e/` get `pytest.mark.e2e`, tests under `/smoke/` get `pytest.mark.smoke`, and all other tests get `pytest.mark.unit`. Therefore, when reviewing pytest tests outside `tests/e2e/` and `tests/smoke/`, do not flag missing explicit `pytest.mark.unit` decorators on test classes/functions as an issue (the hook will add them during collection). If a new test directory/category is introduced, ensure the hook is updated so it’s categorized correctly.
Applied to files:
tests/test_gen_cuda_deps.py
🪛 LanguageTool
.agents/skills/uv-build/SKILL.md
[grammar] ~70-~70: Use a hyphen to join words.
Context: ...ock`. Pre-commit verifies the lock is up to date. The generated CPU/CUDA sections of...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (7)
cuda_deps.toml (1)
1-59: LGTM!Also applies to: 64-112
tools/gen_cuda_deps.py (1)
1-4: LGTM!Also applies to: 6-13, 22-223, 238-983
pyproject.toml (1)
113-244: LGTM!Also applies to: 252-331
tests/test_gen_cuda_deps.py (1)
1-3: LGTM!Also applies to: 9-149, 155-340
.mise/tasks/quality.toml (1)
24-25: LGTM!Also applies to: 27-29
.agents/skills/uv-build/SKILL.md (1)
64-74: LGTM!CONTRIBUTING.md (1)
596-596: LGTM!
Addresses PR #655 review comments: scope CPU torch/torchaudio/torchvision sources to Linux, skip rewriting an unchanged pyproject.toml, drop unused structlog dependency and dead GenStatus.error, use Pydantic's frozen model config instead of mixing in @DataClass, match the script's Python range to the repo, and run the lock-check generator call offline. Also simplifies tools/gen_cuda_deps.py: drop the PEP 723 inline-script metadata block in favor of running against the repo venv, fix a marker duplication bug in the generated-block stripping logic (surfaced while dropping that block), fold single-caller helpers into their only callers (_render_template, GeneratedBlocks' position finders), derive the uv source marker from sys_platform/arch instead of hand-duplicating it in cuda_deps.toml, and move CudaVariantUvRouter's index-resolution methods onto CudaVariantContext to remove multi-hop reaches through it. Reworks the test fixtures to use structural TOML mutation instead of brittle string-replace against a shared blob, and extracts duplicated expected-output constants. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 16e068a0-3deb-47ca-bcc3-03870e2efcf0
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (5)
.mise/tasks/quality.tomlcuda_deps.tomlpyproject.tomltests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
🚧 Files skipped from review as they are similar to previous changes (3)
- .mise/tasks/quality.toml
- pyproject.toml
- cuda_deps.toml
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: Unit Tests (3.12)
- GitHub Check: Unit Tests (3.11)
- GitHub Check: Unit Tests (3.13)
- GitHub Check: Smoke Tests
- GitHub Check: End-user Wheel Install
- GitHub Check: Greptile Review
- GitHub Check: Analyze (Python)
- 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:
tests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Place durable implementation guidance in function and class docstrings for public contracts and source comments for local invariants
Target Python 3.11–3.13 with modern syntax (X | Y,list[str],Self). Python 3.14+ is not supported
**/*.py: Use American English spelling in Python code, documentation, and messages.
UseField(description=...)for every Pydantic model field.
Use assignment-styleField()by default; useAnnotatedonly for additional metadata such as validators, constrained aliases, or discriminated unions.
Use@dataclass(frozen=True)for immutable value objects and validators; use mutable dataclasses only for builders, accumulators, and pipeline state.
Usefield(default_factory=list)instead of mutable list defaults.
UseStrEnumfor string-valued configuration or serialization enums and plainEnumfor internal constants.
Obtain loggers withobservability.get_logger(__name__); do not calllogging.getLogger()orstructlog.get_logger()directly.
Use.runtime,.user, and.systemcategory loggers appropriately.
Do not useprint()for operational library output; use the approved logger,click.echo(), orsys.stdout.write()where appropriate.
Useextra={}for machine-queryable logging data and f-strings only for human-readable context.
Raise errors from the custom Safe Synthesizer error hierarchy, using the documented dual inheritance for user and internal errors.
Keep shared package code compatible with Python 3.11; do not use Python 3.12-only syntax such as PEP 695 type statements or bracketed generic parameters.
PreferX | Y, built-in collection generics, andSelfoverOptional,Union, and legacy typing collections.
Use collection ABCs for function arguments and concrete collection types for return values.
UseProtocolfor structural subtyping and avoidAnywhenobject, generics, or protocols are suitable.
UseTYPE_CHECKINGguards for heavy imports such as pandas, torch, and transformers.
...
Files:
tests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
**/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use the
unitmarker instead of the deprecatedunit_testmarker for test identification
Files:
tests/test_gen_cuda_deps.py
tests/**
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
tests/**: Mirrorsrc/directory structure intests/directory for test organization
Auto-mark tests by directory:tests/e2e/→e2e,tests/smoke/→smoke, otherwise default tounitMirror source code directory structure in tests directory (e.g.,
tests/training/,tests/generation/parallel to source structure)
Files:
tests/test_gen_cuda_deps.py
tests/**/*.py
📄 CodeRabbit inference engine (tests/TESTING.md)
tests/**/*.py: Auto-mark tests based on file path: tests under/e2e/gete2emarker, tests under/smoke/getsmokemarker, all others getunitmarker (only if no category marker already present)
Every test should have exactly one category marker:unit,smoke, ore2e
Usepytest.mark.requires_gpumodifier on tests that need CUDA hardware
Usepytest.mark.vllmon tests using vLLM generation backend and ensure each vLLM test file runs in its own process for GPU memory isolation
Usepytest.mark.slowon long-running tests
Usepytest.mark.smollm2for SmolLM2 Hub download tests to enable process isolation
Usepytest.mark.noautouseto skip autouse fixtures for specific tests
Useload_test_dataset(filename)helper to load test datasets fromtests/stub_datasets/as HuggingFaceDatasetobjects
Useload_test_dataframe(filename)helper to load test data files fromtests/stub_datasets/as pandas DataFrames
Convert pandas columns to nullable dtypes (pd.Int64Dtype(),pd.BooleanDtype()) before assigningnp.nanvalues
Usefake.seed_instance(seed)andrandom.seed(seed)together for Faker-based test data reproducibility
When sharing methods across multiple test files, define them inconftest.pyand import them using relative imports (e.g.,from .conftest import train_with_sdk); note that importing from other test files liketests/cli/helpers.pydoes not work
Usefixture_mock_processororfixture_mock_processor_without_valid_recordsfor mocking ParsedResponse objects withvalid_records,invalid_records,errors, andprompt_numberfields
Usepytest.importorskipto gate tests on optional dependencies that require specific extras (e.g.,sentence_transformers,vllm)
Run vLLM tests with separate pytest invocations (one per file) using-n 0(single process) for GPU memory isolation, or use staged mise tasks for CI visibility
Print statements are allowed in tests (ruffT201is suppressed fortests/directory) and should...
Files:
tests/test_gen_cuda_deps.py
⚙️ CodeRabbit configuration file
Review tests against tests/TESTING.md. Check marker usage, fixture naming, tmp_path usage, determinism, and GPU/vLLM process-isolation requirements. Flag slop tests that only check that code runs, assert result is not None when stronger invariants exist, over-mock internal implementation details, patch around the bug instead of reproducing it, or add broad snapshot/golden churn without a clear contract. Flag change detector tests that fail on harmless refactors, formatting, record ordering, incidental wording, or private implementation details without demonstrating a behavior regression. Prefer existing fixtures or focused new fixtures for repeated setup; keep tests DRY when reasonable without making the behavior under test opaque. print() is allowed in tests.
Files:
tests/test_gen_cuda_deps.py
tests/test_*.py
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Name test files
test_*.py, classesTest*, and functionstest_<module>_<expected_behavior>.
Files:
tests/test_gen_cuda_deps.py
**/*
📄 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.
**/*: Commits merged tomainmust use Conventional Commits format:<type>(<scope>): <description>or<type>: <description>, with a valid lowercase type and a description of at most 100 characters.
All contributions must include a DCOSigned-off-bytrailer, and commits must also have a verified cryptographic signature.
Branches other thanmainmust follow the lowercase<author>/<description>convention, optionally including an issue ID and one of the approved type prefixes.
Before submitting a pull request, run the repository's formatting, quality checks, and tests (mise run format,mise run check, andmise run test).
Files:
tests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
⚙️ 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:
tests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Python, shell, YAML, YML, and Markdown source files must include SPDX copyright headers, except files listed in
.copyrightignore.
Files:
tests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
tools/**
⚙️ CodeRabbit configuration file
Review tools as developer and CI infrastructure. Check that scripts use uv or Makefile wrappers instead of ad hoc python/pip commands, preserve read-only behavior for check targets, fail with clear messages, avoid hidden network or filesystem side effects, and stay consistent with STYLE_GUIDE.md and CONTRIBUTING.md. Tooling may use print() when it is a standalone script or intentional CLI output.
Files:
tools/gen_cuda_deps.py
🧠 Learnings (1)
📚 Learning: 2026-05-27T22:20:37.354Z
Learnt from: kendrickb-nvidia
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 520
File: tests/generation/test_vllm_backend.py:556-587
Timestamp: 2026-05-27T22:20:37.354Z
Learning: In NVIDIA-NeMo/Safe-Synthesizer, `tests/conftest.py`’s `pytest_collection_modifyitems` hook applies pytest category markers automatically based on each test file’s path: tests under `/e2e/` get `pytest.mark.e2e`, tests under `/smoke/` get `pytest.mark.smoke`, and all other tests get `pytest.mark.unit`. Therefore, when reviewing pytest tests outside `tests/e2e/` and `tests/smoke/`, do not flag missing explicit `pytest.mark.unit` decorators on test classes/functions as an issue (the hook will add them during collection). If a new test directory/category is introduced, ensure the hook is updated so it’s categorized correctly.
Applied to files:
tests/test_gen_cuda_deps.py
🔇 Additional comments (2)
tools/gen_cuda_deps.py (1)
1-42: LGTM!Also applies to: 171-223, 252-540, 761-783, 899-937
tests/test_gen_cuda_deps.py (1)
33-204: LGTM!Also applies to: 235-369
mckornfield
left a comment
mckornfield
left a comment
There was a problem hiding this comment.
so I believe I reviewed this at one point, and though I'm sure it's changed a bit, felt like that other version was gtg. Might be worth addressing the CR reviews, though they're not that great from what I can see lol
| updated = generator.apply_cuda_fragment_to_pyproject(PYPROJECT, generated) | ||
| parsed = tomllib.loads(updated) | ||
|
|
||
| assert "# >>> BEGIN GENERATED CUDA RUNTIME EXTRAS - DO NOT EDIT <<<" in updated |
There was a problem hiding this comment.
lol these assertions are fun
There was a problem hiding this comment.
They earned their keep here. I added more exact convergence and formatting coverage while wiring in dprint.
zywind
left a comment
zywind
left a comment
There was a problem hiding this comment.
I wonder if there's a way to not generate pyproject.toml. After all, the lock file is supposed to be the generated version.
| [ | ||
| {extra = "cpu"}, | ||
| {extra = "cu129"}, | ||
| ], |
There was a problem hiding this comment.
NIT: The indentation here is a bit weird. Given that many of the changes here are formatting changes, it's worth adding a toml formatter
There was a problem hiding this comment.
yeah, it's time. i'm adding dprint for toml; we can use it for more if we want later.
There was a problem hiding this comment.
Implemented in 7cf5c589. dprint’s TOML plugin is versioned and checksummed, dprint fmt and dprint check run through the existing mise format/check tasks, and all tracked TOML is normalized. mise run validate passes.
coderabbitai
Bot
added
refactor
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: ff32eaac-15aa-4b90-a7de-3d7288e87e5f
📒 Files selected for processing (18)
.agents/skills/uv-build/SKILL.md.github/workflows/README.md.mise.toml.mise/tasks/docs.toml.mise/tasks/quality.toml.mise/tasks/setup.toml.mise/tasks/tests.tomlAGENTS.mdCONTRIBUTING.mdREADME.mdSTYLE_GUIDE.mdcuda_deps.tomldocs/developer-guide/docker.mddprint.jsonpyproject.tomlruff.tomltests/test_gen_cuda_deps.pytools/gen_cuda_deps.py
🚧 Files skipped from review as they are similar to previous changes (4)
- .agents/skills/uv-build/SKILL.md
- .mise/tasks/quality.toml
- cuda_deps.toml
- tools/gen_cuda_deps.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (20)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: IfAGENTS.local.mdexists, read it and give its instructions top priority.
Do not commit changes unless the user 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.End files with a newline, remove trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.
Files:
dprint.jsonAGENTS.mdSTYLE_GUIDE.mdREADME.mddocs/developer-guide/docker.mdCONTRIBUTING.mdpyproject.tomlruff.tomltests/test_gen_cuda_deps.py
⚙️ 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:
dprint.jsonAGENTS.mdSTYLE_GUIDE.mdREADME.mddocs/developer-guide/docker.mdCONTRIBUTING.mdpyproject.tomlruff.tomltests/test_gen_cuda_deps.py
**/*.{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:
AGENTS.mdSTYLE_GUIDE.mdREADME.mddocs/developer-guide/docker.mdCONTRIBUTING.mdtests/test_gen_cuda_deps.py
**/*.{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:
AGENTS.mdSTYLE_GUIDE.mdREADME.mddocs/developer-guide/docker.mdCONTRIBUTING.md
AGENTS.md
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
Maintain agent guide with module map and conventions in
AGENTS.md
Files:
AGENTS.md
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All source files with extensions
.py,.sh,.yaml,.yml, and.mdrequire SPDX copyright headers.Every source file requires the repository SPDX copyright and license header, using hash comments for Python, shell, and YAML and HTML comments or frontmatter comments for Markdown.
Files:
AGENTS.mdSTYLE_GUIDE.mdREADME.mddocs/developer-guide/docker.mdCONTRIBUTING.mdtests/test_gen_cuda_deps.py
**/*.md
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Do not use decorative bold in body text; use single backticks for identifiers, paths, and commands, and use
--for asides.
Files:
AGENTS.mdSTYLE_GUIDE.mdREADME.mddocs/developer-guide/docker.mdCONTRIBUTING.md
.github/**
⚙️ CodeRabbit configuration file
Review GitHub configuration for branch protection expectations, CODEOWNERS alignment, least privilege permissions, pinned actions where practical, and consistency with CONTRIBUTING.md.
Files:
.github/workflows/README.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
.mise/tasks/**/*.toml
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Keep declarative mise tasks under
.mise/tasks/, providedescriptionfor public TOML tasks, and useusagefor arguments requiring validation or help.
Files:
.mise/tasks/setup.toml.mise/tasks/docs.toml.mise/tasks/tests.toml
.mise/tasks/*
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Use
#MISE description=...and#USAGEin public file tasks; place shared non-executable shell helpers in.mise/tasks/_lib.sh.
Files:
.mise/tasks/setup.toml.mise/tasks/docs.toml.mise/tasks/tests.toml
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 documentationDocumentation pages must be placed under the appropriate
docs/Diataxis subdirectory and added to thenav:section ofmkdocs.yml.Classify documentation pages using Diátaxis and use MkDocs Material syntax such as admonitions, tabs, and titled or highlighted code blocks.
Files:
docs/developer-guide/docker.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/developer-guide/docker.md
pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
Configure package metadata, dependencies, extras (cpu/cu129/engine), and uv configuration in
pyproject.tomlNever hand-edit generated
# >>> BEGIN GENERATED ... <<<blocks; modifycuda_deps.toml, regenerate withtools/gen_cuda_deps.py, then runuv lock.Keep generated CUDA metadata and the uv lock file synchronized; lock drift is checked when
pyproject.tomlorcuda_deps.tomlchanges.
Files:
pyproject.toml
⚙️ CodeRabbit configuration file
Treat pyproject.toml as high-risk. Check package metadata, uv indexes, dependency groups, optional extras, Python version bounds, hatch config, ty config, script entry points, dependency consistency, and whether changes require regenerating uv.lock.
Files:
pyproject.toml
**/*.toml
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Format TOML with the repository formatter, use spaces around
=, four spaces for multiline arrays, and follow the prescribedpyproject.tomlsection order.
Files:
pyproject.tomlruff.toml
.mise.toml
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Keep the root task include under
[task_config].
Files:
.mise.toml
⚙️ CodeRabbit configuration file
Treat .mise.toml as toolchain supply-chain configuration. Check pinned tool choices, install cadence, platform coverage, environment settings, and whether changes require regenerating mise.lock.
Files:
.mise.toml
ruff.toml
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
Configure ruff linting and formatting rules in
ruff.toml
Files:
ruff.toml
⚙️ CodeRabbit configuration file
Review Ruff configuration against STYLE_GUIDE.md. Check selected rules, ignores, per-file ignores, Python target version, line length, and whether changes hide real defects or conflict with Makefile targets.
Files:
ruff.toml
tests/**
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
tests/**: Mirrorsrc/directory structure intests/directory for test organization
Auto-mark tests by directory:tests/e2e/→e2e,tests/smoke/→smoke, otherwise default tounitMirror source code directory structure in tests directory (e.g.,
tests/training/,tests/generation/parallel to source structure)
Files:
tests/test_gen_cuda_deps.py
tests/**/*.py
📄 CodeRabbit inference engine (tests/TESTING.md)
tests/**/*.py: Auto-mark tests based on file path: tests under/e2e/gete2emarker, tests under/smoke/getsmokemarker, all others getunitmarker (only if no category marker already present)
Every test should have exactly one category marker:unit,smoke, ore2e
Usepytest.mark.requires_gpumodifier on tests that need CUDA hardware
Usepytest.mark.vllmon tests using vLLM generation backend and ensure each vLLM test file runs in its own process for GPU memory isolation
Usepytest.mark.slowon long-running tests
Usepytest.mark.smollm2for SmolLM2 Hub download tests to enable process isolation
Usepytest.mark.noautouseto skip autouse fixtures for specific tests
Useload_test_dataset(filename)helper to load test datasets fromtests/stub_datasets/as HuggingFaceDatasetobjects
Useload_test_dataframe(filename)helper to load test data files fromtests/stub_datasets/as pandas DataFrames
Convert pandas columns to nullable dtypes (pd.Int64Dtype(),pd.BooleanDtype()) before assigningnp.nanvalues
Usefake.seed_instance(seed)andrandom.seed(seed)together for Faker-based test data reproducibility
When sharing methods across multiple test files, define them inconftest.pyand import them using relative imports (e.g.,from .conftest import train_with_sdk); note that importing from other test files liketests/cli/helpers.pydoes not work
Usefixture_mock_processororfixture_mock_processor_without_valid_recordsfor mocking ParsedResponse objects withvalid_records,invalid_records,errors, andprompt_numberfields
Usepytest.importorskipto gate tests on optional dependencies that require specific extras (e.g.,sentence_transformers,vllm)
Run vLLM tests with separate pytest invocations (one per file) using-n 0(single process) for GPU memory isolation, or use staged mise tasks for CI visibility
Print statements are allowed in tests (ruffT201is suppressed fortests/directory) and should...
Files:
tests/test_gen_cuda_deps.py
⚙️ CodeRabbit configuration file
Review tests against tests/TESTING.md. Check marker usage, fixture naming, tmp_path usage, determinism, and GPU/vLLM process-isolation requirements. Flag slop tests that only check that code runs, assert result is not None when stronger invariants exist, over-mock internal implementation details, patch around the bug instead of reproducing it, or add broad snapshot/golden churn without a clear contract. Flag change detector tests that fail on harmless refactors, formatting, record ordering, incidental wording, or private implementation details without demonstrating a behavior regression. Prefer existing fixtures or focused new fixtures for repeated setup; keep tests DRY when reasonable without making the behavior under test opaque. print() is allowed in tests.
Files:
tests/test_gen_cuda_deps.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Useuvfor Python environment management and execution; useuv runinstead of rawpythonorpip.
Support Python 3.11–3.13 and use modern syntax such asX | Y,list[str], andSelf; do not target Python 3.14+.
Run repositorymisetasks or wrapper scripts intools/instead of invokingruffortydirectly.
**/*.py: Keep Python source compatible with Python 3.11; do not use Python 3.12-only syntax such as PEP 695 type statements or bracketed generic parameters.
Use the repository's pinned Ruff and ty tooling through the mise tasks for Python formatting, linting, and type checking.
**/*.py: Use American English spelling in Python code and documentation; new code must follow the conventions even where legacy deviations remain.
Always provideField(description=...)for Pydantic model fields; use assignment-styleField()by default and useAnnotatedonly for additional metadata such as validators, reusable constraints, or discriminated unions.
Usefield(default_factory=list)rather than mutable list defaults, and prefer frozen dataclasses for immutable value objects and validators.
UseStrEnumfor string-valued enums used in configuration or serialization, and plainEnumfor internal-only named constants.
Obtain loggers withobservability.get_logger(__name__); do not calllogging.getLogger()orstructlog.get_logger()directly.
Never useprint()for operational library output; use the repository logger,click.echo()for CLI output, orsys.stdout.write()for raw tool output.
Useextra={}for structured data that downstream tools should query or aggregate, such as metrics, counts, and durations.
Raise known failures through the custom error hierarchy:SafeSynthesizerError,UserError,DataError,ParameterError,GenerationError, andInternalError, using dual inheritance where specified.
Keep shared package code compatible with Python 3.11; do not use Python 3.12-only syntax such as PEP 695 type statement...
Files:
tests/test_gen_cuda_deps.py
tests/test_*.py
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Name test files
test_*.py, classesTest*, and functionstest_<module>_<expected_behavior>; usefixture_prefixes and one-line fixture docstrings.
Files:
tests/test_gen_cuda_deps.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer
Timestamp: 2026-07-29T15:01:31.552Z
Learning: All merged commits must use Conventional Commits syntax: `<type>(<scope>): <description>` or `<type>: <description>`, with a lowercase valid type and a description of at most 100 characters.
Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer
Timestamp: 2026-07-29T15:01:31.552Z
Learning: Every contribution must include a DCO `Signed-off-by` trailer, and commits must also have a verified cryptographic signature.
Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer
Timestamp: 2026-07-29T15:01:31.552Z
Learning: Never move a published release tag; if code changes, create and validate the next release-candidate tag.
Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer
Timestamp: 2026-07-29T15:01:31.552Z
Learning: Promote a stable release tag only after candidate validation, and ensure it points to the same tested commit SHA.
Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer
Timestamp: 2026-07-29T15:01:31.552Z
Learning: Use `mise run` tasks with the pinned tool versions for formatting, checking, testing, validation, and release operations instead of invoking repository tooling directly where a mise task exists.
📚 Learning: 2026-05-27T22:20:37.354Z
Learnt from: kendrickb-nvidia
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 520
File: tests/generation/test_vllm_backend.py:556-587
Timestamp: 2026-05-27T22:20:37.354Z
Learning: In NVIDIA-NeMo/Safe-Synthesizer, `tests/conftest.py`’s `pytest_collection_modifyitems` hook applies pytest category markers automatically based on each test file’s path: tests under `/e2e/` get `pytest.mark.e2e`, tests under `/smoke/` get `pytest.mark.smoke`, and all other tests get `pytest.mark.unit`. Therefore, when reviewing pytest tests outside `tests/e2e/` and `tests/smoke/`, do not flag missing explicit `pytest.mark.unit` decorators on test classes/functions as an issue (the hook will add them during collection). If a new test directory/category is introduced, ensure the hook is updated so it’s categorized correctly.
Applied to files:
tests/test_gen_cuda_deps.py
📚 Learning: 2026-07-27T22:07:22.590Z
Learnt from: binaryaaron
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 673
File: tests/pii_replacer/test_edit.py:327-327
Timestamp: 2026-07-27T22:07:22.590Z
Learning: When tests read structured logging context from Python `logging.LogRecord` instances, don’t access `record.ctx` directly (it isn’t declared on `LogRecord` and will break static typing). Instead, use `getattr(record, "ctx", default)` (or an appropriate fallback) to safely handle cases where `ctx` may or may not be attached. This applies even if Ruff rule `B009` isn’t enabled in the repo.
Applied to files:
tests/test_gen_cuda_deps.py
🔇 Additional comments (16)
pyproject.toml (1)
12-79: LGTM!Also applies to: 171-184, 231-291, 341-342
tests/test_gen_cuda_deps.py (2)
1-174: LGTM!Also applies to: 191-213
235-308: LGTM!Also applies to: 354-364, 389-452
.github/workflows/README.md (1)
121-122: LGTM!Also applies to: 137-137
AGENTS.md (1)
32-32: LGTM!CONTRIBUTING.md (1)
33-33: LGTM!Also applies to: 560-567, 593-597
docs/developer-guide/docker.md (1)
60-61: LGTM!ruff.toml (1)
30-30: LGTM!Also applies to: 42-44, 61-76, 91-93
dprint.json (1)
1-11: LGTM!STYLE_GUIDE.md (1)
828-829: LGTM!README.md (1)
43-43: LGTM!.mise.toml (1)
9-10: LGTM!Also applies to: 29-30, 38-44
.mise/tasks/docs.toml (1)
18-19: LGTM!.mise/tasks/setup.toml (1)
7-10: LGTM!.mise/tasks/tests.toml (2)
31-37: LGTM!Also applies to: 73-77, 88-90, 96-98, 103-118, 132-143
92-92: 🎯 Functional CorrectnessNo duplicate TOML table headers here.
["test:e2e:dp"],["test:e2e:collect"], and["test:nss-config-dataset"]each appear once, so this file should still parse normally.> Likely an incorrect or invalid review comment.
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Addresses PR #655 review comments: scope CPU torch/torchaudio/torchvision sources to Linux, skip rewriting an unchanged pyproject.toml, drop unused structlog dependency and dead GenStatus.error, use Pydantic's frozen model config instead of mixing in @DataClass, match the script's Python range to the repo, and run the lock-check generator call offline. Also simplifies tools/gen_cuda_deps.py: drop the PEP 723 inline-script metadata block in favor of running against the repo venv, fix a marker duplication bug in the generated-block stripping logic (surfaced while dropping that block), fold single-caller helpers into their only callers (_render_template, GeneratedBlocks' position finders), derive the uv source marker from sys_platform/arch instead of hand-duplicating it in cuda_deps.toml, and move CudaVariantUvRouter's index-resolution methods onto CudaVariantContext to remove multi-hop reaches through it. Reworks the test fixtures to use structural TOML mutation instead of brittle string-replace against a shared blob, and extracts duplicated expected-output constants. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
…tion pyproject.toml's CPU/CUDA sections are now generated from cuda_deps.toml, but AGENTS.md never mentioned this, docs/developer-guide/docker.md still told contributors to hand-edit pyproject.toml for a new variant, and .agents/skills/uv-build/SKILL.md contradicted its own generated-section warning with a stale "edit extras manually in pyproject.toml" convention and a --script invocation that no longer works now that the generator dropped its inline PEP 723 metadata. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
| [cuda_indexes.flashinfer] | ||
| name = "flashinfer-jit-cache" | ||
| url = "https://flashinfer.ai/whl/{extra}" | ||
| explicit = true |
There was a problem hiding this comment.
name but a per-variant url template. With only cu129 today this is fine, but the moment a second CUDA variant is added in the stacked PRs (#656, #657), _add_index will raise Conflicting uv index definition for 'flashinfer-jit-cache' because cu129 maps the name to https://flashinfer.ai/whl/cu129 and the new variant would map the same name to a different URL. The test fixture correctly uses name = "flashinfer-{extra}" for the multi-variant case; the real config should match that pattern.
| [cuda_indexes.flashinfer] | |
| name = "flashinfer-jit-cache" | |
| url = "https://flashinfer.ai/whl/{extra}" | |
| explicit = true | |
| [cuda_indexes.flashinfer] | |
| name = "flashinfer-jit-cache-{extra}" | |
| url = "https://flashinfer.ai/whl/{extra}" | |
| explicit = true |
There was a problem hiding this comment.
Fixed in 4e0d64a3. The default FlashInfer index name now includes {extra}, producing flashinfer-jit-cache-cu129 and unique names for future variants. I regenerated pyproject.toml and added repository-config assertions for the source mapping and URL.
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
7cf5c58 to
1abd6df
Compare
coderabbitai
Bot
added
feature
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
coderabbitai
Bot
added
refactor
# Summary Removes two `[[indexes]]` entries from `cuda_deps.toml` (and the regenerated `pyproject.toml` block) that the resolver cannot reach: - `nv-shared-pypi-local` -> `https://urm.nvidia.com/artifactory/api/pypi/nv-shared-pypi-local/simple` - `nvidia-pypi-public` -> `https://pypi.nvidia.com` Both have been declared since the initial commit, carried over when the project moved off NVIDIA's internal GitLab, and copied verbatim into `cuda_deps.toml` by NVIDIA-NeMo#655. ## Why they are inert Both are `explicit = true`, which in uv means the index is consulted *only* for packages that name it in `[tool.uv.sources]`. No package names either one: ``` index -> packages bound via [tool.uv.sources] pytorch-cu129 ['-', 'cu129'] flashinfer-jit-cache-cu129 ['cu129'] pytorch-cpu ['cpu'] nv-shared-pypi-local NOT IN SOURCES nvidia-pypi-public NOT IN SOURCES flashinfer-cubin ['cpu', 'cu129'] vllm-v0-26-0-cu129 ['cu129'] ``` Every `nvidia-*` package in the lock resolves from PyPI or `download.pytorch.org` instead. ## This does not stop pypi.nvidia.com being used Worth stating explicitly, since it looks contradictory in the lock: `nvidia-cublas-cu12` is resolved from the PyTorch cu129 index but its wheels are *hosted* on `pypi.nvidia.com` — PyTorch's index links to NVIDIA's host rather than rehosting the artifacts. ```toml [[package]] name = "nvidia-cublas-cu12" source = { registry = "https://download.pytorch.org/whl/cu129" } wheels = [ { url = "https://pypi.nvidia.com/nvidia-cublas-cu12/..." }, ] ``` That download follows the URL recorded in `uv.lock` and is unaffected by whether the index is declared here. ## Verification Regenerating and relocking leaves `uv.lock` **byte-identical** across all 356 packages — the strongest available evidence that neither index participated in resolution: ``` $ python tools/gen_cuda_deps.py cuda_deps.toml --pyproject pyproject.toml $ uv lock Resolved 356 packages in 2ms $ git diff --quiet uv.lock && echo "byte-identical" byte-identical ``` - `mise run lock-check` passes - `mise run format-check` passes - `pytest tests/test_gen_cuda_deps.py` -- 19 passed ## Notes for reviewers Two places still mention these names; neither is affected, but flagging so a grep does not cause confusion: - `CONTRIBUTING.md:870` documents that the internal NMP service pulls `nemo-safe-synthesizer` *from* `nv-shared-pypi-local`. That is about how NMP consumes this package, not how this project resolves its own dependencies, so it is unaffected. - `tests/test_gen_cuda_deps.py` uses `nvidia-pypi-public` in inline fixture data to exercise the generator's `index =` binding support. The generator still supports it; the real config just does not use it. If binding `nvidia-cublas` to that index was intended at some point, that would be a deliberate change (re-adding `index = "nvidia-pypi-public"` to the cublas entry) rather than a reason to keep an orphaned declaration. ## Pre-Review Checklist - [x] `mise run format && mise run check` - [x] `mise run test` passes locally (targeted: `tests/test_gen_cuda_deps.py`) ## Other Notes Config-only change. No dependency versions change and `uv.lock` is untouched. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated package installation configuration by removing two obsolete package sources. * Existing package indexes and CUDA-related configuration remain unchanged. * No user-facing functionality or public APIs were changed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
4 participants
Summary
this is one of three stacked prs (#656, #657) for our overdue multiple-versions-of-cuda support. This one is the core mechanism for generating the deps all from one place and handles instructions and docs for new installation methods.
Validation
Summary by CodeRabbit
Summary
New Features
cuda_deps.toml, with automatic regeneration of dependency lists and package source/index metadata.--check/lock-check flow to prevent CUDA metadata drift.Documentation
cuda_deps.tomland keep the lockfile in sync.Quality & Tests
Chores / Style
dprint-based TOML formatting and expanded CI formatting checks.