refactor(onboard): extract provider prompt selection#5175
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
📝 WalkthroughWalkthroughThis PR extracts the interactive inference-provider selection logic from the onboarding flow into a new reusable ChangesProvider Selection Prompt Extraction
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
PR Review AdvisorFindings: 0 needs attention, 0 worth checking, 0 nice ideas Consider writing more tests for
This is an automated advisory review. A human maintainer must make the final merge decision. |
Selective E2E Results — ✅ All requested jobs passedRun: 27305045685
|
|
Validation update for the onboard slimming stack slice:
Leaving this as draft while the lower stack settles: #5165 ready, #5171 draft, #5173 draft, #5175 draft. |
|
Next onboard slimming slice opened above this draft: #5176. Implementation note: #5176 extracts only the non-interactive provider-selection failure reporting from Local validation before opening #5176: Biome format/lint on touched files, |
…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
# Conflicts: # src/lib/onboard.ts # src/lib/onboard/provider-host-state.test.ts # src/lib/onboard/provider-host-state.ts
There was a problem hiding this comment.
🧹 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.tscore 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
📒 Files selected for processing (3)
src/lib/onboard.tssrc/lib/onboard/provider-selection-prompt.test.tssrc/lib/onboard/provider-selection-prompt.ts
Summary
Extracts the interactive inference-provider prompt rendering out of
setupNiminto 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
src/lib/onboard/provider-selection-prompt.tsfor detected local-provider suggestions, menu rendering,NEMOCLAW_PROVIDERdefault hint handling, prompting, and numbered-choice delegation.setupNimwith a call to the new helper.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
Release Notes