chore: Ensure Rust and Python package versions are always in sync - #40
chore: Ensure Rust and Python package versions are always in sync#40dagardner-nv wants to merge 10 commits into
Conversation
… there was no equivelant in Cargo Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
…abric/integrations/harbor/__init__.py Signed-off-by: David Gardner <dagardner@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (3)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)python/src/nemo_fabric/**/*⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2026-06-28T04:03:32.877ZApplied to files:
🔇 Additional comments (3)
WalkthroughSwitches Python packaging from dynamic version generation to static pinned versions, adds ChangesStatic version pinning and automation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
📖 Fern docs preview: https://nvidia-preview-pull-request-40.docs.buildwithfern.com/nemo/fabric |
dagardner-nv
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.agents/skills/update-project-version/SKILL.md:
- Around line 54-58: The skill docs still mention patching
FabricAgent.version(), but Harbor integration now reads version dynamically via
importlib.metadata so that step is obsolete. Update the update-project-version
guide and the referenced set_harbor_integration_version / just set-version flow
to remove any FabricAgent.version() patching language, and align the documented
steps with the current helper names and behavior in the justfile and SKILL.md.
In `@justfile`:
- Around line 241-277: Remove the obsolete Harbor version patching flow from the
justfile: `set_harbor_integration_version` now targets a literal return value
that no longer exists in
`python/src/nemo_fabric/integrations/harbor/__init__.py`, so it will always
fail. Delete that helper and stop calling it from `set_project_version`, keeping
only the remaining version update steps (`set_cargo_workspace_version` and
`set_python_project_versions`) aligned with the current Harbor `version()`
implementation.
In `@python/src/nemo_fabric/integrations/harbor/__init__.py`:
- Around line 56-60: The Harbor integration version method now reads dynamically
from importlib.metadata.version in version(self), so the justfile’s
version-patching regex no longer matches and breaks just set-version. Update the
related set_harbor_integration_version logic to stop patching this method
entirely and instead rely on the runtime-derived version, keeping the dynamic
importlib.metadata.version / PackageNotFoundError implementation as-is.
- Around line 57-60: The Harbor version lookup in the version() helper is
querying the wrong distribution name, so it can’t find the installed wheel for
this module. Update the importlib.metadata.version call in the version()
function to use the runtime distribution name built by the package, and keep the
PackageNotFoundError fallback so the helper still returns None when the
distribution is absent.
🪄 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: dbc03d3a-aaf5-4d66-9b06-7d197dd01156
⛔ Files ignored due to path filters (5)
adapters/codex-cli/uv.lockis excluded by!**/*.lockadapters/common/uv.lockis excluded by!**/*.lockadapters/hermes-cli/uv.lockis excluded by!**/*.lockadapters/hermes-sdk/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.agents/skills/update-project-version/SKILL.mdadapters/codex-cli/pyproject.tomladapters/common/pyproject.tomladapters/hermes-cli/pyproject.tomladapters/hermes-sdk/pyproject.tomljustfilepyproject.tomlpython/src/nemo_fabric/integrations/harbor/__init__.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
python/src/nemo_fabric/**/*
⚙️ CodeRabbit configuration file
python/src/nemo_fabric/**/*: Review Python SDK changes for typed API consistency, import-time dependency neutrality, async/session behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/src/nemo_fabric/integrations/harbor/__init__.py
{pyproject.toml,adapters/**/pyproject.toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
{pyproject.toml,adapters/**/pyproject.toml}: For the setuptools-built Python packages, update the literalproject.versionin the rootpyproject.tomland in everyadapters/**/pyproject.toml.
Keep internal Python dependency pins aligned with the Python release version: allnemo-fabric-* == <version>requirements in the rootpyproject.tomloptional dependencies, and each adapter'snemo-fabric-adapters-common == <version>dependency.
Files:
adapters/hermes-cli/pyproject.tomladapters/codex-cli/pyproject.tomladapters/common/pyproject.tomladapters/hermes-sdk/pyproject.tomlpyproject.toml
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public Fabric contracts.
Files:
adapters/hermes-cli/pyproject.tomladapters/codex-cli/pyproject.tomladapters/common/pyproject.tomladapters/hermes-sdk/pyproject.toml
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Fabric
Timestamp: 2026-07-08T22:53:23.074Z
Learning: For normal releases, use the same `X.Y.Z` version string everywhere; for prerelease or build-metadata releases, use valid Cargo SemVer in `Cargo.toml` and the equivalent PEP 440 version in explicit Python metadata, and ensure the Maturin-built runtime and setuptools-built packages resolve to equivalent versions.
Learnt from: CR
Repo: NVIDIA/NeMo-Fabric
Timestamp: 2026-07-08T22:53:23.074Z
Learning: When changing the project version, follow the prescribed workflow: read the current version from `Cargo.toml`, run `just set-version <cargo-version>`, keep `python/pyproject.toml` unchanged and dynamic, audit old-version references, and run the specified validation commands (including `cargo check --workspace --locked`, `just build-python`, `just test-python` when relevant, `just wheels` when release-facing, and `git diff --check`).
Learnt from: CR
Repo: NVIDIA/NeMo-Fabric
Timestamp: 2026-07-08T22:53:23.074Z
Learning: If editing helper code, keep `set_project_version`, `set_cargo_workspace_version`, `set_python_project_versions`, `set_harbor_integration_version`, and the `set-version` recipe aligned with the documented version-update contracts.
Learnt from: CR
Repo: NVIDIA/NeMo-Fabric
Timestamp: 2026-07-08T22:53:23.074Z
Learning: Avoid partial or blind version updates: do not update only `Cargo.toml`, do not add a literal version to `python/pyproject.toml`, do not update Python package versions without their exact internal pins, and do not forget `Cargo.lock` or the relevant `uv.lock` files.
📚 Learning: 2026-06-28T04:03:32.877Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 26
File: python/tests/smoke_typed_config.py:163-177
Timestamp: 2026-06-28T04:03:32.877Z
Learning: In NVIDIA NeMo Fabric Python SDK serialization of `RuntimeCapabilities` (to satisfy the “parity contract” with Rust core and the CLI), do not emit metadata keys when the corresponding metadata is absent. Instead, omit those fields entirely so the produced JSON matches the Rust/CLI output (e.g., avoid `null`, empty objects, or placeholder metadata). During review, verify the serializer/builders follow this omission rule and that Python outputs/parity tests reflect the same shape.
Applied to files:
python/src/nemo_fabric/integrations/harbor/__init__.py
🪛 LanguageTool
.agents/skills/update-project-version/SKILL.md
[uncategorized] ~40-~40: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ...e same X.Y.Z string everywhere. For a prerelease or build-metadata version, use valid Ca...
(EN_WORD_COHERENCY)
[uncategorized] ~51-~51: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ...cipe converts supported Cargo SemVer prereleases to PEP 440 and updates: - `Cargo.tom...
(EN_WORD_COHERENCY)
[style] ~87-~87: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...all Python package metadata resolves. - Run just test-python when the integration...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~90-~90: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ing validation of every Python wheel. - Run git diff --check. ## Avoid - Updati...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (10)
pyproject.toml (1)
4-16: LGTM!Also applies to: 20-59, 98-98
adapters/common/pyproject.toml (1)
4-18: LGTM!adapters/codex-cli/pyproject.toml (1)
12-19: LGTM!adapters/hermes-cli/pyproject.toml (1)
12-20: LGTM!adapters/hermes-sdk/pyproject.toml (1)
12-19: LGTM!justfile (4)
15-64: LGTM!
65-177: LGTM!
179-239: LGTM!
330-345: LGTM!.agents/skills/update-project-version/SKILL.md (1)
1-111: LGTM!
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
1 participant
Overview
setuptools-scmandsetuptools_dynamic_dependenciesfrom the setuptools-based Python packages. The reason is that Cargo and Maturin don't have the ability to infer the version from a git tag. This caused thenemo-fabric-runtimepackage to have a slightly different versionset-versionJust recipe from Relay to automate setting/updating version stringsWhere should the reviewer start?
Start with the
set-versionrecipe and its helper functions injustfile, then review.agents/skills/update-project-version/SKILL.mdfor the intended release workflow.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to: FABRIC-57
I confirm this contribution is my own work, or I have the right to submit it under this project's license.
I searched existing issues and open pull requests, and this does not duplicate existing work.
Summary by CodeRabbit
New Features
Bug Fixes