Skip to content

refactor(onboard): extract provider prompt selection#5175

Merged
cv merged 11 commits into
mainfrom
codex/onboard-provider-prompt-flow
Jun 11, 2026
Merged

refactor(onboard): extract provider prompt selection#5175
cv merged 11 commits into
mainfrom
codex/onboard-provider-prompt-flow

Conversation

@cv

@cv cv commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Extracts the interactive inference-provider prompt rendering out of setupNim into a focused helper. This keeps the prompt/default-selection behavior unchanged while making the provider menu presentation contract directly testable.

Related Issue

Refs #3802

Changes

  • Add src/lib/onboard/provider-selection-prompt.ts for detected local-provider suggestions, menu rendering, NEMOCLAW_PROVIDER default hint handling, prompting, and numbered-choice delegation.
  • Add unit coverage for prompt rendering, vLLM/Ollama suggestions, env-hint defaulting, and fallback to the build provider.
  • Replace the inline interactive provider prompt block in setupNim with a call to the new helper.

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

  • npx prek run --all-files passes
  • npm test passes
  • 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)

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

Summary by CodeRabbit

Release Notes

  • Improvements
    • Enhanced inference provider selection with clearer prompts and a numbered menu interface for easier navigation.
    • Added automatic detection and user-friendly display of available local inference options, including vLLM and Ollama.
    • Support for environment-based provider configuration to streamline onboarding workflows.

@cv cv added refactor PR restructures code without intended behavior change area: cli Command line interface, flags, terminal UX, or output area: architecture Architecture, design debt, major refactors, or maintainability labels Jun 10, 2026
@cv cv self-assigned this Jun 10, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jun 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR extracts the interactive inference-provider selection logic from the onboarding flow into a new reusable promptForInferenceProviderSelection module. The module handles environment-driven defaults, local inference detection, numbered menu presentation, and user selection, with comprehensive test coverage for all scenarios.

Changes

Provider Selection Prompt Extraction

Layer / File(s) Summary
Provider selection prompt module
src/lib/onboard/provider-selection-prompt.ts
Generic typed input interface, default provider index computation (from NEMOCLAW_PROVIDER env with build fallback), local inference suggestion detection, and main async function that logs suggestions, prints numbered menu, prompts user with computed default, and returns selection.
Provider selection prompt tests
src/lib/onboard/provider-selection-prompt.test.ts
Test setup with provider options and helpers, then four test cases: (1) basic menu rendering with default build selection, (2) local inference detection and logging, (3) environment-variable-driven default selection, (4) fallback behavior with reordered options.
Onboarding flow integration
src/lib/onboard.ts
Imports promptForInferenceProviderSelection and replaces inline interactive provider menu construction with delegated call, passing options, local-inference flags, prompt/log callbacks, and menu-selection helper.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5173: Refactor of surrounding provider-selection flow logic in src/lib/onboard.ts complementary to this prompt extraction.
  • NVIDIA/NemoClaw#5165: Also modifies provider menu construction in src/lib/onboard.ts; this PR consumes the built options via the new delegated promptForInferenceProviderSelection call.
  • NVIDIA/NemoClaw#5171: Both PRs refactor local inference state detection (vllmRunning/ollamaRunning flags) and use those same flags to drive provider-menu prompt logic.

Poem

🐰 The rabbit hops through provider land,
Where menus prompt with guiding hand,
Env hints default, locals suggest,
A prompt extracted—clean and blessed!
extracted

🚥 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 title accurately summarizes the main change: extracting the provider prompt selection logic into a dedicated helper module for better testability.
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 codex/onboard-provider-prompt-flow

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

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: branch-validation-full
Optional E2E: onboard-negative-paths-vitest, cloud-onboard-e2e

Dispatch hint: test_suite=full

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • branch-validation-full (high; provisions Brev CPU instance and uses NVIDIA_API_KEY): Core onboarding code changed. Run the source-install full branch validation to prove install.sh → onboard → sandbox creation → inference configuration still works end-to-end after the new provider-selection prompt module is imported from onboard.ts.

