Skip to content

fix(inference): honor host-local serving recipe contracts - #8391

Merged
cv merged 2 commits into
mainfrom
agent/8246-host-local-spark-fixes
Aug 6, 2026
Merged

fix(inference): honor host-local serving recipe contracts#8391
cv merged 2 commits into
mainfrom
agent/8246-host-local-spark-fixes

Conversation

@prekshivyas

@prekshivyas prekshivyas commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Serving presets can now evaluate schema-valid readiness observation comparisons. Host-local vLLM recipes with an empty runtime environment now produce a valid serving command instead of a leading &&.

Related Issue

Refs #8246

Changes

  • Evaluate equals, one-of, at-least, and version-at-least observation comparisons against typed readiness values.
  • Compare numeric dotted versions segment by segment and reject invalid version values.
  • Omit the vLLM environment prefix when runtime.environment is empty.
  • Add regression tests for matching and rejected readiness comparisons and for the empty-environment command.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: The change restores existing internal serving-catalog and vLLM command contracts. It adds no command, flag, schema field, default, or public workflow.
  • 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:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: The review of commit d6491799c found that the change restores existing internal behavior and does not change a documented user-facing surface. No terminology, structure, voice, or test-title findings were reported.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable.
  • Station profile/scenario: Not applicable.
  • Result: Not applicable.
  • Supporting evidence: Not applicable.

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project cli src/lib/inference/serving/resolver.test.ts src/lib/inference/vllm-models.test.ts: 2 files and 73 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: Not run; targeted source tests cover both changed behaviors.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Prekshi Vyas prekshiv@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Readiness requirements now correctly support string, membership, numeric minimum, and version minimum comparisons.
    • Version comparisons safely handle dotted numeric versions and reject insufficient or malformed values.
    • Serving commands no longer include an invalid command separator when no environment overrides are configured.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas prekshivyas added area: inference Inference routing, serving, model selection, or outputs platform: dgx-spark Affects DGX Spark hardware or workflows labels Aug 5, 2026
@prekshivyas prekshivyas self-assigned this Aug 5, 2026
@prekshivyas prekshivyas added area: inference Inference routing, serving, model selection, or outputs platform: dgx-spark Affects DGX Spark hardware or workflows labels Aug 5, 2026
@github-code-quality

github-code-quality Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit d649179 in the agent/8246-host-loca... branch remains at 96%, unchanged from commit b148bc1 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit d649179 in the agent/8246-host-loca... branch remains at 82%, unchanged from commit b148bc1 in the main branch.

Show a code coverage summary of the most impacted files.
File main b148bc1 agent/8246-host-loca... d649179 +/-
src/lib/domain/.../connect-env.ts 97% 89% -8%
src/lib/inferen.../vllm-models.ts 86% 86% 0%
src/lib/sandbox...rce-identity.ts 87% 87% 0%
src/lib/inferen...ing/resolver.ts 89% 90% +1%

Updated August 05, 2026 20:48 UTC

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The resolver now evaluates equality, membership, numeric minimum, and semantic version minimum readiness comparisons. The vLLM command builder omits empty serving environment exports.

Changes

Readiness comparison evaluation

Layer / File(s) Summary
Typed readiness comparison flow
src/lib/inference/serving/resolver.ts, src/lib/inference/serving/resolver.test.ts
The resolver evaluates comparison requirements against matching observations. Tests cover successful matches and rejection for mismatched, insufficient, and malformed values.

vLLM command generation

Layer / File(s) Summary
Conditional serving environment prefix
src/lib/inference/vllm-models.ts, src/lib/inference/vllm-models.test.ts
buildVllmServeCommand omits the export prefix when serveEnv is empty. A regression test verifies the command starts with the installation and serve commands.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ReadinessResolver
  participant ReadinessObservations
  participant ServingPreset
  ReadinessResolver->>ReadinessObservations: Find observation by ID
  ReadinessResolver->>ReadinessResolver: Evaluate comparison
  ReadinessResolver->>ServingPreset: Accept or reject readiness requirements
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the inference serving changes that enforce host-local serving recipe contracts.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/8246-host-local-spark-fixes

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/resolver.test.ts`:
- Around line 502-596: Extend the resolver-level test “matches typed readiness
observation comparisons (`#8246`)” with a malformed observed
host.gpu.driver_version value such as “580.65.x”. Invoke
resolveManagedInferenceServing through the existing resolverInput and
comparedCatalog boundary, and assert the result is rejected with code
“requirements-not-met”.
🪄 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: 03122dc9-6014-4d00-a1f7-1e8423f18575

📥 Commits

