feat(inference): add interactive managed-vLLM model picker - #5038
Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds platform-aware vLLM registry filtering, an interactive picker with gated-model validation and default-first ordering (re-exported), and refactors installVllm to resolve models via the new resolver with profile platform discriminators and tests covering resolution flows. ChangesvLLM Platform-Aware Interactive Model Picker
Sequence Diagram(s)sequenceDiagram
participant installVllm
participant Env
participant modelsForPlatform
participant promptVllmModel
installVllm->>Env: check NEMOCLAW_VLLM_MODEL
alt env override present
installVllm->>Env: selectVllmModelFromEnv + assertGatedModelAccess
installVllm-->>installVllm: modelSource = env
else no env override and non-interactive
installVllm-->>installVllm: model = profile.defaultModel
installVllm-->>installVllm: modelSource = default
else interactive
installVllm->>modelsForPlatform: get platform-curated list
installVllm->>promptVllmModel: show picker
promptVllmModel-->>installVllm: selected model or BACK_TO_SELECTION
installVllm-->>installVllm: modelSource = picker or default
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
E2E Advisor RecommendationRequired E2E: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
PR Review AdvisorFindings: 1 needs attention, 5 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
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 `@src/lib/inference/vllm-prompt.ts`:
- Around line 69-73: The current logic in the selection loop (using trimmed,
index, parseInt and ordered) accepts partially numeric input like "2abc" because
parseInt tolerates trailing text; update the validation to reject any
non-pure-numeric input before parsing: if trimmed is empty treat as 0 as before,
otherwise ensure trimmed matches a strict integer regex (e.g., /^\d+$/) or that
String(Number(trimmed)) === trimmed, and only then parse to compute index; if
validation fails call errorLine and continue. Apply this change around the
trimmed/index logic used to validate choices so partially numeric inputs are
rejected.
🪄 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: 41a42993-fb08-4fc7-be8e-134646095131
📒 Files selected for processing (6)
src/lib/inference/model-prompts.test.tssrc/lib/inference/model-prompts.tssrc/lib/inference/vllm-models.test.tssrc/lib/inference/vllm-models.tssrc/lib/inference/vllm-prompt.tssrc/lib/inference/vllm.ts
Selective E2E Results — ✅ All requested jobs passedRun: 27200903801
|
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
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 `@src/lib/inference/vllm.test.ts`:
- Line 188: The test uses untyped map callbacks over Jest mock calls and an
unsafe access to questions[0]; update each map callback (the calls mapping that
builds summary and the other two similar maps) to explicitly type the parameter
as unknown[] (e.g., (c: unknown[]) => String(c[0])) to remove implicit any, and
protect the questions[0] access in the assertion around the questions array (in
the test referencing questions) by first asserting questions.length > 0 or using
a safe expression like (questions.length ? questions[0] : '') or questions.at(0)
?? '' so the build no longer fails.
🪄 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: b79c47c9-33e2-4393-a29e-9fb8d5318bde
📒 Files selected for processing (4)
src/lib/inference/model-prompts.test.tssrc/lib/inference/vllm-models.tssrc/lib/inference/vllm-prompt.tssrc/lib/inference/vllm.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/inference/vllm-models.ts
- src/lib/inference/model-prompts.test.ts
- src/lib/inference/vllm-prompt.ts
Selective E2E Results — ✅ All requested jobs passedRun: 27203986387
|
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 27205547622
|
Drop main-side biome line-wrap on src/lib/inference/vllm.ts where it conflicted with the picker refactor. Keep HEAD's new VllmProfile.platform field (needed for VllmModelDef filtering) and HEAD's modelSource-based predicate for the (NEMOCLAW_VLLM_MODEL override) label, which replaces the prior model.id vs profile.defaultModel.id comparison now that the picker returns an explicit source. Biome wrap re-applied around the preserved predicate. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 27249069423
|
Selective E2E Results — ✅ All requested jobs passedRun: 27249232580
|
## Summary - Add the v0.0.63 release-note section using the published development note as source context. - Update source docs for sandbox recovery, OpenClaw config restore safety, managed vLLM selection, Slack Socket Mode conflict handling, and host diagnostics. - Refresh generated `nemoclaw-user-*` skills from the updated Fern MDX docs. - Update the release-doc refresh skill so post-release docs for version `n` look up the matching announcement discussion and use the `n+1` patch release label. - Fix CLI/docs parity by avoiding a `--from <Dockerfile>` flag mention inside the `upgrade-sandboxes` command section. ## Source summary - #5034 -> `docs/reference/troubleshooting.mdx`, `docs/about/release-notes.mdx`: Document safer stale-sandbox recovery through `rebuild --yes` before recreating from scratch. - #5091 -> `docs/reference/troubleshooting.mdx`, `docs/about/release-notes.mdx`: Document Docker-driver post-reboot recovery from OpenShell container labels. - #5101, #5174, #5177 -> `docs/manage-sandboxes/backup-restore.mdx`, `docs/about/release-notes.mdx`: Document OpenClaw `openclaw.json` preservation, merge behavior, and fail-safe restore handling. - #5102 -> `docs/reference/commands.mdx`, `docs/reference/commands-nemohermes.mdx`, `docs/manage-sandboxes/lifecycle.mdx`, `docs/about/release-notes.mdx`: Document `upgrade-sandboxes` image-fingerprint drift detection. - #4201 -> `docs/reference/troubleshooting.mdx`, `docs/about/release-notes.mdx`: Document the installer diagnostic for unexpected Docker daemon access outside the `docker` group. - #5038 -> `docs/inference/inference-options.mdx`, `docs/inference/use-local-inference.mdx`, `docs/about/release-notes.mdx`: Document the interactive managed-vLLM model picker and non-interactive override behavior. - #5040, #5041 -> `docs/reference/troubleshooting.mdx`, `docs/about/release-notes.mdx`: Document Ollama auth-proxy recovery and host DNS preflight diagnostics. - #4986, #5039 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/about/release-notes.mdx`: Document Slack validation and duplicate Slack Socket Mode sandbox handling. - #4981, #5168 -> `docs/about/release-notes.mdx`: Capture Hermes gateway secret-guard and wrapped-argv startup hardening in the release surface. - Follow-up -> `.agents/skills/nemoclaw-contributor-update-docs/SKILL.md`: Record the post-release docs workflow, discussion-announcement lookup, and next-patch release label rule. - Follow-up -> `docs/reference/commands.mdx`, `docs/reference/commands-nemohermes.mdx`: Reword custom Dockerfile sandbox text so CLI parity does not treat `--from` as an `upgrade-sandboxes` flag. ## Verification - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` - `npm run docs` - `npm run build:cli` - `bash test/e2e/e2e-cloud-experimental/check-docs.sh --only-cli` - Skip-term scan for `docs/.docs-skip` blocked terms across generated user skills <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Enhanced local inference setup with interactive model selection prompts and environment variable overrides * Improved sandbox upgrade detection using build fingerprints and version checks * Clarified configuration restore behavior preserving user settings during rebuild/restore * Added gateway authentication as fifth security layer * Expanded Slack messaging validation with live credential checking * Enhanced troubleshooting guidance for Docker access, DNS issues, and sandbox recovery * Updated release notes for v0.0.63 featuring sandbox recovery and inference improvements <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Refreshes the published catalog copies of the generated inference and reference user skills so they match the source `.agents/skills` output after #5038. This addresses the review advisor finding that the root `skills/` packages still had stale managed-vLLM defaults and override slugs. ## Related Issue Follow-up to #5038; addresses #5038 (comment). ## Changes - Refreshed `skills/nemoclaw-user-configure-inference` from the generated `.agents` source while preserving catalog-only metadata and signing artifacts. - Refreshed `skills/nemoclaw-user-reference` from the generated `.agents` source while preserving catalog-only metadata and signing artifacts. - Updated the published skill references to list DGX Spark as `qwen3.6-35b-a3b-nvfp4` / `nvidia/Qwen3.6-35B-A3B-NVFP4`, DGX Station as `qwen3.6-27b` / `Qwen/Qwen3.6-27B-FP8`, generic Linux as `nemotron-3-nano-4b` / `nvidia/NVIDIA-Nemotron-3-Nano-4B-FP8`, and the `deepseek-v4-flash` override slug. - Removed the stale published `use-local-inference-details.md` reference file because the current generated inference skill carries that content in `references/inference-options.md`. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] 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) `npm run docs` completed with 0 errors, but Fern reported two pre-existing/environmental warnings: unauthenticated redirects check skipped, and a light-mode accent contrast warning. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Major overhaul of local inference onboarding and references: Ollama, vLLM, GPU fallbacks, context-window behavior, WSL/Windows-host nuances, install-mode guidance, PATH/zstd hints, and Docker/WSL restriction + remediation. * Consolidated and clarified model-switching, validation/retry/timeout rules, proxy health/token semantics, CLI examples, network policy presets, and troubleshooting. * Removed/merged older local-inference detail pages into centralized guidance. * **Tests** * Added an eval constraint to discourage fabrication when the skill cannot answer. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Summary
Express managed-vLLM install now offers an interactive picker for the curated
VLLM_MODELSregistry instead of silently selecting the per-platform default.NEMOCLAW_VLLM_MODELand non-interactive runs keep their existing behaviour.Related Issue
Resolves #4705
Changes
platforms: VllmPlatform[](spark,station,linux) and addmodelsForPlatform()so the picker only offers models that fit the host.promptVllmModel(newsrc/lib/inference/vllm-prompt.ts, also re-exported frommodel-prompts) that lists the per-profile subset, marks the default as recommended, prints HF ids inline, and re-runsassertGatedModelAccesson the selection.VllmProfilegains aplatformkey and the three profiles are tagged accordingly.installVllm()routes through env override → non-interactive default → interactive picker, keeping the gated-token preflight before any docker side effect.(NEMOCLAW_VLLM_MODEL override).backnavigation.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
Improvements
Tests