chore: add end-to-end version-bump script#434
Conversation
Add scripts/bump_version.py, a stdlib-only tool that automates the on-main "Bumping the ModelExpress Version" procedure: it edits the workspace/chart/Python version literals and every mx_version fixture, regenerates Cargo.lock and uv.lock, regenerates the SHA256-derived mx_source_id pinned-hash assertions in both Python and Rust, and verifies with cargo and pytest. Supports --dry-run and per-stage --skip-* flags. The discovery-based literal pass and global hash replacement close two gaps in the documented manual procedure: an mx_version fixture in workspace-tests/ that the file list omits, and pinned-hash occurrences the -k test filter misses. scripts/ is excluded from the Docker build context via .dockerignore and lives outside the Python wheel, so the tool never ships to users. Document usage in CLAUDE.md (synced to .github/copilot-instructions.md and .cursor/rules/rust.mdc) and add a CONTRIBUTING.md command entry. Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WalkthroughThis PR introduces ChangesVersion Bumping Automation
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes introduce a substantial new Python CLI tool (388 lines of logic) with five operational stages, requiring careful review of version validation, file replacement mechanics, test-driven hash extraction, and subprocess handling. However, the implementation is well-structured by stage, uses familiar patterns, and supporting documentation is comprehensive. Configuration and doc updates are straightforward. The moderate complexity stems from the multi-stage orchestration and regex-based file patching rather than exotic patterns. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLAUDE.md (1)
23-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
bump_version.pycommand in Build and Test Commands section.The "Build and Test Commands" section in CLAUDE.md does not include the
python scripts/bump_version.py 0.6.0command, but both.cursor/rules/rust.mdc(line 41) and.github/copilot-instructions.md(line 41) include it. Per coding guidelines, these three files should be synchronized when making build command changes.📝 Proposed fix to add the command
./run_integration_tests.sh # Integration tests (starts server) +python scripts/bump_version.py 0.6.0 # Bump workspace/chart/python version + source-id hashes (see scripts/README.md)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@CLAUDE.mdaround lines 23 - 36, The Build and Test Commands section in
CLAUDE.md is missing the version bump step; add the missing command "python
scripts/bump_version.py 0.6.0" into the listed commands so CLAUDE.md matches
.cursor/rules/rust.mdc and .github/copilot-instructions.md; update the commands
block under "## Build and Test Commands" to include that exact bump_version
invocation as a separate line consistent with the other entries.</details> <!-- cr-comment:v1:dac03369b9c8de8ef3a28b4b --> _Source: Coding guidelines_ </blockquote></details> </blockquote></details>🤖 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/bump_version.py`: - Around line 365-369: The script currently calls require_tool("cargo")/run(["cargo", "update", "--workspace"], repo_root, args.dry_run) and require_tool("uv")/run(["uv", "lock"], repo_root / PYTHON_DIR, args.dry_run) but ignores failures; change the calls so that run's non-zero exit causes immediate failure: either have run raise an exception on non-zero or capture its return value and call sys.exit(1) (or raise RuntimeError) when the command fails for both the cargo update and uv lock invocations, ensuring the script stops rather than printing success after a failed lock stage. - Line 60: The function signature def fail(message: str) -> "NoReturn": uses NoReturn as a quoted name and suppresses the lint error; import NoReturn from typing (add from typing import NoReturn) and remove the string literal/type: ignore so the signature becomes def fail(message: str) -> NoReturn:. Also replace the lambda assigned to sub (currently sub = lambda ...) with a properly named def (e.g., def sub(...): return ...) to satisfy E731 and keep the same behavior; update any references to sub unchanged. --- Outside diff comments: In `@CLAUDE.md`: - Around line 23-36: The Build and Test Commands section in CLAUDE.md is missing the version bump step; add the missing command "python scripts/bump_version.py 0.6.0" into the listed commands so CLAUDE.md matches .cursor/rules/rust.mdc and .github/copilot-instructions.md; update the commands block under "## Build and Test Commands" to include that exact bump_version invocation as a separate line consistent with the other entries.🪄 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: CHILL
Plan: Enterprise
Run ID:
4738a9da-fcfa-4892-8a89-319a024ccc5f📒 Files selected for processing (7)
.cursor/rules/rust.mdc.dockerignore.github/copilot-instructions.mdCLAUDE.mdCONTRIBUTING.mdscripts/README.mdscripts/bump_version.py
| HEX16 = r"[0-9a-f]{16}" | ||
|
|
||
|
|
||
| def fail(message: str) -> "NoReturn": # type: ignore[name-defined] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ruff check scripts/bump_version.py --select F821,E731Repository: ai-dynamo/modelexpress
Length of output: 1196
Fix lint blockers on NoReturn and lambda assignment.
ruff check scripts/bump_version.py --select F821,E731 fails with:
F821at line 60:NoReturnis used but never imported (def fail(...) -> "NoReturn": # type: ignore[name-defined])E731at line 114:sub = lambda ...triggers “Do not assign alambdaexpression, use adef”
Proposed fix
from pathlib import Path
+from typing import NoReturn
@@
-def fail(message: str) -> "NoReturn": # type: ignore[name-defined]
+def fail(message: str) -> NoReturn:
@@
- sub = lambda m: m.group(1) + new + m.group(2)
+ def sub(m):
+ return m.group(1) + new + m.group(2)🧰 Tools
🪛 Ruff (0.15.15)
[error] 60-60: Undefined name NoReturn
(F821)
🤖 Prompt for 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.
In `@scripts/bump_version.py` at line 60, The function signature def fail(message:
str) -> "NoReturn": uses NoReturn as a quoted name and suppresses the lint
error; import NoReturn from typing (add from typing import NoReturn) and remove
the string literal/type: ignore so the signature becomes def fail(message: str)
-> NoReturn:. Also replace the lambda assigned to sub (currently sub = lambda
...) with a properly named def (e.g., def sub(...): return ...) to satisfy E731
and keep the same behavior; update any references to sub unchanged.
Source: Linters/SAST tools
| require_tool("cargo") | ||
| run(["cargo", "update", "--workspace"], repo_root, args.dry_run) | ||
| print("[3/5] uv.lock") | ||
| require_tool("uv") | ||
| run(["uv", "lock"], repo_root / PYTHON_DIR, args.dry_run) |
There was a problem hiding this comment.
Handle lock regeneration command failures immediately.
Line 365-369 runs cargo update and uv lock but ignores exit codes, so the script can continue and print success after a failed lock stage.
Proposed fix
else:
print("[2/5] cargo.lock")
require_tool("cargo")
- run(["cargo", "update", "--workspace"], repo_root, args.dry_run)
+ cargo_res = run(["cargo", "update", "--workspace"], repo_root, args.dry_run)
+ if not args.dry_run and cargo_res.returncode != 0:
+ fail("lock regeneration failed: cargo update --workspace")
print("[3/5] uv.lock")
require_tool("uv")
- run(["uv", "lock"], repo_root / PYTHON_DIR, args.dry_run)
+ uv_res = run(["uv", "lock"], repo_root / PYTHON_DIR, args.dry_run)
+ if not args.dry_run and uv_res.returncode != 0:
+ fail("lock regeneration failed: uv lock")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| require_tool("cargo") | |
| run(["cargo", "update", "--workspace"], repo_root, args.dry_run) | |
| print("[3/5] uv.lock") | |
| require_tool("uv") | |
| run(["uv", "lock"], repo_root / PYTHON_DIR, args.dry_run) | |
| require_tool("cargo") | |
| cargo_res = run(["cargo", "update", "--workspace"], repo_root, args.dry_run) | |
| if not args.dry_run and cargo_res.returncode != 0: | |
| fail("lock regeneration failed: cargo update --workspace") | |
| print("[3/5] uv.lock") | |
| require_tool("uv") | |
| uv_res = run(["uv", "lock"], repo_root / PYTHON_DIR, args.dry_run) | |
| if not args.dry_run and uv_res.returncode != 0: | |
| fail("lock regeneration failed: uv lock") |
🤖 Prompt for 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.
In `@scripts/bump_version.py` around lines 365 - 369, The script currently calls
require_tool("cargo")/run(["cargo", "update", "--workspace"], repo_root,
args.dry_run) and require_tool("uv")/run(["uv", "lock"], repo_root / PYTHON_DIR,
args.dry_run) but ignores failures; change the calls so that run's non-zero exit
causes immediate failure: either have run raise an exception on non-zero or
capture its return value and call sys.exit(1) (or raise RuntimeError) when the
command fails for both the cargo update and uv lock invocations, ensuring the
script stops rather than printing success after a failed lock stage.
1 participant
Summary
Adds
scripts/bump_version.py, a stdlib-only tool that automates the on-main"Bumping the ModelExpress Version" procedure documented in
CLAUDE.md. The repois already at 0.5.0, so this captures the procedure as a reusable, parameterized
tool rather than a one-shot edit.
It runs a five-stage pipeline, each skippable, with
--dry-run:mx_versionfixture/example, discovered across the tree.
cargo update --workspace.uv lock.mx_versionfeeds the SHA256-derivedmx_source_id, so the pinnedhashes change. Runs the pinned tests, parses the new hashes, patches both the
Python assertions and the Rust cross-checks, then re-runs to confirm. A bad
regeneration leaves the tests red and aborts.
cargo check --workspace --tests, the Rustsource_identitytests, and the Python test suite.
Closes two gaps in the documented manual procedure
mx_versionfixture inworkspace-tests/tests/artifact_transfer_contract.rsthat the documented file list omits (the discovery pass finds it).
-k "pinned_hash or case_colliding"filter misses (
test_empty_artifact_fields_preserve_existing_idand its Rustcounterpart). The global old->new hash replacement covers all of them.
Build isolation
scripts/is added to.dockerignore(the Dockerfile doesCOPY . .) and livesoutside the Python wheel (built from
modelexpress_client/python/), so the toolnever ships to users.
Does NOT touch
nvcr.io/.../modelexpress-server:<tag>) - separaterelease-branch cadence.
Cargo.lock.docs/CLI.mdexample outputs.Docs
Usage note in
CLAUDE.md(synced to.github/copilot-instructions.mdand.cursor/rules/rust.mdc) and aCONTRIBUTING.mdcommand-table entry.Verification
only
Cargo.lockdiff is the four ModelExpress crates.source_identitytests pass with the Python-derived hashes (cross-languageparity), 17/17.
🤖 Generated with Claude Code
Summary by CodeRabbit