feat(inference): add host-local vLLM serving presets - #8249
Conversation
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds host-local DGX Spark vLLM catalog definitions, selection and materialization, topology-independent resolution, installer integration, provenance labels, and shell-safe vLLM command generation. ChangesHost-local vLLM serving
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Catalog
participant Resolver
participant HostLocalSelection
participant VllmInstaller
participant Docker
Catalog->>Resolver: provide host-local preset and recipe
Resolver->>HostLocalSelection: materialize selected vLLM profile
HostLocalSelection-->>Resolver: return model and catalog metadata
Resolver->>VllmInstaller: install selected profile
VllmInstaller->>Docker: run vLLM container with provenance labels
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 27be8a6 in the TypeScript / code-coverage/cliThe overall coverage in commit 27be8a6 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Quote catalog-derived model arguments and environment values at the shared shell command boundary. Reject invalid environment names. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
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 `@src/lib/inference/serving/adapter-registry.ts`:
- Around line 255-275: Introduce a distinct discriminated host-local recipe type
and use it in ResolvedHostLocalInferenceSelection instead of
ManagedInferenceServingRecipe, with distributed execution fields and
spec.bindings explicitly unavailable. Update
validateHostLocalVllmMaterializerRecipe and the related test to consume this
type directly and remove the current unknown/Record casts while preserving the
existing validation behavior.
In `@src/lib/inference/serving/managed-cluster-installer.ts`:
- Around line 434-437: Update the preview branch in the managed-cluster
installation flow around the topologyQualification check to return a
not-selected outcome for host-local presets instead of handled, allowing
runVllmInstall to continue into resolveHostLocalVllmSelection. Preserve the
revalidation branch’s fail-closed handled behavior, and add a regression test
covering a ready capability with an explicitly selected host-local preset.
In `@src/lib/inference/vllm.ts`:
- Around line 1706-1707: Update the hostLocalSelection branch in the inference
resolution flow to handle gated recipes before any Docker work occurs. When
recipe.spec.model.gated is true, invoke the existing assertGatedModelAccess
check before assigning resolved or proceeding to pullImage; otherwise preserve
the current host-local selection behavior.
🪄 Autofix
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: 0a1e85a7-7dcb-4ac8-a282-84c65535c909
📒 Files selected for processing (15)
src/lib/inference/serving/adapter-registry.test.tssrc/lib/inference/serving/adapter-registry.tssrc/lib/inference/serving/catalog-loader.test.tssrc/lib/inference/serving/catalog-loader.tssrc/lib/inference/serving/host-local-vllm-selection.tssrc/lib/inference/serving/managed-cluster-installer.tssrc/lib/inference/serving/managed-cluster-materialize.tssrc/lib/inference/serving/resolver.test.tssrc/lib/inference/serving/resolver.tssrc/lib/inference/serving/types.tssrc/lib/inference/serving/vllm-managed-support.tssrc/lib/inference/vllm-models.test.tssrc/lib/inference/vllm-models.tssrc/lib/inference/vllm.test.tssrc/lib/inference/vllm.ts
Signed-off-by: Prekshi Vyas <prekshiv@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/serving/types.ts`:
- Around line 118-138: Update HostLocalInferenceServingRecipe so omitted
host-local fields, including bindings and execution’s cluster-only properties,
use optional never types rather than required never properties. Adjust the
shared recipe input and related type guards to accept host-local recipes without
requiring structural compatibility with ManagedInferenceServingRecipe, while
preserving existing managed-cluster narrowing. Remove the resulting as unknown
as HostLocalInferenceServingRecipe and direct assertions, and ensure all
TypeScript CLI and plugin type-check configurations remain compatible.
🪄 Autofix
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: 28f3fd1c-30b8-41a7-b7f9-bf641ea7df87
📒 Files selected for processing (7)
src/lib/inference/serving/adapter-registry.tssrc/lib/inference/serving/managed-cluster-installer.test.tssrc/lib/inference/serving/managed-cluster-installer.tssrc/lib/inference/serving/resolver.tssrc/lib/inference/serving/types.tssrc/lib/inference/vllm.test.tssrc/lib/inference/vllm.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/inference/serving/managed-cluster-installer.ts
- src/lib/inference/serving/resolver.ts
- src/lib/inference/vllm.ts
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
senthilr-nv
left a comment
There was a problem hiding this comment.
Reviewed head 27be8a6 against base cbabb66. Product scope passes for the preparatory #8246 subset; all nine security categories pass with no findings. The 176 focused tests, CLI type-check, CLI build, repository checks, and hooks passed, and current automated review reports zero actionable findings.
Summary
Add the model-independent host-local vLLM serving contract required for optional single-host DGX Spark recipes. The change connects topology-free catalog selections to the existing managed local vLLM installer while preserving the automatic DGX Spark default.
No public model recipe, checkpoint, image, revision, or private release detail is included. A future public model requires a separately approved explicit-only recipe and preset.
Related Issue
Related to #8246.
Changes
vllm.host-local/v1materializer and lifecycle descriptors.NEMOCLAW_SERVING_PRESETagainst a fresh single-host readiness report.The serving catalog needs one topology-free adapter contract because the existing adapter requires distributed topology inputs. Direct installer flags would duplicate recipe settings in the installer and shell boundaries. The adapter, catalog, resolver, installer, vLLM command, and provenance-label tests protect this contract.
Type of Change
Quality Gates
27be8a665found no security findings across the nine repository security categories. Focused tests include shell-injection inputs, conflict rejection, gated-model access, and failure-before-effects behavior.Documentation Writer Review
no-docs-needed27be8a665againstcbabb66bf. No shipped host-local vLLM recipe or preset, public CLI or configuration contract, default, documentation, Fern source, or managed-inference definition changed. Existing automatic DGX Spark behavior and the documented two-DGX Spark preset remain unchanged. The change adds internal adapter, resolver, materialization, provenance-label, and shell-quoting support for a future separately approved recipe.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run --project clifor the seven affected test files passed 176 tests on27be8a665;npm run typecheck:cli,npm run build:cli, andnpm run checks:repositorypassed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable. The focused source tests, CLI type-check, CLI build, repository checks, pre-commit hook, commit-msg hook, and pre-push hook cover this adapter and installer change.npm run docsbuilds without warnings (doc changes only)Follow-up
Signed-off-by: Prekshi Vyas prekshiv@nvidia.com