Reviewing files that changed from the base of the PR and between c6713c9 and bcedc5b.

📒 Files selected for processing (4)
  • src/lib/inference/serving/resolver.test.ts
  • src/lib/inference/serving/resolver.ts
  • src/lib/inference/vllm-models.test.ts
  • src/lib/inference/vllm-models.ts

Comment thread src/lib/inference/serving/resolver.test.ts Outdated
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized terminology decisions differ; normalized E2E selections differ; severity counts match.
3 additional E2E selections from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

  • spark-install: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
  • vllm-docker-storage: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
  • llama-cpp-dgx-spark-qualification: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate.

1 semantic terminology decision

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

  • justified — readiness observation comparison at src/lib/inference/serving/resolver.test.ts:502: Keep “readiness observation comparisons” where the contrast with other readiness requirement types matters.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: inference-routing, network-policy

Workflow run details

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>

@apurvvkumaria apurvvkumaria left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve — reviewed current head d649179 with focus on readiness comparison semantics, version parsing, fail-closed selection, vLLM command construction, compatibility, and tests. No blocking defect found; required checks are green.

@cv
cv merged commit 054b5ff into main Aug 6, 2026
95 of 97 checks passed
@cv
cv deleted the agent/8246-host-local-spark-fixes branch August 6, 2026 04:32
cv added a commit that referenced this pull request Aug 6, 2026
<!-- markdownlint-disable MD041 -->
## Summary

The `main` branch fails CLI type checking because it contains two
private `readinessComparisonMatches` functions after changes from #8399
and #8391 were combined.
This change removes the superseded implementation and retains the dotted
numeric version comparison covered by the resolver tests.
It also aligns the positive storage-remediation test data with the
host-local preset that the test resolves.

## Changes

- Remove the earlier `versionAtLeast` helper and duplicate
`readinessComparisonMatches` function.
- Retain `compareNumericDottedVersions` and the single resolver call
path added by #8391.
- Add a resolver regression for a version segment above
`Number.MAX_SAFE_INTEGER`.
- Build the positive storage-remediation test data from the host-local
preset under test.

## Type of Change

- [x] Code change (feature, bug fix, or refactor)
- [ ] 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:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: This removes a duplicate
private implementation and preserves the resolver contract, schema, call
site, and user-visible behavior.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Independent Codex
Desktop review passed all nine security categories for PR commit
`05999730c`; the deletion retains the stricter validation path, and the
test-fixture change does not alter production behavior.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Documentation Writer Review

