Skip to content

test(e2e): avoid inference calls in hosted reachability probe#5411

Merged
cv merged 2 commits into
mainfrom
fix/e2e-hosted-inference-reachability-probe
Jun 14, 2026
Merged

test(e2e): avoid inference calls in hosted reachability probe#5411
cv merged 2 commits into
mainfrom
fix/e2e-hosted-inference-reachability-probe

Conversation

@cv

@cv cv commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Changes the hosted inference E2E preflight from an authenticated chat completion to a lightweight HTTP reachability probe. This avoids spending model requests and tripping HTTP 429 during full parallel nightly runs before the scenario reaches the behavior under test.

Related Issue

Related to #5406 nightly follow-up.

Changes

  • Update test/e2e/lib/ci-compatible-inference.sh so nemoclaw_e2e_probe_hosted_inference checks network/TLS reachability only and accepts any non-000 HTTP response.
  • Add support-test coverage proving the probe does not call /chat/completions.

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)

Verification

  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • 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)

Targeted verification:

  • npx biome check --write test/e2e-scenario/support-tests/hosted-inference.test.ts
  • bash -n test/e2e/lib/ci-compatible-inference.sh
  • npx vitest run --project cli test/e2e-scenario/support-tests/hosted-inference.test.ts

Docs review: no user-facing docs changes needed; this is E2E harness stabilization only.

Note: local broad hooks still fail in unrelated runtime recovery preload tests because temp preload files are seen as group-writable (mode=664), matching prior local hook failures. Targeted changed tests passed.


Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Tests

    • Added end-to-end coverage for hosted inference probe validation, including lightweight reachability assertions and negative cases when connectivity fails or the probe reports errors.
  • Refactor

    • Updated the hosted inference probe to run a lightweight network/TLS connectivity check only, avoiding authenticated requests and preventing inference/auth-related endpoints from being called.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this Jun 14, 2026
@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d6aa061a-ea4a-432d-b4cf-ebf6f55578b7

📥 Commits

Reviewing files that changed from the base of the PR and between 5910ba0 and cfc7a3b.

📒 Files selected for processing (2)
  • test/e2e-scenario/support-tests/hosted-inference.test.ts
  • test/e2e/lib/ci-compatible-inference.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/e2e-scenario/support-tests/hosted-inference.test.ts

📝 Walkthrough

Walkthrough

nemoclaw_e2e_probe_hosted_inference in ci-compatible-inference.sh is simplified from an authenticated probe (issuing /chat/completions or /models requests with an API key) to a single curl call that checks network/TLS reachability by inspecting the HTTP status of the hosted inference base_url. A new E2E test validates this behavior by stubbing curl and asserting the probe does not invoke chat/completions or /models.

Changes

Hosted-inference probe simplification and test coverage

Layer / File(s) Summary
Probe function reduced to curl reachability check
test/e2e/lib/ci-compatible-inference.sh
nemoclaw_e2e_probe_hosted_inference now only curls base_url, captures the HTTP status, and returns success when the status is present and not 000; all authenticated inference and /models request logic is removed.
E2E test suite verifying probe skips authenticated requests
test/e2e-scenario/support-tests/hosted-inference.test.ts
Adds Node.js imports, a COMPAT_HELPER path constant, a runHostedProbe helper that stubs curl in a temp directory, and E2E tests that verify the probe runs successfully while avoiding chat/completions and /models requests; includes negative cases confirming failures on HTTP status 000 or nonzero exit codes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5395: Both PRs adjust the hosted-inference CI probe behavior — this PR removes the chat/completions request from the probe, while that PR defaults NEMOCLAW_PREFERRED_API to openai-completions to avoid stricter chat/response constraints in the same hosted-inference flow.

Poem

🐇 A curl to the sky, just checking the door,
No tokens to send, no model to implore.
The probe hops lightly, status in paw,
"Not 000!" it cheers — that's the only law.
The test stubs the curl, the log tells the tale:
No chat/completions — the rabbit won't fail! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically summarizes the main change: avoiding inference calls in the hosted reachability probe.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 fix/e2e-hosted-inference-reachability-probe

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

@github-code-quality

github-code-quality Bot commented Jun 14, 2026

Copy link
Copy Markdown

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the branch is 96%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File cfc7a3b +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the branch is 44%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File cfc7a3b +/-
src/lib/state/o...oard-session.ts 90%
src/lib/inference/local.ts 77%
src/lib/sandbox/config.ts 72%
src/lib/inference/nim.ts 72%
src/lib/onboard/preflight.ts 64%
src/lib/state/sandbox.ts 55%
src/lib/onboard...er-gpu-patch.ts 50%
src/lib/actions...licy-channel.ts 49%
src/lib/policy/index.ts 48%
src/lib/onboard.ts 17%

Updated June 14, 2026 02:36 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: cloud-onboard-e2e, cloud-e2e, cloud-inference-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None.

Optional E2E

  • cloud-onboard-e2e (medium): Best targeted confidence check for the changed hosted inference preflight: this script sources ci-compatible-inference.sh, calls nemoclaw_e2e_probe_hosted_inference before install/onboard, and then validates hosted compatible onboarding.
  • cloud-e2e (medium): Broader optional smoke for the full install/onboard/sandbox flow using the same hosted inference helper and preflight path.
  • cloud-inference-e2e (medium): Adjacent optional coverage for the hosted compatible inference environment configured by the shared helper, with an actual inference.local chat request after sandbox setup.

New E2E recommendations

  • None.

@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: None
Optional Vitest E2E scenarios: None

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • None. No Vitest live scenario dispatch is required. The PR changes a support-test plus the legacy shell helper test/e2e/lib/ci-compatible-inference.sh; the live Vitest scenario workflow runs registry-scenarios.test.ts and wired test/e2e-scenario/live/*.test.ts jobs, and none of those dispatches directly exercises nemoclaw_e2e_probe_hosted_inference from this shell helper.

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • test/e2e-scenario/support-tests/hosted-inference.test.ts

@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 0 worth checking, 0 nice ideas
Since last review: 3 prior items resolved, 0 still apply, 0 new items found

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

Comment thread test/e2e-scenario/support-tests/hosted-inference.test.ts Fixed
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv merged commit 129717f into main Jun 14, 2026
44 checks passed
@cv cv deleted the fix/e2e-hosted-inference-reachability-probe branch June 14, 2026 02:54
@cv cv added the v0.0.65 Release target label Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v0.0.65 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants