fix(ui): state-aware bootstrap buttons with user feedback (#353)#426
Conversation
…ai#353) When local AI state is "ready", replace the Bootstrap button with a "Running" badge so clicking it no longer appears to do nothing. Show "Retry" label when state is degraded. Add transient success/error messages after manual bootstrap/re-bootstrap actions so the user always gets clear feedback. Closes tinyhumansai#353 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdded ephemeral bootstrap feedback and adjusted bootstrap control behavior: UI clears/sets a transient Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/pages/Home.tsx (1)
357-361: Consider: Failure message uses success color.The
bootstrapMessageis always rendered withtext-green-600, but it can contain failure text like "Bootstrap failed — check warning below" or "Bootstrap failed". Consider using conditional styling:♻️ Optional: Use error color for failure messages
{bootstrapMessage && ( - <span className="text-[11px] text-green-600 animate-fade-up"> + <span className={`text-[11px] animate-fade-up ${ + bootstrapMessage.toLowerCase().includes('failed') + ? 'text-red-600' + : 'text-green-600' + }`}> {bootstrapMessage} </span> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/Home.tsx` around lines 357 - 361, The bootstrapMessage is always styled with text-green-600 even when it reports failures; update the rendering logic for bootstrapMessage (the JSX that references bootstrapMessage) to apply conditional styling based on status (e.g., a boolean like bootstrapError / bootstrapFailed or by checking the message content for "failed") so success messages use text-green-600 and failure messages use an error color such as text-red-600; locate the span that renders bootstrapMessage and switch its className to choose the color dynamically according to the status variable or message check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/settings/panels/LocalModelPanel.tsx`:
- Around line 207-212: The timeout clearing logic can update state after the
component unmounts; modify the code around
openhumanLocalAiStatus()/setBootstrapMessage() to store the timeout ID (from
setTimeout) in a ref or variable and clear it in a cleanup (e.g., useEffect
return) so setBootstrapMessage('') is not called on an unmounted component;
alternatively track a mounted ref (isMounted) set to false on cleanup and guard
the timeout callback to only call setBootstrapMessage when mounted. Ensure you
reference the existing openhumanLocalAiStatus call and the setBootstrapMessage
setter when adding the timeout ID/ref and cleanup.
In `@app/src/pages/Home.tsx`:
- Around line 54-64: The setTimeout calls after calling refreshLocalAiStatus and
setting setBootstrapMessage (in the success branch where freshStatus?.state ===
'ready' / 'degraded' and in the catch block) can fire after the Home component
unmounts; store the timeout ID (e.g., in a ref like bootstrapTimeoutRef) when
calling setTimeout in both places and clearTimeout(bootstrapTimeoutRef.current)
in a useEffect cleanup (or when scheduling a new timeout) to avoid state updates
on unmounted components and to mirror the fix used in LocalModelPanel.tsx.
---
Nitpick comments:
In `@app/src/pages/Home.tsx`:
- Around line 357-361: The bootstrapMessage is always styled with text-green-600
even when it reports failures; update the rendering logic for bootstrapMessage
(the JSX that references bootstrapMessage) to apply conditional styling based on
status (e.g., a boolean like bootstrapError / bootstrapFailed or by checking the
message content for "failed") so success messages use text-green-600 and failure
messages use an error color such as text-red-600; locate the span that renders
bootstrapMessage and switch its className to choose the color dynamically
according to the status variable or message check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 053ee29f-d439-47e2-a1f9-04d090242ad7
📒 Files selected for processing (2)
app/src/components/settings/panels/LocalModelPanel.tsxapp/src/pages/Home.tsx
| const freshStatus = await openhumanLocalAiStatus(); | ||
| setStatus(freshStatus.result); | ||
| if (freshStatus.result?.state === 'ready') { | ||
| setBootstrapMessage(force ? 'Re-bootstrap complete' : 'Models verified'); | ||
| } | ||
| setTimeout(() => setBootstrapMessage(''), 3000); |
There was a problem hiding this comment.
Minor: setTimeout may fire after unmount.
If the component unmounts within 3 seconds of a successful bootstrap, the setBootstrapMessage('') call will attempt to update unmounted state. Consider using a ref to track mounted state or storing the timeout ID for cleanup.
🛡️ Optional fix using a ref
+ const mountedRef = useRef(true);
+
+ useEffect(() => {
+ return () => {
+ mountedRef.current = false;
+ };
+ }, []);
+
const triggerDownload = async (force: boolean) => {
setIsTriggeringDownload(true);
setStatusError('');
setBootstrapMessage('');
try {
await openhumanLocalAiDownload(force);
await openhumanLocalAiDownloadAllAssets(force);
const freshStatus = await openhumanLocalAiStatus();
setStatus(freshStatus.result);
if (freshStatus.result?.state === 'ready') {
setBootstrapMessage(force ? 'Re-bootstrap complete' : 'Models verified');
}
- setTimeout(() => setBootstrapMessage(''), 3000);
+ setTimeout(() => {
+ if (mountedRef.current) setBootstrapMessage('');
+ }, 3000);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/settings/panels/LocalModelPanel.tsx` around lines 207 -
212, The timeout clearing logic can update state after the component unmounts;
modify the code around openhumanLocalAiStatus()/setBootstrapMessage() to store
the timeout ID (from setTimeout) in a ref or variable and clear it in a cleanup
(e.g., useEffect return) so setBootstrapMessage('') is not called on an
unmounted component; alternatively track a mounted ref (isMounted) set to false
on cleanup and guard the timeout callback to only call setBootstrapMessage when
mounted. Ensure you reference the existing openhumanLocalAiStatus call and the
setBootstrapMessage setter when adding the timeout ID/ref and cleanup.
| const freshStatus = await refreshLocalAiStatus(); | ||
| if (freshStatus?.state === 'ready') { | ||
| setBootstrapMessage(force ? 'Re-bootstrap complete' : 'Local AI is ready'); | ||
| } else if (freshStatus?.state === 'degraded') { | ||
| setBootstrapMessage('Bootstrap failed — check warning below'); | ||
| } | ||
| setTimeout(() => setBootstrapMessage(''), 3000); | ||
| } catch (error) { | ||
| console.warn('[Home] manual Local AI bootstrap failed:', error); | ||
| setBootstrapMessage('Bootstrap failed'); | ||
| setTimeout(() => setBootstrapMessage(''), 3000); |
There was a problem hiding this comment.
Minor: setTimeout calls lack cleanup on unmount.
Both success (line 60) and error (line 64) paths schedule setTimeout without cleanup. If the user navigates away from Home within 3 seconds, React will warn about state updates on unmounted components.
The same fix pattern suggested for LocalModelPanel.tsx applies here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/Home.tsx` around lines 54 - 64, The setTimeout calls after
calling refreshLocalAiStatus and setting setBootstrapMessage (in the success
branch where freshStatus?.state === 'ready' / 'degraded' and in the catch block)
can fire after the Home component unmounts; store the timeout ID (e.g., in a ref
like bootstrapTimeoutRef) when calling setTimeout in both places and
clearTimeout(bootstrapTimeoutRef.current) in a useEffect cleanup (or when
scheduling a new timeout) to avoid state updates on unmounted components and to
mirror the fix used in LocalModelPanel.tsx.
…sai#353) Cover the four key rendering states of the Home local-AI card: - "Running" badge when state is ready (Bootstrap button hidden) - "Retry" label when state is degraded - "Bootstrap" label when state is idle - Transient "Re-bootstrap complete" message after successful re-bootstrap Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/pages/__tests__/HomeBootstrapButtons.test.tsx (2)
115-151: Add coverage for busy/failure/transient bootstrap feedback paths.The suite currently validates success and base state rendering, but the PR behavior also includes
Working.../disabled controls, failure messaging, and 3s auto-dismiss. Add tests for those to prevent UX regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/__tests__/HomeBootstrapButtons.test.tsx` around lines 115 - 151, Add tests to cover the busy, failure, and transient feedback flows: create new cases in HomeBootstrapButtons.test.tsx that (1) mock tauriCommands.openhumanLocalAiStatus to return a non-ready state (e.g., 'starting') and assert the UI shows "Working..." and the Re-bootstrap button is disabled; (2) mock bootstrapLocalAiWithRecommendedPreset to reject (throw) and assert the failure message is shown after clicking the Re-bootstrap button; and (3) verify transient auto-dismiss by using vi.useFakeTimers()/vi.advanceTimersByTime(3000) to assert the success/failure toast is removed after ~3s; use the same mocked helpers (tauriCommands.openhumanLocalAiStatus, bootstrapUtils.ensureRecommendedLocalAiPresetIfNeeded, bootstrapUtils.bootstrapLocalAiWithRecommendedPreset), fireEvent.click, and waitFor to drive and assert each path.
34-43: Replaceas neverwith properly typed fixtures usingsatisfies.Using
as neverbypasses type-checking on mock payloads and hides contract drift. The return types foropenhumanLocalAiStatusandensureRecommendedLocalAiPresetIfNeededare exported and accessible. Rewrite mocks withsatisfies CommandResponse<LocalAiStatus>andsatisfies LocalAiPresetResolutionto catch schema mismatches when APIs change.Alternatively, if test payloads must remain partial, document why with a comment and use
as unknownto signal intentional incompleteness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/__tests__/HomeBootstrapButtons.test.tsx` around lines 34 - 43, Replace the unsafe "as never" casts in the test mocks with proper typed fixtures: change the mock return for openhumanLocalAiStatus to use a value that satisfies CommandResponse<LocalAiStatus> and change the mockResolvedValue for bootstrapUtils.ensureRecommendedLocalAiPresetIfNeeded to satisfy LocalAiPresetResolution so TypeScript will validate schema changes; if you intentionally provide only partial payloads, add a short comment explaining why and cast to "as unknown" instead of "as never". Ensure you update the mocks for the symbols openhumanLocalAiStatus and bootstrapUtils.ensureRecommendedLocalAiPresetIfNeeded accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/pages/__tests__/HomeBootstrapButtons.test.tsx`:
- Around line 115-151: Add tests to cover the busy, failure, and transient
feedback flows: create new cases in HomeBootstrapButtons.test.tsx that (1) mock
tauriCommands.openhumanLocalAiStatus to return a non-ready state (e.g.,
'starting') and assert the UI shows "Working..." and the Re-bootstrap button is
disabled; (2) mock bootstrapLocalAiWithRecommendedPreset to reject (throw) and
assert the failure message is shown after clicking the Re-bootstrap button; and
(3) verify transient auto-dismiss by using
vi.useFakeTimers()/vi.advanceTimersByTime(3000) to assert the success/failure
toast is removed after ~3s; use the same mocked helpers
(tauriCommands.openhumanLocalAiStatus,
bootstrapUtils.ensureRecommendedLocalAiPresetIfNeeded,
bootstrapUtils.bootstrapLocalAiWithRecommendedPreset), fireEvent.click, and
waitFor to drive and assert each path.
- Around line 34-43: Replace the unsafe "as never" casts in the test mocks with
proper typed fixtures: change the mock return for openhumanLocalAiStatus to use
a value that satisfies CommandResponse<LocalAiStatus> and change the
mockResolvedValue for bootstrapUtils.ensureRecommendedLocalAiPresetIfNeeded to
satisfy LocalAiPresetResolution so TypeScript will validate schema changes; if
you intentionally provide only partial payloads, add a short comment explaining
why and cast to "as unknown" instead of "as never". Ensure you update the mocks
for the symbols openhumanLocalAiStatus and
bootstrapUtils.ensureRecommendedLocalAiPresetIfNeeded accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a81610c-464b-459c-b6c0-0d30195885bd
📒 Files selected for processing (1)
app/src/pages/__tests__/HomeBootstrapButtons.test.tsx
…ai#353) (tinyhumansai#426) * fix(ui): state-aware bootstrap buttons with user feedback (tinyhumansai#353) When local AI state is "ready", replace the Bootstrap button with a "Running" badge so clicking it no longer appears to do nothing. Show "Retry" label when state is degraded. Add transient success/error messages after manual bootstrap/re-bootstrap actions so the user always gets clear feedback. Closes tinyhumansai#353 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(ui): add unit tests for state-aware bootstrap buttons (tinyhumansai#353) Cover the four key rendering states of the Home local-AI card: - "Running" badge when state is ready (Bootstrap button hidden) - "Retry" label when state is degraded - "Bootstrap" label when state is idle - Transient "Re-bootstrap complete" message after successful re-bootstrap Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
ready, so clicking it no longer appears non-functional.degradedto indicate actionable recovery.Problem
When the local model runtime shows Ready/100%, the Bootstrap and Re-bootstrap buttons are always enabled but appear to do nothing when clicked. The underlying
download_all_modelsfires in the background and completes near-instantly (models already exist), so the 2-second polling misses the state flicker and the user sees zero feedback. This makes the UI feel broken. Reported on macOS Intel.Closes #353
Solution
State-aware button rendering in both
Home.tsxandLocalModelPanel.tsx:readystate → Bootstrap button replaced with a green "Running" badge; only Re-bootstrap remains clickabledegradedstate → Bootstrap label changes to "Retry" / "Retry Bootstrap"downloading/installing/loading→ both buttons show "Working..." and are disabledCompletion feedback: After
runManualBootstrap/triggerDownloadresolves, a transient message (3s auto-dismiss) shows "Local AI is ready", "Re-bootstrap complete", or "Bootstrap failed" so the user always knows what happened.Submission Checklist
Impact
isTauri()is true.Related
Summary by CodeRabbit
New Features
Tests