Optional E2E

  • onboard-negative-paths-vitest (medium; live CLI checks, no full sandbox happy path required): Useful focused confidence for onboard edge cases and non-interactive provider/env handling, including NEMOCLAW_PROVIDER=cloud being honored. Optional because the PR primarily changes the interactive prompt path and adds direct unit tests for that helper.
  • cloud-onboard-e2e (high; Docker sandbox plus NVIDIA_API_KEY): Optional broader coverage for non-interactive cloud provider onboarding via installer and sandbox health/security checks. Helpful if maintainers want extra confidence that provider defaults/env selection still integrate with cloud onboard, but less directly targeted than branch-validation-full.

New E2E recommendations

  • interactive onboarding provider selection (high): No existing E2E appears to exercise the interactive provider menu after this extraction. Add a pty/expect-style E2E that runs onboard interactively with multiple provider options, verifies the detected local inference suggestion text, verifies Enter accepts the default provider, verifies NEMOCLAW_PROVIDER changes the default index, and verifies a numeric choice flows into provider/model setup or a controlled stubbed onboard phase.
    • Suggested test: Add test/e2e/test-interactive-provider-selection.sh or a Vitest live pty scenario for the interactive provider-selection menu.

Dispatch hint

  • Workflow: .github/workflows/e2e-branch-validation.yaml
  • jobs input: test_suite=full

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: ubuntu-repo-cloud-openclaw
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • ubuntu-repo-cloud-openclaw: The PR refactors the provider-selection prompt integration in src/lib/onboard.ts and adds a new onboard helper module. The smallest live-supported Vitest registry scenario that validates the repo-built CLI can still enter the standard cloud OpenClaw onboarding path is ubuntu-repo-cloud-openclaw.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • src/lib/onboard.ts
  • src/lib/onboard/provider-selection-prompt.ts

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

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

Consider writing more tests for
  • **Runtime validation** — Interactive CLI/runtime validation that bare Enter in the provider menu selects the displayed default build provider through the full onboarding call path.. Helper-level unit coverage is appropriate and covers the main behavior changes. Because this is wired into the interactive onboarding path, targeted runtime validation would improve confidence that the extraction remains behavior-preserving in the full CLI flow.
  • **Runtime validation** — Interactive CLI/runtime validation that NEMOCLAW_PROVIDER=OPENAI changes the displayed default and selected provider through the full onboarding call path.. Helper-level unit coverage is appropriate and covers the main behavior changes. Because this is wired into the interactive onboarding path, targeted runtime validation would improve confidence that the extraction remains behavior-preserving in the full CLI flow.
  • **Runtime validation** — Unit test that promptForInferenceProviderSelection passes exit and/or quit replies through to the injected numbered-menu selector without swallowing existing exit behavior.. Helper-level unit coverage is appropriate and covers the main behavior changes. Because this is wired into the interactive onboarding path, targeted runtime validation would improve confidence that the extraction remains behavior-preserving in the full CLI flow.
  • **Runtime validation** — Unit test or explicit precondition for promptForInferenceProviderSelection when options lacks both a matching env provider and a build entry.. Helper-level unit coverage is appropriate and covers the main behavior changes. Because this is wired into the interactive onboarding path, targeted runtime validation would improve confidence that the extraction remains behavior-preserving in the full CLI flow.
  • **Acceptance clause:** PR body: Refs Umbrella: refactor onboarding into a serializable FSM #3802 — add test evidence or identify existing coverage. The deterministic linkedIssues array is empty, so no trusted issue Umbrella: refactor onboarding into a serializable FSM #3802 body or comment clauses were available to extract literally or map to this diff. The patch only extracts provider prompt rendering/default-selection logic and adds unit tests for that helper.

Workflow run details

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

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27305045685
Target ref: 5ed50a80a4036a2e5ac4ea324dd08ed8d0ec4e70
Workflow ref: main
Requested jobs: cloud-onboard-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ✅ success

@cv

cv commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

Validation update for the onboard slimming stack slice:

  • This PR keeps the provider credential/model setup in setupNim and only moves the interactive provider prompt/default-handling path into promptForInferenceProviderSelection.
  • Local validation passed: Biome format/lint on touched files, npm run build:cli, npm run typecheck:cli, focused Vitest provider/onboard suite, and git diff --check.
  • Standard PR CI is green, merge state is clean, PR Review Advisor is 0/0/0, and CodeRabbit has no actionable feedback because this remains draft.
  • Required live validation passed: cloud-onboard-e2e in https://github.com/NVIDIA/NemoClaw/actions/runs/27305045685.

