-
Notifications
You must be signed in to change notification settings - Fork 88
Fix welcome screen theme violation and unwanted profile display #1027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughApply theme-aware foreground color to WelcomeOnboarding text nodes and add memoized suppression logic in the onboarding hook to skip the welcome flow when environment/profile/provider/model indicate a preconfigured setup; no exported API surface changes. Changes
Sequence Diagram(s)(omitted — changes are presentation and a local readiness check without a multi-component sequential flow that benefits from visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
LLxprt PR Review – PR #1027Issue Alignment
Side Effects
Code Quality
Tests & Coverage
Verdict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts (1)
1-397: Fix Prettier formatting issues before merging.The CI pipeline detected formatting issues in this file. Please run
prettier --write packages/cli/src/ui/hooks/useWelcomeOnboarding.tsto resolve them.packages/cli/src/ui/components/WelcomeOnboarding/AuthenticationStep.tsx (1)
1-174: Fix Prettier formatting issues before merging.The CI pipeline detected formatting issues in this file. Please run
prettier --write packages/cli/src/ui/components/WelcomeOnboarding/AuthenticationStep.tsxto resolve them.packages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsx (1)
1-162: Fix Prettier formatting issues before merging.The CI pipeline detected formatting issues in this file. Please run
prettier --write packages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsxto resolve them.
🤖 Fix all issues with AI Agents
In @packages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsx:
- Around line 83-84: Prettier formatting violations in ModelSelectStep.tsx are
causing CI to fail; run the formatter or fix the spacing around the two <Text>
elements (the empty <Text color={Colors.Foreground}> and the <Text
color={Colors.Foreground}>Select a model for {providerDisplay}:</Text>) to match
project Prettier rules—e.g., run `prettier --write
packages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsx` or apply
the equivalent editor formatting and commit the changes.
🧹 Nitpick comments (3)
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts (3)
110-113: Improve error logging for better debugging.The error object is passed directly to
debug.log(), which may not produce useful output. Consider usingerror instanceof Error ? error.message : String(error)for clearer error messages.🔎 Proposed fix
} catch (error) { // If we can't check for active provider, continue normally - debug.log('Could not check for active provider:', error); + debug.log('Could not check for active provider:', error instanceof Error ? error.message : String(error)); } } // Check if a model has been selected try { const activeModel = runtime.getActiveModelName(); if (activeModel && activeModel !== 'placeholder-model') { debug.log('Welcome suppressed: active model configured'); return true; } } catch (error) { - debug.log('Could not check for active model:', error); + debug.log('Could not check for active model:', error instanceof Error ? error.message : String(error)); }Also applies to: 117-125
128-128: Optimize useMemo dependencies to prevent unnecessary re-computation.The dependency
_settings.mergedis an object reference that may change even whendefaultProfilehasn't changed, causing unnecessary re-computation ofhasUserConfiguration. Consider using_settings.merged.defaultProfileinstead.🔎 Proposed fix
return false; - }, [_settings.merged, runtime]); + }, [_settings.merged.defaultProfile, runtime]);
103-109: Simplify provider check logic.Line 104 checks both
providerNameandhasActiveProvider(). Based on the relevant code snippet showinghasActiveProvider()already validates the active provider, checkingproviderNamefirst appears redundant.🔎 Proposed fix
if (providerManager) { try { - const providerName = providerManager.getActiveProviderName(); const hasActiveProvider = providerManager.hasActiveProvider?.() ?? false; - if (providerName || hasActiveProvider) { + if (hasActiveProvider) { debug.log('Welcome suppressed: active provider configured'); return true; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/AuthenticationStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/ProviderSelectStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/SkipExitStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/WelcomeStep.tsxpackages/cli/src/ui/hooks/useWelcomeOnboarding.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Don't useany- Always specify proper types. Useunknownif the type is truly unknown and add proper type guards.
Do not useconsole.logorconsole.debug- Use the sophisticated logging system instead. Log files are written to ~/.llxprt/debug/
Fix all linting errors, including warnings aboutanytypes
Files:
packages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsxpackages/cli/src/ui/hooks/useWelcomeOnboarding.tspackages/cli/src/ui/components/WelcomeOnboarding/ProviderSelectStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/AuthenticationStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/SkipExitStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/WelcomeStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-17T08:33:42.962Z
Learnt from: MinWooPark-dotcom
Repo: vybestack/llxprt-code PR: 577
File: packages/cli/src/ui/components/messages/ToolMessage.tsx:89-167
Timestamp: 2025-11-17T08:33:42.962Z
Learning: In the vybestack/llxprt-code repository, single-use helper functions in React components can be kept inline rather than extracted to separate utilities, especially when the extraction doesn't provide immediate value (no reuse, no isolated testing planned) and keeps the PR scope focused on feature delivery.
Applied to files:
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts
🧬 Code graph analysis (6)
packages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsx (2)
packages/cli/test-utils/ink-stub.ts (1)
Text(23-23)packages/cli/src/ui/colors.ts (1)
Colors(12-78)
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts (1)
packages/core/src/providers/ProviderManager.ts (1)
hasActiveProvider(851-855)
packages/cli/src/ui/components/WelcomeOnboarding/ProviderSelectStep.tsx (2)
packages/cli/test-utils/ink-stub.ts (1)
Text(23-23)packages/cli/src/ui/colors.ts (1)
Colors(12-78)
packages/cli/src/ui/components/WelcomeOnboarding/AuthenticationStep.tsx (2)
packages/cli/test-utils/ink-stub.ts (2)
Text(23-23)Box(22-22)packages/cli/src/ui/colors.ts (1)
Colors(12-78)
packages/cli/src/ui/components/WelcomeOnboarding/SkipExitStep.tsx (2)
packages/cli/test-utils/ink-stub.ts (2)
Text(23-23)Box(22-22)packages/cli/src/ui/colors.ts (1)
Colors(12-78)
packages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsx (2)
packages/cli/test-utils/ink-stub.ts (1)
Text(23-23)packages/cli/src/ui/colors.ts (1)
Colors(12-78)
🪛 GitHub Actions: LLxprt Code CI
packages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsx
[error] Prettier formatting changes detected. Run 'prettier --write' to fix code style issues in this file.
packages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsx
[error] Prettier formatting changes detected. Run 'prettier --write' to fix code style issues in this file.
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts
[error] Prettier formatting changes detected. Run 'prettier --write' to fix code style issues in this file.
packages/cli/src/ui/components/WelcomeOnboarding/ProviderSelectStep.tsx
[error] Prettier formatting changes detected. Run 'prettier --write' to fix code style issues in this file.
packages/cli/src/ui/components/WelcomeOnboarding/AuthenticationStep.tsx
[error] Prettier formatting changes detected. Run 'prettier --write' to fix code style issues in this file.
packages/cli/src/ui/components/WelcomeOnboarding/SkipExitStep.tsx
[error] Prettier formatting changes detected. Run 'prettier --write' to fix code style issues in this file.
packages/cli/src/ui/components/WelcomeOnboarding/WelcomeStep.tsx
[error] Prettier formatting changes detected. Run 'prettier --write' to fix code style issues in this file.
packages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsx
[error] Prettier formatting changes detected. Run 'prettier --write' to fix code style issues in this file.
⏰ Context from checks skipped due to timeout of 270000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: E2E Test (Linux) - sandbox:docker
- GitHub Check: Slow E2E - Win
- GitHub Check: E2E Test (macOS)
- GitHub Check: E2E Test (Linux) - sandbox:none
🔇 Additional comments (7)
packages/cli/src/ui/components/WelcomeOnboarding/SkipExitStep.tsx (1)
33-50: Fix Prettier formatting to pass CI.The color styling changes properly apply theme foreground colors while preserving inline accent colors. The CI is blocked by Prettier formatting violations.
Run the following to fix formatting:
prettier --write packages/cli/src/ui/components/WelcomeOnboarding/SkipExitStep.tsx⛔ Skipped due to learnings
Learnt from: CR Repo: vybestack/llxprt-code PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-11-25T16:56:18.980Z Learning: Before reporting a task as finished, run `npm run format` from the repository root and ensure it succeeds (exit code 0)Learnt from: CR Repo: vybestack/llxprt-code PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-05T14:59:34.368Z Learning: Always run `npm run format` from the main llxprt-code directory before committing - Never push without formattingpackages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsx (1)
91-92: Fix Prettier formatting to pass CI.The theme color application is correct. However, the CI is blocked by Prettier formatting violations.
Run the following to fix formatting:
prettier --write packages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsx⛔ Skipped due to learnings
Learnt from: CR Repo: vybestack/llxprt-code PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-11-25T16:56:18.980Z Learning: Before reporting a task as finished, run `npm run format` from the repository root and ensure it succeeds (exit code 0)Learnt from: CR Repo: vybestack/llxprt-code PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-05T14:59:34.368Z Learning: Always run `npm run format` from the main llxprt-code directory before committing - Never push without formattingLearnt from: CR Repo: vybestack/llxprt-code PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-05T14:59:34.368Z Learning: After running `npm run format`, always run `git add -A` to stage the formatted changes before committingLearnt from: CR Repo: vybestack/llxprt-code PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-05T14:59:34.368Z Learning: Applies to **/*.{ts,tsx} : Fix all linting errors, including warnings about `any` typesLearnt from: CR Repo: vybestack/llxprt-code PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-05T14:59:34.368Z Learning: Run `npm run typecheck` before push to verify type safetypackages/cli/src/ui/components/WelcomeOnboarding/ProviderSelectStep.tsx (1)
72-73: Fix Prettier formatting to pass CI.The theme color changes correctly address the greenscreen theme issue. The CI pipeline is failing due to Prettier formatting violations.
Run the following to fix formatting:
prettier --write packages/cli/src/ui/components/WelcomeOnboarding/ProviderSelectStep.tsx⛔ Skipped due to learnings
Learnt from: CR Repo: vybestack/llxprt-code PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-11-25T16:56:18.980Z Learning: Before reporting a task as finished, run `npm run format` from the repository root and ensure it succeeds (exit code 0)packages/cli/src/ui/components/WelcomeOnboarding/WelcomeStep.tsx (1)
45-54: Fix Prettier formatting to pass CI.The theme color changes correctly ensure the welcome screen respects the selected theme. However, the CI pipeline is failing due to Prettier formatting violations that must be resolved.
Run the following to fix formatting:
prettier --write packages/cli/src/ui/components/WelcomeOnboarding/WelcomeStep.tsxAlternatively, fix all affected files at once:
prettier --write packages/cli/src/ui/components/WelcomeOnboarding/*.tsx⛔ Skipped due to learnings
Learnt from: CR Repo: vybestack/llxprt-code PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-11-25T16:56:18.980Z Learning: Before reporting a task as finished, run `npm run format` from the repository root and ensure it succeeds (exit code 0)Learnt from: CR Repo: vybestack/llxprt-code PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-05T14:59:34.368Z Learning: Always run `npm run format` from the main llxprt-code directory before committing - Never push without formattingLearnt from: CR Repo: vybestack/llxprt-code PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-05T14:59:34.368Z Learning: After running `npm run format`, always run `git add -A` to stage the formatted changes before committingLearnt from: CR Repo: vybestack/llxprt-code PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-05T14:59:34.368Z Learning: Applies to **/*.{ts,tsx} : Fix all linting errors, including warnings about `any` typesLearnt from: CR Repo: vybestack/llxprt-code PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-11-25T16:56:18.980Z Learning: Before reporting a task as finished, run `npm run typecheck` from the repository root and ensure it succeeds (exit code 0)Learnt from: CR Repo: vybestack/llxprt-code PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-05T14:59:34.368Z Learning: Run `npm run typecheck` to ensure type safetypackages/cli/src/ui/hooks/useWelcomeOnboarding.ts (1)
131-134: LGTM!The logic correctly suppresses the welcome screen when user configuration is detected, aligning with the PR objectives to prevent unwanted display when profiles/models are already configured.
packages/cli/src/ui/components/WelcomeOnboarding/AuthenticationStep.tsx (1)
122-122: LGTM! Theme violation fixed.The addition of
color={Colors.Foreground}to text elements correctly addresses the theme violation issue mentioned in the PR objectives, ensuring text respects the selected theme (e.g., "greenscreen") instead of defaulting to white.Also applies to: 126-127, 152-152, 158-158, 162-163
packages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsx (1)
97-97: LGTM! Theme violation fixed with consistent styling.The addition of
color={Colors.Foreground}across all informational text elements correctly addresses the theme violation. The change from "✓" to "[OK]" on Line 99 is also sensible for better cross-platform terminal compatibility and theme consistency.Also applies to: 99-99, 104-106, 112-112, 130-130, 133-134, 149-149
packages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsx
Outdated
Show resolved
Hide resolved
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-24.x-ubuntu-latest' artifact from the main CI run. |
|
@CodeRabbit, will this suppress it all the time? what if the user does llxprt and it falls back to gemini + gemini-2.5-pro or whatever? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 12100 @acoliver, that's an excellent catch! You're absolutely right to be concerned. Looking at the current implementation, the const providerName = providerManager.getActiveProviderName();
const hasActiveProvider = providerManager.hasActiveProvider?.() ?? false;
if (providerName || hasActiveProvider) {
debug.log('Welcome suppressed: active provider configured');
return true;
}And for models: const activeModel = runtime.getActiveModelName();
if (activeModel && activeModel !== 'placeholder-model') {
debug.log('Welcome suppressed: active model configured');
return true;
}The problem: This doesn't distinguish between:
So yes, if the system falls back to gemini + gemini-2.5-pro on a fresh install, this logic would suppress the welcome screen for a first-time user who hasn't configured anything yet. Possible solutions:
What's the intended behavior here? Should the welcome screen:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @packages/cli/src/ui/hooks/useWelcomeOnboarding.ts:
- Around line 84-129: The welcome suppression logic treats system defaults as
user configuration; move the isWelcomeCompleted() check to be the primary gate
in the useMemo so explicit completion always shows onboarding, and only then
consider supplemental non-interactive signals (_settings.merged.defaultProfile,
process.env.LLXPRT_PROFILE, runtime.getCliProviderManager() /
providerManager.getActiveProviderName() / providerManager.hasActiveProvider,
runtime.getActiveModelName()) to suppress the welcome for CLI/env-driven runs;
also ensure runtime.getActiveModelName() ignores placeholder or known system
default model names when deciding to suppress.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/AuthenticationStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/ProviderSelectStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/SkipExitStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/WelcomeStep.tsxpackages/cli/src/ui/hooks/useWelcomeOnboarding.ts
✅ Files skipped from review due to trivial changes (1)
- packages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/cli/src/ui/components/WelcomeOnboarding/AuthenticationStep.tsx
- packages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsx
- packages/cli/src/ui/components/WelcomeOnboarding/WelcomeStep.tsx
- packages/cli/src/ui/components/WelcomeOnboarding/ProviderSelectStep.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Don't useany- Always specify proper types. Useunknownif the type is truly unknown and add proper type guards.
Do not useconsole.logorconsole.debug- Use the sophisticated logging system instead. Log files are written to ~/.llxprt/debug/
Fix all linting errors, including warnings aboutanytypes
Files:
packages/cli/src/ui/hooks/useWelcomeOnboarding.tspackages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/SkipExitStep.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-17T08:33:42.962Z
Learnt from: MinWooPark-dotcom
Repo: vybestack/llxprt-code PR: 577
File: packages/cli/src/ui/components/messages/ToolMessage.tsx:89-167
Timestamp: 2025-11-17T08:33:42.962Z
Learning: In the vybestack/llxprt-code repository, single-use helper functions in React components can be kept inline rather than extracted to separate utilities, especially when the extraction doesn't provide immediate value (no reuse, no isolated testing planned) and keeps the PR scope focused on feature delivery.
Applied to files:
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts
🧬 Code graph analysis (2)
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts (1)
packages/core/src/providers/ProviderManager.ts (1)
hasActiveProvider(851-855)
packages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsx (2)
packages/cli/test-utils/ink-stub.ts (1)
Text(23-23)packages/cli/src/ui/colors.ts (1)
Colors(12-78)
⏰ Context from checks skipped due to timeout of 270000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Test (windows-latest, 24.x)
- GitHub Check: Test (ubuntu-latest, 24.x)
- GitHub Check: Test (macos-latest, 24.x)
- GitHub Check: Slow E2E - Win
- GitHub Check: E2E Test (macOS)
- GitHub Check: E2E Test (Linux) - sandbox:none
- GitHub Check: E2E Test (Linux) - sandbox:docker
🔇 Additional comments (4)
packages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsx (1)
91-93: LGTM!The explicit
color={Colors.Foreground}correctly applies the theme-aware foreground color, fixing the white-text issue under themes like "greenscreen". This is consistent with how other text elements in this component use themed colors.packages/cli/src/ui/hooks/useWelcomeOnboarding.ts (2)
131-133: Logic structure looks correct.The updated
showWelcomecondition properly gates the welcome screen on folder trust completion and configuration status. The main concern about distinguishing explicit vs. fallback configuration is noted above.
111-114: Good defensive error handling.The try/catch blocks around provider and model lookups ensure the onboarding flow continues gracefully if these checks fail, rather than crashing. Errors are correctly logged via
DebugLoggerper project guidelines.packages/cli/src/ui/components/WelcomeOnboarding/SkipExitStep.tsx (1)
33-50: LGTM!The
color={Colors.Foreground}additions consistently apply theme-aware text colors to all instructional text. The nestedAccentCyanhighlights for command references provide good visual hierarchy while maintaining theme compliance.
Fixes #1005 - Add welcome suppression logic to useWelcomeOnboarding hook when user has configured profiles/models via defaultProfile, LLXPRT_PROFILE env var, or active provider/runtime - Fix theme color application in all WelcomeOnboarding components by explicitly setting color={Colors.Foreground} on all text elements that were using default white - Components fixed: WelcomeStep, ProviderSelectStep, ModelSelectStep, AuthMethodStep, AuthenticationStep, CompletionStep, SkipExitStep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @packages/cli/src/ui/hooks/useWelcomeOnboarding.ts:
- Around line 86-88: The arrow function is using a block body which triggers the
ESLint arrow-body-style rule; change the isSystemDefaultModelName declaration to
use a concise arrow body that directly returns the boolean expression (e.g.,
replace the block with an expression using
PLACEHOLDER_MODEL_NAMES.has(modelName)) so the function becomes a
single-expression arrow function.
🧹 Nitpick comments (1)
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts (1)
18-19: Consider expandingPLACEHOLDER_MODEL_NAMESto include known system default models.The
PLACEHOLDER_MODEL_NAMESset currently only contains'placeholder-model'. If the system falls back to a default model likegemini-2.5-prowhen no explicit configuration exists, this check won't prevent suppression for first-run users.🔎 Suggested approach
Either expand the placeholder set to include known fallback defaults, or add a more robust check that determines whether the model was explicitly configured vs. auto-selected:
-const PLACEHOLDER_MODEL_NAMES = new Set(['placeholder-model']); +const PLACEHOLDER_MODEL_NAMES = new Set([ + 'placeholder-model', + // Add other known system default models if applicable +]);Alternatively, consider checking for an explicit configuration source flag if available in the runtime.
Also applies to: 122-130
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/AuthenticationStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/ProviderSelectStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/SkipExitStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/WelcomeStep.tsxpackages/cli/src/ui/hooks/useWelcomeOnboarding.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/cli/src/ui/components/WelcomeOnboarding/AuthMethodStep.tsx
- packages/cli/src/ui/components/WelcomeOnboarding/AuthenticationStep.tsx
- packages/cli/src/ui/components/WelcomeOnboarding/ProviderSelectStep.tsx
- packages/cli/src/ui/components/WelcomeOnboarding/WelcomeStep.tsx
- packages/cli/src/ui/components/WelcomeOnboarding/SkipExitStep.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Don't useany- Always specify proper types. Useunknownif the type is truly unknown and add proper type guards.
Do not useconsole.logorconsole.debug- Use the sophisticated logging system instead. Log files are written to ~/.llxprt/debug/
Fix all linting errors, including warnings aboutanytypes
Files:
packages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsxpackages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsxpackages/cli/src/ui/hooks/useWelcomeOnboarding.ts
🧠 Learnings (3)
📚 Learning: 2025-11-25T16:56:18.980Z
Learnt from: CR
Repo: vybestack/llxprt-code PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:56:18.980Z
Learning: Before reporting a task as finished, run `npm run format` from the repository root and ensure it succeeds (exit code 0)
Applied to files:
packages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsx
📚 Learning: 2026-01-06T03:14:42.588Z
Learnt from: acoliver
Repo: vybestack/llxprt-code PR: 0
File: :0-0
Timestamp: 2026-01-06T03:14:42.588Z
Learning: Auto-switching the active provider after OAuth authentication can conflict with multi-provider workflows: pre-authenticating backup providers for bucket failover, setting up load balancer profiles, authenticating GoogleWebSearch, and authenticating subagents. Users need explicit control in these scenarios.
Applied to files:
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts
📚 Learning: 2025-11-17T08:33:42.962Z
Learnt from: MinWooPark-dotcom
Repo: vybestack/llxprt-code PR: 577
File: packages/cli/src/ui/components/messages/ToolMessage.tsx:89-167
Timestamp: 2025-11-17T08:33:42.962Z
Learning: In the vybestack/llxprt-code repository, single-use helper functions in React components can be kept inline rather than extracted to separate utilities, especially when the extraction doesn't provide immediate value (no reuse, no isolated testing planned) and keeps the PR scope focused on feature delivery.
Applied to files:
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts
🧬 Code graph analysis (1)
packages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsx (2)
packages/cli/test-utils/ink-stub.ts (2)
Text(23-23)Box(22-22)packages/cli/src/ui/colors.ts (1)
Colors(12-78)
🪛 GitHub Actions: LLxprt Code CI
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts
[error] 86-86: ESLint: Unexpected block statement surrounding arrow body; move the returned value immediately after the '=>' (arrow-body-style).
🪛 GitHub Check: Lint (Javascript)
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts
[failure] 86-86:
Unexpected block statement surrounding arrow body; move the returned value immediately after the =>
⏰ Context from checks skipped due to timeout of 270000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Slow E2E - Win
- GitHub Check: E2E Test (Linux) - sandbox:none
- GitHub Check: E2E Test (Linux) - sandbox:docker
🔇 Additional comments (4)
packages/cli/src/ui/components/WelcomeOnboarding/ModelSelectStep.tsx (1)
83-86: LGTM!The theme fix correctly applies
Colors.Foregroundto ensure the text respects the active theme (e.g., "greenscreen"). The leading space inside the Text element is a clean approach to maintain visual spacing after removing the empty spacer element.packages/cli/src/ui/components/WelcomeOnboarding/CompletionStep.tsx (2)
97-99: LGTM!The change from "✓" to "[OK]" improves terminal compatibility across different environments while maintaining the semantic meaning. The
Colors.AccentGreenstyling is appropriate for a success message.
103-105: Theme-aware styling applied consistently.All text elements now properly use
Colors.Foregroundto respect the active theme, fixing the reported issue where text displayed as white under the "greenscreen" theme.Also applies to: 111-113, 131-131, 134-135, 150-150
packages/cli/src/ui/hooks/useWelcomeOnboarding.ts (1)
90-147: Memoized suppression logic structure is sound.The refactored logic correctly prioritizes
welcomeCompleted(handled inshowWelcome) and adds memoization for performance. The defensive try-catch blocks around provider/model checks prevent errors from breaking the flow. The explicit comment at lines 132-135 documents the intentional decision not to suppress based ondefaultProfilealone.
| const isSystemDefaultModelName = useCallback((modelName: string): boolean => { | ||
| return PLACEHOLDER_MODEL_NAMES.has(modelName); | ||
| }, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix ESLint arrow-body-style violation (CI failure).
The linter reports: "Unexpected block statement surrounding arrow body; move the returned value immediately after the =>".
🔎 Proposed fix
- const isSystemDefaultModelName = useCallback((modelName: string): boolean => {
- return PLACEHOLDER_MODEL_NAMES.has(modelName);
- }, []);
+ const isSystemDefaultModelName = useCallback(
+ (modelName: string): boolean => PLACEHOLDER_MODEL_NAMES.has(modelName),
+ [],
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isSystemDefaultModelName = useCallback((modelName: string): boolean => { | |
| return PLACEHOLDER_MODEL_NAMES.has(modelName); | |
| }, []); | |
| const isSystemDefaultModelName = useCallback( | |
| (modelName: string): boolean => PLACEHOLDER_MODEL_NAMES.has(modelName), | |
| [], | |
| ); |
🧰 Tools
🪛 GitHub Actions: LLxprt Code CI
[error] 86-86: ESLint: Unexpected block statement surrounding arrow body; move the returned value immediately after the '=>' (arrow-body-style).
🪛 GitHub Check: Lint (Javascript)
[failure] 86-86:
Unexpected block statement surrounding arrow body; move the returned value immediately after the =>
🤖 Prompt for AI Agents
In @packages/cli/src/ui/hooks/useWelcomeOnboarding.ts around lines 86 - 88, The
arrow function is using a block body which triggers the ESLint arrow-body-style
rule; change the isSystemDefaultModelName declaration to use a concise arrow
body that directly returns the boolean expression (e.g., replace the block with
an expression using PLACEHOLDER_MODEL_NAMES.has(modelName)) so the function
becomes a single-expression arrow function.
Summary
Fixes #1005
This PR addresses two issues with the new welcome screen:
1. Theme Violation
When the "greenscreen" theme is selected, text elements were displaying in white instead of respecting the theme colors. This has been fixed by explicitly setting
color={Colors.Foreground}on all text elements in the welcome screen components.Fixed components:
2. Unwanted Display
The welcome screen was still appearing even when users had configured profiles/models via various methods. The welcome screen now properly suppresses display when:
settings.merged.defaultProfile)LLXPRT_PROFILEenvironment variable is setChanges
packages/cli/src/ui/hooks/useWelcomeOnboarding.tsto add suppression checksTesting
Summary by CodeRabbit
Style
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.