fix: rewrite version pins with dependency extras - #154
Conversation
WalkthroughThe release helper now invokes a standalone Python utility to synchronize package versions and internal dependency pins across project manifests. The utility validates dynamic runtime versioning and pin consistency, with pytest coverage for root and adapter manifests. ChangesPython version synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseHelper
participant VersionSyncScript
participant ProjectManifests
ReleaseHelper->>VersionSyncScript: Pass computed PEP 440 version
VersionSyncScript->>ProjectManifests: Update package versions and internal pins
VersionSyncScript->>ProjectManifests: Validate dynamic runtime version and pin consistency
VersionSyncScript-->>ReleaseHelper: Report success or exit with validation error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Fern docs preview: https://nvidia-preview-pull-request-154.docs.buildwithfern.com/nemo/fabric |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/ci/set_python_project_versions.py`:
- Around line 25-52: The runtime manifest validation currently occurs after the
project-version write loop, allowing partial updates on failure. Move the
runtime_path/tomllib validation block before iterating over project_paths,
ensuring all failure-prone checks complete before any manifest is mutated;
preserve the existing dynamic-version validation behavior.
- Around line 27-33: Update the version replacement logic in the project-version
processing flow to match only the version field under the [project] section,
count all matching entries before replacing, and require exactly one match. Do
not use subn(..., count=1) as the invariant check; reject manifests with zero or
multiple [project].version entries before writing changes.
In `@tests/scripts/test_set_python_project_versions.py`:
- Around line 17-87: Extend
test_set_python_project_versions_updates_internal_pins_with_extras or add a
focused test covering python/pyproject.toml runtime validation: create a runtime
project with a static project.version or without dynamic including "version",
call set_python_project_versions, and assert pytest.raises(SystemExit). Keep the
existing successful dynamic-version case unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 070b1d9f-4cd8-4a08-aacb-f60e0c9ed570
📒 Files selected for processing (3)
justfilescripts/ci/set_python_project_versions.pytests/scripts/test_set_python_project_versions.py
📜 Review details
⏰ Context from checks skipped due to timeout. (18)
- GitHub Check: Preview docs
- GitHub Check: Pre-commit
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.12, linux-amd64)
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.pyjustfile
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
**/*.{rs,py}: When changing the Rust core or public schemas, run both the Rust and Python test suites.
When adding functionality, include tests in the corresponding Rust crate or the relevant area undertests/.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected and run by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; place fixtures needed by multiple test files inconftest.py.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a function named<fixture_name>_fixture; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its return value is unused or it does not return a value.
Use the autouserestore_environ_fixturefromtests/conftest.pyto restore environment variables; modify variables withos.environand do not usemonkeypatch.setenv.
Avoid defensive programming in tests; access expected data directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
Files:
tests/scripts/test_set_python_project_versions.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{rs,py,html,md,mdx,toml,yml,yaml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Rust, Python, HTML, Markdown, MDX, TOML, YAML, and shell source files must include the project SPDX copyright and Apache-2.0 license headers using the comment syntax appropriate to each file type.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use type annotations for public Python APIs.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{py,rs}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,rs}: Keep native Python binding declarations synchronized with their Rust implementations.
Usesnake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/scripts/test_set_python_project_versions.py
{docs/package.json,docs/package-lock.json,.github/workflows/*.yml,justfile}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Keep documentation tooling, CI workflows, install commands, build/test/clean recipes, and examples consistent with current package names and commands.
Files:
justfile
{pyproject.toml,python/pyproject.toml,Cargo.toml,docs/package.json,.github/workflows/*.yml,justfile}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Reflect public packaging changes in release-facing documentation and ensure CI references the same package names and commands used by local workflows.
Files:
justfile
justfile
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
justfile: Theset-versionrecipe must runjust lock-pythonafter source metadata is updated.
set_project_versionmust invoke both Cargo and Python project version helpers.
Files:
justfile
🪛 Ruff (0.16.0)
scripts/ci/set_python_project_versions.py
[warning] 33-33: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 50-52: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 58-58: Use list.extend to create a transformed list
(PERF401)
[warning] 69-69: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (1)
justfile (1)
243-255: 📐 Maintainability & Code QualityProvide release-workflow validation evidence.
Please include results for
just test-pythonandjust build-python; this changes Python release/packaging behavior. As per coding guidelines, “If Python code or a Python-facing adapter changes, runjust test-python” and rebuild withjust build-pythonwhen packaging changes.Source: Coding guidelines
zhongxuanwang-nv
left a comment
There was a problem hiding this comment.
Thanks Anuradha!!!
Signed-off-by: Anuradha Karuppiah <26330987+AnuradhaKaruppiah@users.noreply.github.com>
5b389da to
8ae6313
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/ci/set_python_project_versions.py`:
- Around line 67-70: Validate the CLI version argument before
set_python_project_versions begins rewriting manifests, allowing only the
project’s expected version format and rejecting quotes, newlines, and
replacement backreferences. Ensure invalid input exits without modifying any
manifest, and add a regression test covering unsafe version text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 06d8341f-31f6-4451-aa85-e36e7013c5eb
📒 Files selected for processing (3)
justfilescripts/ci/set_python_project_versions.pytests/scripts/test_set_python_project_versions.py
📜 Review details
⏰ Context from checks skipped due to timeout. (18)
- GitHub Check: Preview docs
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (16)
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
justfiletests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
{docs/package.json,docs/package-lock.json,.github/workflows/*.yml,justfile}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Keep documentation tooling, CI workflows, install commands, build/test/clean recipes, and examples consistent with current package names and commands.
Files:
justfile
{pyproject.toml,python/pyproject.toml,Cargo.toml,docs/package.json,.github/workflows/*.yml,justfile}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Reflect public packaging changes in release-facing documentation and ensure CI references the same package names and commands used by local workflows.
Files:
justfile
justfile
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
justfile: Theset-versionrecipe must runjust lock-pythonafter source metadata is updated.
set_project_versionmust invoke both Cargo and Python project version helpers.
Files:
justfile
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
**/*.{rs,py}: When changing the Rust core or public schemas, run both the Rust and Python test suites.
When adding functionality, include tests in the corresponding Rust crate or the relevant area undertests/.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected and run by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; place fixtures needed by multiple test files inconftest.py.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a function named<fixture_name>_fixture; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its return value is unused or it does not return a value.
Use the autouserestore_environ_fixturefromtests/conftest.pyto restore environment variables; modify variables withos.environand do not usemonkeypatch.setenv.
Avoid defensive programming in tests; access expected data directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
Files:
tests/scripts/test_set_python_project_versions.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{rs,py,html,md,mdx,toml,yml,yaml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Rust, Python, HTML, Markdown, MDX, TOML, YAML, and shell source files must include the project SPDX copyright and Apache-2.0 license headers using the comment syntax appropriate to each file type.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use type annotations for public Python APIs.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
**/*.{py,rs}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,rs}: Keep native Python binding declarations synchronized with their Rust implementations.
Usesnake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
tests/scripts/test_set_python_project_versions.pyscripts/ci/set_python_project_versions.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/scripts/test_set_python_project_versions.py
🪛 Ruff (0.16.0)
scripts/ci/set_python_project_versions.py
[warning] 33-33: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 50-52: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 58-58: Use list.extend to create a transformed list
(PERF401)
[warning] 69-69: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (4)
scripts/ci/set_python_project_versions.py (2)
25-52: Validate every manifest before the first write.Runtime-manifest parsing still occurs after root and adapter rewrites, so an invalid runtime manifest leaves a partial release stamp. This existing finding remains unresolved.
27-33: Enforce exactly one[project].version.Line 30 limits
subnto one replacement, socountcannot detect a second match; the pattern is also not scoped to[project]. This existing finding remains unresolved.tests/scripts/test_set_python_project_versions.py (1)
17-87: Add runtime-validation failure coverage.The suite still tests only the valid dynamic-version path. Add static-version and missing-
dynamiccases assertingSystemExit.As per coding guidelines, “Define verifiable success criteria … invalid-input tests for validation”; as per path instructions, tests must cover “error paths.”
Sources: Coding guidelines, Path instructions
justfile (1)
243-255: LGTM!
|
/merge |
Summary
nemo-fabric-adapters-hermes[harness].Root cause
The release regex matched bare package names only, so RC builds updated package versions but left root extras pinned to the stable version. Installing a root extra could therefore request an adapter version that was never published.
Validation
just --set no_uv true test-python— 531 passed, 14 skippedcargo check -p fabric-python --lockedjust wheelsv0.2.0-rc5stamping and verified the built root wheel pins all adapter extras to0.2.0rc5Summary by CodeRabbit