- [x] Documentation writer subagent reviewed the completed changes
- Result: `no-docs-needed`
- Evidence: PR commit `05999730c` contains the duplicate-removal repair,
resolver regression, and test-fixture alignment without changing a
user-visible API, CLI, configuration, workflow, default, error, or
supported behavior.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 0599973 -->
<!-- docs-review-agents-blob-sha: c69aad4 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable.
- Station profile/scenario: Not applicable.
- Result: Not applicable.
- Supporting evidence: Not applicable.

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run validate:pr` passed after refreshing `origin/main` when hooks
were skipped or unavailable
- [ ] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification:
GitHub Actions is running for PR commit `05999730c`; no standalone local
test suite ran.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: Required GitHub Actions
checks are pending; no standalone local test suite ran.
- [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)
- [ ] 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: Carlos Villela <cvillela@nvidia.com>

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cjagwani cjagwani mentioned this pull request Aug 7, 2026
23 tasks
cjagwani added a commit that referenced this pull request Aug 7, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds the canonical dated changelog entry required before cutting
`v0.0.104`.
The entry reconciles user-facing changes merged from `v0.0.103` through
`8d2b86aaf44968b4f7bc3b714222a73bd28e0403` while excluding hidden and
experimental product surfaces.

## Changes

- Added `docs/changelog/2026-08-06.mdx` with the exact `## v0.0.104`
heading and release themes for local inference, private endpoints,
network policy, state authority, lifecycle recovery, uninstall, Hermes,
MCP diagnostics, credential safety, and installation guidance.
- Source summary links:
- [#8399](#8399) ->
`docs/changelog/2026-08-06.mdx`: fixed DGX Spark local serving profiles.
- [#8418](#8418) ->
`docs/changelog/2026-08-06.mdx`: durable llama.cpp lifecycle management.
- [#8422](#8422) ->
`docs/changelog/2026-08-06.mdx`: recoverable llama.cpp receipt
publication.
- [#8402](#8402) ->
`docs/changelog/2026-08-06.mdx`: remediable DGX Spark storage admission.
- [#8391](#8391) ->
`docs/changelog/2026-08-06.mdx`: host-local serving recipe contracts.
- [#8401](#8401) ->
`docs/changelog/2026-08-06.mdx`: serving profile lifecycle provenance.
- [#8322](#8322) ->
`docs/changelog/2026-08-06.mdx`: guarded llama.cpp route compatibility.
- [#8272](#8272) ->
`docs/changelog/2026-08-06.mdx`: explicitly trusted private endpoints
with stable policy pins and CA trust.
- [#8431](#8431) ->
`docs/changelog/2026-08-06.mdx`: Personal onboarding policy tier and its
trust boundary.
- [#8143](#8143) ->
`docs/changelog/2026-08-06.mdx`: manifest-derived state authority.
- [#7859](#7859) ->
`docs/changelog/2026-08-06.mdx`: side-effect-free lifecycle lock
timeouts.
- [#8262](#8262) ->
`docs/changelog/2026-08-06.mdx`: managed gateway lease waiting.
- [#8339](#8339) ->
`docs/changelog/2026-08-06.mdx`: continued journaled rebuild recreation.
- [#8373](#8373) ->
`docs/changelog/2026-08-06.mdx`: restore readiness after compatibility
decisions.
- [#8443](#8443) ->
`docs/changelog/2026-08-06.mdx`: fail-closed malformed registry
handling.
- [#8419](#8419) ->
`docs/changelog/2026-08-06.mdx`: bounded recovery for a gateway that
never served.
- [#8486](#8486) ->
`docs/changelog/2026-08-06.mdx`: target-scoped registry recovery.
- [#8259](#8259) ->
`docs/changelog/2026-08-06.mdx`: scoped uninstall ordering and retry
safety.
- [#8457](#8457) ->
`docs/changelog/2026-08-06.mdx`: desktop metadata exclusion during
uninstall.
- [#8026](#8026) ->
`docs/changelog/2026-08-06.mdx`: typed Hermes configuration policy.
- [#8242](#8242) ->
`docs/changelog/2026-08-06.mdx`: Hermes WhatsApp session diagnostics.
- [#8344](#8344) ->
`docs/changelog/2026-08-06.mdx`: patched Hermes image and dependency
checks.
- [#8491](#8491) ->
`docs/changelog/2026-08-06.mdx`: bounded MCP discovery timeout.
- [#8490](#8490) ->
`docs/changelog/2026-08-06.mdx`: MCP shadow diagnostics.
- [#7619](#7619) ->
`docs/changelog/2026-08-06.mdx`: web-search credential isolation.
- [#8476](#8476) ->
`docs/changelog/2026-08-06.mdx`: stable preflight advisory identifiers.
- [#8452](#8452) ->
`docs/changelog/2026-08-06.mdx`: user-local CLI resolution.
- [#8481](#8481) ->
`docs/changelog/2026-08-06.mdx`: remote network-policy terminal
guidance.
- Product-scope exclusions:
[#8429](#8429) remains
experimental; [#8261](#8261)
remains feature-gated; and portable-profile changes
[#8408](#8408),
[#8415](#8415),
[#8446](#8446),
[#8458](#8458),
[#8462](#8462), and
[#8506](#8506) are not promoted
as supported product surfaces.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification: `npx vitest
run test/changelog-docs.test.ts` passed 6/6 and validates dated
changelog structure and published links.
- [ ] 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:

## Documentation Writer Review

- [x] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated`
- Evidence: `docs/changelog/2026-08-06.mdx`; release-range scope,
writing rules, documentation style, skip terms, exact names,
threat-boundary wording, and published routes reviewed; changelog tests
and docs build passed.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 02b51ae -->
<!-- docs-review-agents-blob-sha: c69aad4 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable; no DGX Station host preparation script
changed.
- Station profile/scenario: Not applicable.
- Result: Not applicable.
- Supporting evidence: Not applicable.

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run validate:pr` passed after refreshing `origin/main` when hooks
were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: `npx
vitest run test/changelog-docs.test.ts` passed 6/6.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: Not applicable to a single
changelog entry.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [x] `npm run docs` builds without warnings (doc changes only)
- [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)

The new dated changelog file includes the required parser-safe SPDX
header and intentionally has no frontmatter, matching the changelog
contract and existing entries.

---
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
  * Added release notes for v0.0.104.
* Documented fixes for local model runtimes, private endpoints, network
policies, state recovery, uninstall behavior, safety updates, MCP
diagnostics, credential isolation, and installation guidance.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: inference Inference routing, serving, model selection, or outputs platform: dgx-spark Affects DGX Spark hardware or workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants