fix(cli): apply bracketed default on empty interactive prompt#4389
fix(cli): apply bracketed default on empty interactive prompt#4389jason-ma-nv wants to merge 1 commit into
Conversation
`promptOrDefault` displayed the default value in the prompt label (e.g. `Choose [6]:`) but only substituted that default in non-interactive mode. In interactive mode it returned the raw `deps.prompt()` reply, which is the empty string on bare Enter, leaving every interactive caller to re-implement the fallback. The resource-profile prompt did not, so pressing Enter at `Choose [6]:` exited with `Invalid resource profile selection ''.` even though the prompt advertised 6 as the default. Fall back to `defaultValue` when the interactive reply is empty or whitespace-only. Callers that already had their own `|| "25%"`-style fallback continue to work unchanged — the helper-level default fires first and the caller's redundant branch never triggers. Add `prompt-helpers.test.ts` pinning the new behavior (empty → default, whitespace → default, non-empty → verbatim). The function previously had no direct tests; the interactive branch was the untested path that hid this regression. Fixes #4387. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Jason Ma <jama@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 (2)
📝 WalkthroughWalkthroughThis PR fixes a bug where ChangesPrompt Default Value Fallback
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
PR Review AdvisorFindings: 0 needs attention, 0 worth checking, 0 nice ideas This is an automated advisory review. A human maintainer must make the final merge decision. |
1 participant
Summary
promptOrDefaultdisplayed the bracketed default value (e.g.Choose [6]:) but only substituted that default in non-interactive mode. In interactive mode it returned the rawdeps.prompt()reply, which is the empty string on bare Enter — so every interactive caller had to re-implement the empty-reply fallback. The resource-profile prompt did not, so pressing Enter atChoose [6]:exited withInvalid resource profile selection ''.even though the prompt advertised 6 as the default.Related Issue
Fixes #4387.
Changes
src/lib/onboard/prompt-helpers.ts: in the interactive branch ofpromptOrDefault, fall back todefaultValuewhen the reply is empty or whitespace-only. Callers that already had their own|| "25%"-style fallback continue to work unchanged — the helper-level default fires first and their branch is now a redundant no-op.src/lib/onboard/prompt-helpers.test.ts: new test file (promptOrDefaultpreviously had no direct tests; the interactive branch was the untested path that hid this regression). Three#4387tests pin empty → default, whitespace → default, non-empty → verbatim.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Signed-off-by: Jason Ma jama@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests