fix(onboard): warn on arm64 NIM image compatibility (Fixes #5772) - #5868
Conversation
|
Warning Review limit reached
More reviews will be available in 18 minutes and 30 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds an ARM64/Linux NVIDIA NIM compatibility warning helper, calls it during onboarding NIM setup, and adds unit and integration tests covering the warning condition, formatted message, and logged onboarding output. ChangesArm64 NIM compatibility warning
Sequence Diagram(s)sequenceDiagram
participant setupNim
participant warnAboutArm64NimImageCompatibility
participant "console.log" as consoleLog
setupNim->>warnAboutArm64NimImageCompatibility: pass gpu and nim-local availability
warnAboutArm64NimImageCompatibility->>consoleLog: emit warning lines when the condition matches
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
25ddc8b to
f9e79e7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/onboard.ts (1)
4009-4012: 🩺 Stability & Availability | 🔵 TrivialRun the onboarding E2E cohort before merge.
This hook lands in the core sandbox creation flow, so it’s worth running the selective nightly onboarding jobs called out for
src/lib/onboard.tsto catch cross-flow regressions that the unit/integration tests here won’t cover. As per path instructions,src/lib/onboard.ts“contains core onboarding logic” and the listed E2E jobs are the recommended coverage for changes in this file.🤖 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 `@src/lib/onboard.ts` around lines 4009 - 4012, This change touches the core onboarding flow via warnAboutArm64NimImageCompatibility in onboard.ts, so validate it by running the selective onboarding E2E cohort before merging. Use the recommended nightly onboarding jobs for src/lib/onboard.ts to cover cross-flow regressions that unit/integration tests may miss, and confirm the new nim-local option handling behaves correctly in the sandbox creation path.Source: Path instructions
src/lib/onboard/nim-image-compat-warning.test.ts (1)
13-54: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression case for the
gpu.sparkbranch.The predicate supports both
gpu.spark === trueandgpu.platform === "spark" | "station", but this suite only locks down theplatformpath. A future refactor could break the existing GB10 boolean path without failing tests. Based on the PR objectives, onboarding should keep the current GB10 detection behavior unchanged.🤖 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 `@src/lib/onboard/nim-image-compat-warning.test.ts` around lines 13 - 54, Add a regression test in shouldWarnAboutArm64NimImageCompatibility coverage for the gpu.spark boolean path, since the current suite only exercises gpu.platform for spark/station. Extend the existing nim-image-compat-warning.test case to assert the same warning behavior when gpu.spark is true on Linux arm64 with nimLocalAvailable enabled, keeping the existing function behavior unchanged.
🤖 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 `@test/onboard-nim-image-compat-warning.test.ts`:
- Around line 88-100: The spawned onboarding flow in the compat warning test has
no bound and can hang the Vitest worker if it becomes interactive or deadlocks.
Update the spawnSync call in this test to include a timeout so the process fails
deterministically instead of wedging CI, keeping the change localized to the
onboarding test helper invocation.
---
Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 4009-4012: This change touches the core onboarding flow via
warnAboutArm64NimImageCompatibility in onboard.ts, so validate it by running the
selective onboarding E2E cohort before merging. Use the recommended nightly
onboarding jobs for src/lib/onboard.ts to cover cross-flow regressions that
unit/integration tests may miss, and confirm the new nim-local option handling
behaves correctly in the sandbox creation path.
In `@src/lib/onboard/nim-image-compat-warning.test.ts`:
- Around line 13-54: Add a regression test in
shouldWarnAboutArm64NimImageCompatibility coverage for the gpu.spark boolean
path, since the current suite only exercises gpu.platform for spark/station.
Extend the existing nim-image-compat-warning.test case to assert the same
warning behavior when gpu.spark is true on Linux arm64 with nimLocalAvailable
enabled, keeping the existing function behavior 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: CHILL
Plan: Enterprise
Run ID: a241359c-89d4-475f-95cd-01b72a31b42f
📒 Files selected for processing (4)
src/lib/onboard.tssrc/lib/onboard/nim-image-compat-warning.test.tssrc/lib/onboard/nim-image-compat-warning.tstest/onboard-nim-image-compat-warning.test.ts
|
✨ Thanks for adding the arm64 NIM image-compatibility warning for DGX Spark and Station users during onboarding. This proposes a way to surface an advisory notice when Local NIM is offered on Linux arm64 so users know some images may lack linux/arm64 manifests while the pull path remains unchanged. Related open issues: |
5e64565 to
715b93e
Compare
|
Rebased on current main and adjusted the onboard warning test for the current source/package and linear-test guards. Build, typecheck, lint/checks, conditional scan, and focused tests pass; commits are verified. |
715b93e to
45db6bb
Compare
|
Rebased on current main and refreshed the verified commits. Focused arm64 NIM warning tests, build, typecheck, lint/checks, and the conditional scan pass. |
Refs NVIDIA#5772 Signed-off-by: Deepak Jain <deepujain@gmail.com>
Signed-off-by: Deepak Jain <deepujain@gmail.com>
45db6bb to
e5098dd
Compare
|
Rebased on current main and refreshed the verified commits. Focused arm64 NIM warning tests, build, typecheck, lint/checks, and the conditional scan pass. |
cv
left a comment
There was a problem hiding this comment.
Substantive review passed. ARM64 Spark and Station detection and warning behavior are covered, all required checks are green, both commits are GitHub Verified, and the trusted exact-head manual PR Review Advisor returned merge_as_is with high confidence. The test tmpDir cleanup note is nonblocking hygiene.
<!-- markdownlint-disable MD041 --> ## Summary Prepares the user-facing documentation for NemoClaw v0.0.76 and closes the release-prep documentation gate. It adds the release highlights, documents the arm64 Local NIM warning and expanded image cleanup behavior, and fixes agent-specific command headings in generated guides. ## Changes - Add the v0.0.76 release-notes section and move the shared-gateway route containment entry out of the v0.0.74 history where it was incorrectly placed. - Document the advisory Linux arm64 Local NIM manifest warning in the canonical platform matrix and local-inference guidance. - Document that `gc` scans both gateway-built and locally prebuilt sandbox image repositories. - Keep OpenClaw and Hermes session headings out of the generated Deep Agents command guide. - Add a focused variant regression test for the agent-specific session headings. ### Source summary | Merged sources | Documentation coverage | | --- | --- | | [#6414](#6414), [#6418](#6418), [#6416](#6416), [#6344](#6344) | v0.0.76 release notes and the Deep Agents quickstart/inference routes | | [#6340](#6340) | v0.0.76 release notes and existing Deep Agents observability guidance | | [#6338](#6338), [#6378](#6378), [#6297](#6297) | v0.0.76 release notes and existing inference/troubleshooting guidance | | [#6362](#6362) | v0.0.76 release notes and existing lifecycle, command, and credential guidance | | [#6330](#6330), [#6307](#6307), [#6008](#6008) | v0.0.76 release notes and existing security, troubleshooting, and command guidance | | [#6382](#6382) | v0.0.76 release notes and existing MCP/command guidance | | [#6326](#6326), [#5868](#5868), [#5539](#5539) | v0.0.76 release notes, platform matrix, inference options, and local-inference guidance | | [#6396](#6396), [#6390](#6390), [#6007](#6007) | v0.0.76 release notes and existing messaging guidance | | [#5388](#5388), [#6249](#6249), [#6303](#6303), [#6306](#6306) | v0.0.76 release notes and command/lifecycle guidance | ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run --project integration test/generate-platform-docs.test.ts test/agent-variant-docs.test.ts test/sync-agent-variant-docs.test.ts` (3 files, 29 tests passed) - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — completed with 0 errors and 2 pre-existing Fern warnings - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added v0.0.76 release notes content, and removed an older conflicting bullet from the surrounding release history. * Expanded Local NVIDIA NIM guidance across inference/provider docs, including an advisory for Linux arm64 DGX Spark/DGX Station hosts when a matching `linux/arm64` image manifest is unavailable. * Updated the command reference for correct session-section rendering and clarified `gc` image cleanup sources. * **Tests** * Added coverage ensuring Deep Agents omits sessions headings while Hermes includes them. * **CI** * Refreshed Local NVIDIA NIM provider notes used in the platform matrix. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
… (NVIDIA#5868) ## Summary Adds the missing arm64 Local NVIDIA NIM image-compatibility warning during onboarding. GB10 detection and the NIM-local provider menu are already working on current `main`, so this PR leaves that behavior alone. The new warning is advisory only: it tells Linux arm64 DGX Spark/Station users that some NIM images may not publish `linux/arm64` manifests, while still allowing the existing NIM pull/start path to try the selected image. Fixes NVIDIA#5772 ## Changes - `src/lib/onboard/nim-image-compat-warning.ts`: adds a small helper for the Linux arm64 DGX Spark/Station warning. - `src/lib/onboard/provider-host-state.ts`: prints the warning once when `nim-local` is available in the provider options. - `src/lib/onboard/nim-image-compat-warning.test.ts`: covers the warning eligibility and wording. - `test/onboard-nim-image-compat-warning.test.ts`: exercises the compiled onboarding flow with a fake Linux arm64 DGX Spark GPU and confirms the warning appears when Local NIM is offered. ## Testing - `npm install --ignore-scripts` - completed. - `npm run build:cli` - passed. - `npx vitest run src/lib/onboard/nim-image-compat-warning.test.ts test/onboard-nim-image-compat-warning.test.ts` - passed. - `npm run typecheck:cli` - passed. - `npm run source-shape:check` - passed outside the sandbox after the sandboxed `tsx` IPC pipe failed with `EPERM`. - `npm run test-size:check` - passed outside the sandbox after the sandboxed `tsx` IPC pipe failed with `EPERM`. - `git diff --check` - passed. - `npx @biomejs/biome format src/lib/onboard/nim-image-compat-warning.ts src/lib/onboard/nim-image-compat-warning.test.ts src/lib/onboard.ts test/onboard-nim-image-compat-warning.test.ts` - passed. - `npx @biomejs/biome lint src/lib/onboard/nim-image-compat-warning.ts src/lib/onboard/nim-image-compat-warning.test.ts src/lib/onboard.ts test/onboard-nim-image-compat-warning.test.ts` - passed. ## Evidence it works The focused onboarding regression overrides `process.arch` and `process.platform` to simulate a Linux arm64 DGX Spark host, passes a GB10-style GPU object with `nimCapable: true`, enables `NEMOCLAW_EXPERIMENTAL=1`, and selects the default cloud provider so no real NIM image is pulled. The test confirms onboarding still completes with `nvidia-prod` while the output contains both the Local NIM arm64 warning and the `linux/arm64` manifest note. Signed-off-by: Deepak Jain <deepujain@gmail.com> --------- Signed-off-by: Deepak Jain <deepujain@gmail.com>
<!-- markdownlint-disable MD041 --> ## Summary Prepares the user-facing documentation for NemoClaw v0.0.76 and closes the release-prep documentation gate. It adds the release highlights, documents the arm64 Local NIM warning and expanded image cleanup behavior, and fixes agent-specific command headings in generated guides. ## Changes - Add the v0.0.76 release-notes section and move the shared-gateway route containment entry out of the v0.0.74 history where it was incorrectly placed. - Document the advisory Linux arm64 Local NIM manifest warning in the canonical platform matrix and local-inference guidance. - Document that `gc` scans both gateway-built and locally prebuilt sandbox image repositories. - Keep OpenClaw and Hermes session headings out of the generated Deep Agents command guide. - Add a focused variant regression test for the agent-specific session headings. ### Source summary | Merged sources | Documentation coverage | | --- | --- | | [NVIDIA#6414](NVIDIA#6414), [NVIDIA#6418](NVIDIA#6418), [NVIDIA#6416](NVIDIA#6416), [NVIDIA#6344](NVIDIA#6344) | v0.0.76 release notes and the Deep Agents quickstart/inference routes | | [NVIDIA#6340](NVIDIA#6340) | v0.0.76 release notes and existing Deep Agents observability guidance | | [NVIDIA#6338](NVIDIA#6338), [NVIDIA#6378](NVIDIA#6378), [NVIDIA#6297](NVIDIA#6297) | v0.0.76 release notes and existing inference/troubleshooting guidance | | [NVIDIA#6362](NVIDIA#6362) | v0.0.76 release notes and existing lifecycle, command, and credential guidance | | [NVIDIA#6330](NVIDIA#6330), [NVIDIA#6307](NVIDIA#6307), [NVIDIA#6008](NVIDIA#6008) | v0.0.76 release notes and existing security, troubleshooting, and command guidance | | [NVIDIA#6382](NVIDIA#6382) | v0.0.76 release notes and existing MCP/command guidance | | [NVIDIA#6326](NVIDIA#6326), [NVIDIA#5868](NVIDIA#5868), [NVIDIA#5539](NVIDIA#5539) | v0.0.76 release notes, platform matrix, inference options, and local-inference guidance | | [NVIDIA#6396](NVIDIA#6396), [NVIDIA#6390](NVIDIA#6390), [NVIDIA#6007](NVIDIA#6007) | v0.0.76 release notes and existing messaging guidance | | [NVIDIA#5388](NVIDIA#5388), [NVIDIA#6249](NVIDIA#6249), [NVIDIA#6303](NVIDIA#6303), [NVIDIA#6306](NVIDIA#6306) | v0.0.76 release notes and command/lifecycle guidance | ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run --project integration test/generate-platform-docs.test.ts test/agent-variant-docs.test.ts test/sync-agent-variant-docs.test.ts` (3 files, 29 tests passed) - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — completed with 0 errors and 2 pre-existing Fern warnings - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added v0.0.76 release notes content, and removed an older conflicting bullet from the surrounding release history. * Expanded Local NVIDIA NIM guidance across inference/provider docs, including an advisory for Linux arm64 DGX Spark/DGX Station hosts when a matching `linux/arm64` image manifest is unavailable. * Updated the command reference for correct session-section rendering and clarified `gc` image cleanup sources. * **Tests** * Added coverage ensuring Deep Agents omits sessions headings while Hermes includes them. * **CI** * Refreshed Local NVIDIA NIM provider notes used in the platform matrix. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Summary
Adds the missing arm64 Local NVIDIA NIM image-compatibility warning during onboarding.
GB10 detection and the NIM-local provider menu are already working on current
main, so this PR leaves that behavior alone. The new warning is advisory only: it tells Linux arm64 DGX Spark/Station users that some NIM images may not publishlinux/arm64manifests, while still allowing the existing NIM pull/start path to try the selected image.Fixes #5772
Changes
src/lib/onboard/nim-image-compat-warning.ts: adds a small helper for the Linux arm64 DGX Spark/Station warning.src/lib/onboard/provider-host-state.ts: prints the warning once whennim-localis available in the provider options.src/lib/onboard/nim-image-compat-warning.test.ts: covers the warning eligibility and wording.test/onboard-nim-image-compat-warning.test.ts: exercises the compiled onboarding flow with a fake Linux arm64 DGX Spark GPU and confirms the warning appears when Local NIM is offered.Testing
npm install --ignore-scripts- completed.npm run build:cli- passed.npx vitest run src/lib/onboard/nim-image-compat-warning.test.ts test/onboard-nim-image-compat-warning.test.ts- passed.npm run typecheck:cli- passed.npm run source-shape:check- passed outside the sandbox after the sandboxedtsxIPC pipe failed withEPERM.npm run test-size:check- passed outside the sandbox after the sandboxedtsxIPC pipe failed withEPERM.git diff --check- passed.npx @biomejs/biome format src/lib/onboard/nim-image-compat-warning.ts src/lib/onboard/nim-image-compat-warning.test.ts src/lib/onboard.ts test/onboard-nim-image-compat-warning.test.ts- passed.npx @biomejs/biome lint src/lib/onboard/nim-image-compat-warning.ts src/lib/onboard/nim-image-compat-warning.test.ts src/lib/onboard.ts test/onboard-nim-image-compat-warning.test.ts- passed.Evidence it works
The focused onboarding regression overrides
process.archandprocess.platformto simulate a Linux arm64 DGX Spark host, passes a GB10-style GPU object withnimCapable: true, enablesNEMOCLAW_EXPERIMENTAL=1, and selects the default cloud provider so no real NIM image is pulled. The test confirms onboarding still completes withnvidia-prodwhile the output contains both the Local NIM arm64 warning and thelinux/arm64manifest note.Signed-off-by: Deepak Jain deepujain@gmail.com