Leaving this as draft while the lower stack settles: #5165 ready, #5171 draft, #5173 draft, #5175 draft.

@cv

cv commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

Next onboard slimming slice opened above this draft: #5176.

Implementation note: #5176 extracts only the non-interactive provider-selection failure reporting from setupNim into reportProviderSelectionFailure. The final process.exit(1) remains in setupNim, so the control-flow ownership stays where it was; the new helper only owns existing stderr messages and the existing Windows-host Ollama rejection callback. Focused tests cover every current failure reason.

Local validation before opening #5176: Biome format/lint on touched files, npm run build:cli, npm run typecheck:cli, focused Vitest provider/onboard suite, and git diff --check.

@cv cv added the v0.0.64 Release target label Jun 10, 2026
cv added 4 commits June 10, 2026 23:48
…nu-merge-update

# Conflicts:
#	test/e2e/test-onboard-inference-smoke.sh
…' into codex/tmp-provider-host-state-merge-update
…e-flow' into codex/tmp-provider-selection-merge-update
…-flow' into codex/tmp-provider-prompt-merge-update
@cv cv added the area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow label Jun 11, 2026
Base automatically changed from codex/onboard-provider-selection-flow to main June 11, 2026 08:50
# Conflicts:
#	src/lib/onboard.ts
#	src/lib/onboard/provider-host-state.test.ts
#	src/lib/onboard/provider-host-state.ts
@cv cv marked this pull request as ready for review June 11, 2026 08:56

@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.

🧹 Nitpick comments (1)
src/lib/onboard.ts (1)

3869-3876: Run the targeted onboarding E2E matrix before merge.

Given this is in src/lib/onboard.ts core flow, please run the recommended selective jobs to de-risk regressions in provider-selection onboarding paths. As per coding guidelines, this file affects full sandbox creation/configuration and has a specified E2E recommendation list.

🤖 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 3869 - 3876, Summary: The onboarding
provider-selection flow (promptForInferenceProviderSelection with
selectFromNumberedMenuOrExit) must be validated by running the targeted
onboarding E2E matrix before merging. Run the selective E2E jobs that exercise
provider-selection paths (e.g., vllm-only, ollama-only, interactive prompt
selection, and the combined sandbox creation path) to de-risk regressions:
execute those jobs locally or in CI, verify success for each path, capture
logs/screenshots for failures, and add the job results to the PR description; if
any failure appears, reproduce, fix the failing branch in
promptForInferenceProviderSelection or selectFromNumberedMenuOrExit, and re-run
the affected E2E job until green.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 3869-3876: Summary: The onboarding provider-selection flow
(promptForInferenceProviderSelection with selectFromNumberedMenuOrExit) must be
validated by running the targeted onboarding E2E matrix before merging. Run the
selective E2E jobs that exercise provider-selection paths (e.g., vllm-only,
ollama-only, interactive prompt selection, and the combined sandbox creation
path) to de-risk regressions: execute those jobs locally or in CI, verify
success for each path, capture logs/screenshots for failures, and add the job
results to the PR description; if any failure appears, reproduce, fix the
failing branch in promptForInferenceProviderSelection or
selectFromNumberedMenuOrExit, and re-run the affected E2E job until green.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d6ec3d46-239a-4f19-9b38-659d2f264ce8

📥 Commits

Reviewing files that changed from the base of the PR and between 463d59f and 0b29e92.

📒 Files selected for processing (3)
  • src/lib/onboard.ts
  • src/lib/onboard/provider-selection-prompt.test.ts
  • src/lib/onboard/provider-selection-prompt.ts

@cv cv merged commit fa3adc2 into main Jun 11, 2026
37 of 39 checks passed
@cv cv deleted the codex/onboard-provider-prompt-flow branch June 11, 2026 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: architecture Architecture, design debt, major refactors, or maintainability area: cli Command line interface, flags, terminal UX, or output area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow refactor PR restructures code without intended behavior change v0.0.64 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants