refactor(onboard): extract provider selection resolver#5173
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR extracts provider-selection resolution logic into a new ChangesProvider Selection Resolver
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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: Auto-dispatched E2E: 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, 1 worth checking, 1 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 27303793577
|
|
Validation update for
Remaining advisory note is optional/future coverage for deeper provider-selection runtime cases such as recorded-provider recovery and WSL Ollama host-boundary regression; the required advisor-selected runtime checks for this draft are green. |
|
Next draft stack slice planned on top of this PR: extract the interactive provider prompt/menu rendering from Proposed boundary:
This should be behavior-preserving and small enough for a draft PR: it only makes the prompt/default-selection contract directly testable and trims the next chunk of presentation logic out of |
|
Next draft stack slice opened: #5175 ( What moved:
Coverage added:
Local validation on head
Known local environment note remains: full |
…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
# Conflicts: # src/lib/onboard.ts # src/lib/onboard/provider-host-state.test.ts # src/lib/onboard/provider-host-state.ts
Selective E2E Results — ✅ All requested jobs passedRun: 27334817467
|
Summary
Extracts non-interactive and resume provider selection out of
setupNiminto a focused resolver so the onboarding flow can keep shrinking without changing provider behavior. The resolver returns structured success/failure outcomes whilesetupNimpreserves the existing user-facing error messages.Related Issue
Refs #3802
Changes
src/lib/onboard/provider-selection.tsfor requested-provider fallback, recorded-provider recovery, and provider-selection failure classification.setupNimwith a resolver call plus existing message rendering.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
New Features
Bug Fixes
Tests