refactor(settings): restructure settings page for better UX#494
Merged
graycyrus merged 3 commits intotinyhumansai:mainfrom Apr 10, 2026
Merged
refactor(settings): restructure settings page for better UX#494graycyrus merged 3 commits intotinyhumansai:mainfrom
graycyrus merged 3 commits intotinyhumansai:mainfrom
Conversation
Reorganize settings into 4 clean sections (Account & Billing, Features, AI & Models, Developer Options) and extract developer-oriented options from user-facing panels into dedicated debug panels. - Merge Account & Security + Billing into Account & Billing section - Rename Automation & Channels to Features; add Tools, Voice here - Simplify AI & Skills to AI & Models (just Local AI Model) - Expand Developer Options from 4 to 10 items - Create 4 debug panels: ScreenAwarenessDebug, AutocompleteDebug, VoiceDebug, LocalModelDebug - Simplify 4 user panels by stripping dev knobs (FPS, debounce timers, test harnesses, diagnostics, etc.) - Delete AccessibilityPanel (merged into Screen Awareness) - Add "Advanced settings" link in each simplified panel - Update navigation breadcrumbs for new hierarchy
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughReorganized settings: renamed menu sections (Account & Security → Account & Billing, Automation & Channels → Features, AI & Skills → AI & Models), removed Accessibility, split advanced/debug flows into new dedicated debug panels, added multiple debug routes, and updated routing, breadcrumbs, and menu items. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as AutocompleteDebugPanel (UI)
participant Tauri as Tauri Command Bridge
participant Runtime as Autocomplete Runtime
User->>UI: Click "Refresh" / Start / Test Suggestion
UI->>Tauri: openhumanAutocompleteStatus / Start / Current / Accept / ...
Tauri->>Runtime: invoke runtime command
Runtime-->>Tauri: status / result / error
Tauri-->>UI: return data or error
UI-->>User: update status, logs, history, notices
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/components/settings/panels/ScreenIntelligencePanel.tsx (1)
163-183:⚠️ Potential issue | 🟠 MajorPersist or relabel the
Screen Monitoringtoggle.This checkbox sits in the saved settings form, but
saveConfig()never writes it anywhere. Right now it only affects the nextstartSession()call viafeatureOverrides, then falls back tostatus?.features.screen_monitoringon remount. That makes it look like a persistent setting when it is actually a transient session override.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/settings/panels/ScreenIntelligencePanel.tsx` around lines 163 - 183, The "Screen Monitoring" checkbox (screenMonitoring) is being stored only in the transient featureOverrides via setFeatureOverrides and never persisted by saveConfig, causing UI to misrepresent it as a saved setting; update saveConfig to persist the screen_monitoring value (alongside other saved settings) so the saved configuration reflects status?.features.screen_monitoring on remount, or alternatively remove/relabel the checkbox as a session-only toggle and ensure startSession reads from featureOverrides; locate the checkbox handling (screenMonitoring, setFeatureOverrides), the saveConfig function, and the component's use of status?.features.screen_monitoring/startSession and either: (A) add screen_monitoring to the persisted payload in saveConfig or (B) change the UI label and flow to make it clear it's a session-only override.
🤖 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/AutocompleteDebugPanel.tsx`:
- Around line 372-377: Guard against a null response.result from
openhumanAutocompleteDebugFocus(): after awaiting response, still call
appendLogs(response.logs) but check if response.result is null/undefined before
accessing fields; if null, setFocusDebug to a safe message (e.g., 'null' or
'{}') and call appendUiLog with fallback values (use optional chaining and
default values for app_name, role, and context length such as 0) or skip the
detailed UI log entirely. Update the block that currently calls
setFocusDebug(JSON.stringify(response.result, ...)) and appendUiLog(...) so it
only reads response.result.app_name, response.result.role, and
response.result.context.length when response.result exists, and otherwise uses
safe defaults.
In `@app/src/components/settings/panels/AutocompletePanel.tsx`:
- Around line 120-149: The saveConfig function currently reads pass-through
fields from fullConfigRef.current which may still be DEFAULT_CONFIG if load()
hasn't completed; update saveConfig (and the similar block around lines 156-165)
to block until the real config is loaded by checking a loaded flag or awaiting
the load() promise before proceeding: e.g., ensure fullConfigRef has been
populated (not DEFAULT_CONFIG) or await a loadConfig()/ensureConfigLoaded()
helper, and only then read debounce_ms, max_chars, style_examples,
overlay_ttl_ms, etc., so you preserve the actual advanced settings instead of
writing defaults back; reference saveConfig and fullConfigRef to locate where to
add the guard/await.
In `@app/src/components/settings/panels/LocalModelDebugPanel.tsx`:
- Around line 135-161: The polling function loadStatus should not clear errors
originating from user actions; stop calling setStatusError('') inside
loadStatus's successful path and instead create a separate state (e.g.,
pollStatusError via useState) to hold polling-specific failures from
openhumanLocalAiStatus/openhumanLocalAiAssetsStatus/openhumanLocalAiDownloadsProgress,
set that on catch in loadStatus, and leave the existing statusError (action
errors) untouched; update any UI reads to show both sources (action errors from
statusError and polling errors from pollStatusError) so action errors don't
disappear on the next successful poll.
In `@app/src/components/settings/panels/ScreenAwarenessDebugPanel.tsx`:
- Around line 56-70: The effect syncs multiple local state setters
(lastSyncedConfigSigRef, setBaselineFps, setUseVisionModel, setKeepScreenshots,
setAllowlistText, setDenylistText) from status.config, which violates the
project's set-state-in-effect rule; remove this useEffect and instead derive the
form values from status.config (e.g. compute a single derivedForm object with
baseline_fps, use_vision_model, keep_screenshots, allowlist, denylist) using
useMemo(() => ({...}), [status?.config]) and feed those derived values to the
inputs (or initialize a single formState via a lazy useState initializer that
reads status.config once), so you no longer call multiple synchronous setState
inside useEffect; use the derivedForm values where the component currently reads
the individual state setters.
In `@app/src/components/settings/panels/VoiceDebugPanel.tsx`:
- Around line 44-49: The Promise.all fan-out includes an unused RPC
openhumanLocalAiAssetsStatus() which can cause loadData() to reject; remove
openhumanLocalAiAssetsStatus() from the Promise.all call so that
settingsResponse, serverResponse, and voiceResponse are awaited only, and if you
still need to call openhumanLocalAiAssetsStatus() do it outside loadData() with
its own try/catch (or ignore errors) to avoid failing the VoiceDebugPanel's load
path; update the Promise.all invocation and related variable handling
accordingly (look for settingsResponse, serverResponse, voiceResponse,
Promise.all, and openhumanLocalAiAssetsStatus).
---
Outside diff comments:
In `@app/src/components/settings/panels/ScreenIntelligencePanel.tsx`:
- Around line 163-183: The "Screen Monitoring" checkbox (screenMonitoring) is
being stored only in the transient featureOverrides via setFeatureOverrides and
never persisted by saveConfig, causing UI to misrepresent it as a saved setting;
update saveConfig to persist the screen_monitoring value (alongside other saved
settings) so the saved configuration reflects status?.features.screen_monitoring
on remount, or alternatively remove/relabel the checkbox as a session-only
toggle and ensure startSession reads from featureOverrides; locate the checkbox
handling (screenMonitoring, setFeatureOverrides), the saveConfig function, and
the component's use of status?.features.screen_monitoring/startSession and
either: (A) add screen_monitoring to the persisted payload in saveConfig or (B)
change the UI label and flow to make it clear it's a session-only override.
🪄 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: 0fc87b17-b7e3-406b-b823-751213d79289
📒 Files selected for processing (14)
app/src/components/settings/SettingsHome.tsxapp/src/components/settings/hooks/useSettingsNavigation.tsapp/src/components/settings/panels/AccessibilityPanel.tsxapp/src/components/settings/panels/AutocompleteDebugPanel.tsxapp/src/components/settings/panels/AutocompletePanel.tsxapp/src/components/settings/panels/DeveloperOptionsPanel.tsxapp/src/components/settings/panels/LocalModelDebugPanel.tsxapp/src/components/settings/panels/LocalModelPanel.tsxapp/src/components/settings/panels/ScreenAwarenessDebugPanel.tsxapp/src/components/settings/panels/ScreenIntelligencePanel.tsxapp/src/components/settings/panels/VoiceDebugPanel.tsxapp/src/components/settings/panels/VoicePanel.tsxapp/src/components/settings/panels/__tests__/AccessibilityPanel.test.tsxapp/src/pages/Settings.tsx
💤 Files with no reviewable changes (2)
- app/src/components/settings/panels/tests/AccessibilityPanel.test.tsx
- app/src/components/settings/panels/AccessibilityPanel.tsx
- Guard null result from openhumanAutocompleteDebugFocus() - Block save/start until full config loaded in AutocompletePanel - Separate poll errors from action errors in LocalModelDebugPanel - Replace useEffect setState with render-time one-shot init in ScreenAwarenessDebugPanel to avoid set-state-in-effect lint rule - Remove unused openhumanLocalAiAssetsStatus call from VoiceDebugPanel
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
app/src/components/settings/panels/LocalModelDebugPanel.tsx (2)
135-148:⚠️ Potential issue | 🟡 MinorExpose poll failures instead of swallowing them.
This keeps the last snapshot, but it also makes the panel look healthy when polling is broken. After the first successful refresh, stale status/progress can sit indefinitely with no indication that the data is no longer updating. Please keep a dedicated poll error state and surface it alongside
statusErrorinstead of ignoring the failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/settings/panels/LocalModelDebugPanel.tsx` around lines 135 - 148, The poll currently swallows errors in loadStatus, leaving stale UI state; add a dedicated poll error state (e.g., pollError and setPollError) and update loadStatus to clear pollError on success (after setting setStatus/setAssets/setDownloads) and set pollError with the caught error in the catch block; ensure this new pollError is exposed/used alongside statusError in the component UI so polling failures are surfaced to the user.
151-157:⚠️ Potential issue | 🟠 MajorSerialize the polling loop to avoid stale overwrites.
setIntervalstarts a newloadStatus()every 1.5s even when the previous one is still in flight. If one of those slower requests resolves last, it can overwrite fresherstatus/assets/downloadsdata; the same late completion can also still callsetStateafter unmount. Prefer a self-schedulingsetTimeoutloop (or an in-flight guard/request id) so only one poll runs at a time.Suggested direction
-useEffect(() => { - void loadStatus(); - const timer = setInterval(() => { - void loadStatus(); - }, 1500); - return () => clearInterval(timer); -}, []); +useEffect(() => { + let cancelled = false; + let timer: ReturnType<typeof setTimeout> | null = null; + + const poll = async () => { + try { + await loadStatus(); + } finally { + if (!cancelled) { + timer = setTimeout(() => { + void poll(); + }, 1500); + } + } + }; + + void poll(); + + return () => { + cancelled = true; + if (timer) clearTimeout(timer); + }; +}, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/settings/panels/LocalModelDebugPanel.tsx` around lines 151 - 157, The polling loop using setInterval in the useEffect should be serialized to avoid concurrent loadStatus() calls and stale state updates: modify the effect that currently calls loadStatus() and sets timer via setInterval so it instead uses a self-scheduling pattern (setTimeout) or an in-flight guard/request-id around loadStatus() to ensure only one request runs at a time; also ensure any pending async work checks a mounted flag or the request-id before calling setState on status/assets/downloads to avoid updating state after unmount. Target the useEffect and loadStatus symbols and replace the setInterval/clearInterval logic with a serialized loop or guard as described.app/src/components/settings/panels/AutocompleteDebugPanel.tsx (1)
148-173:⚠️ Potential issue | 🟠 MajorGate Start/Save on a real config load here too.
This panel still seeds
debounceMs,maxChars,overlayTtlMs, etc. fromDEFAULT_CONFIG, but unlike the main panel it never tracks whetherload()actually replaced those defaults with backend values. IfopenhumanGetConfig()is slow or fails while status polling succeeds,Startcan run with120msandSavecan write default-backed advanced settings. Please add a loaded flag/ref here as well and keep both actions disabled until hydration completes.Also applies to: 239-265, 393-428, 490-503, 625-631
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/settings/panels/AutocompleteDebugPanel.tsx` around lines 148 - 173, The load() flow currently seeds UI fields (setDebounceMs, setMaxChars, setOverlayTtlMs, setStyleInstructions, setStyleExamplesText) from DEFAULT_CONFIG without tracking whether openhumanGetConfig() actually hydrated values, allowing Start/Save to run with defaults; add a boolean state/ref (e.g., isHydrated or configLoaded) initialized false, set it false at start of load(), set it true only after parseAutocompleteConfig(...) succeeds and fields are set, and leave it false on error; then update the Start and Save controls (the buttons or handlers that call start/save actions) to be disabled unless isHydrated is true; apply the same change pattern to the other affected blocks noted (lines 239-265, 393-428, 490-503, 625-631) to ensure no actions run before backend config hydration.
🧹 Nitpick comments (2)
app/src/components/settings/panels/ScreenAwarenessDebugPanel.tsx (1)
9-13: Use aninterfaceforDebugSectionpropsPlease extract the inline object shape into an interface for
DebugSectionprops (Lines 9-13) to match repository TypeScript conventions.As per coding guidelines: "Prefer
interfacefor defining object shapes in TypeScript across the app."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/settings/panels/ScreenAwarenessDebugPanel.tsx` around lines 9 - 13, Extract the inline prop type for DebugSection into a named interface: create an interface (e.g., DebugSectionProps) that defines state: ComponentProps<typeof ScreenIntelligenceDebugPanel>['state'], then update the DebugSection function signature to accept props: DebugSectionProps. Reference the DebugSection component and the ScreenIntelligenceDebugPanel type when making the change.app/src/components/settings/panels/AutocompletePanel.tsx (1)
16-57: Extract the autocomplete defaults/parser into a shared helper.
DEFAULT_CONFIGandparseAutocompleteConfig()are now duplicated here and inAutocompleteDebugPanel.tsx. That makes schema changes easy to miss in one panel, and the two screens can drift into parsing/saving different shapes over time. A small shared module for the config defaults + normalization would keep both panels in lockstep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/settings/panels/AutocompletePanel.tsx` around lines 16 - 57, Extract DEFAULT_CONFIG and parseAutocompleteConfig into a small shared module and have both AutocompletePanel and AutocompleteDebugPanel import them; specifically, create and export the constants/functions (preserving the AutocompleteConfig type, DEFAULT_CONFIG, and parseAutocompleteConfig behavior) in a new helper, replace the duplicate definitions in AutocompletePanel and remove the duplicate in AutocompleteDebugPanel, and update both components to import DEFAULT_CONFIG and parseAutocompleteConfig from the shared helper so schema/default changes stay in one place.
🤖 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/AutocompleteDebugPanel.tsx`:
- Around line 244-247: The code currently treats cleared numeric inputs like
debounceMs as 0 because it uses Number(debounceMs); update the conversion before
calling openhumanAutocompleteStart (and the similar conversions around the other
fields referenced) to first check if the raw input string is empty or only
whitespace (e.g. value.trim() === '') and in that case use the intended fallback
(e.g. undefined or the existing default like 120) or prevent the submit,
otherwise coerce to Number and then clamp with Number.isFinite(...) ?
Math.min(Math.max(Number(value), min, ), max) : default; apply this change to
the debounceMs handling and the other numeric inputs used in the
openhumanAutocompleteStart call so cleared inputs are not treated as 0 and do
not silently overwrite config.
In `@app/src/components/settings/panels/ScreenAwarenessDebugPanel.tsx`:
- Around line 67-79: The saveConfig function is allowing submits when
status?.config is undefined which causes fallback defaults to be sent and can
overwrite backend settings; update saveConfig (and the UI enable/disable logic
that currently uses isSavingConfig) to guard against saving until the existing
config is loaded: check a loaded flag or require status?.config to be non-null
at the start of saveConfig and return early (or set a user-facing error) if it's
missing, and also change the UI button disabling condition (where isSavingConfig
is used) to also include a check like !status?.config (or a dedicated
isConfigLoaded boolean) so the save action and
openhumanUpdateScreenIntelligenceSettings call only run when status.config is
available.
In `@app/src/components/settings/panels/VoiceDebugPanel.tsx`:
- Around line 63-67: The polling success path currently clears the shared error
state used by both loadData() and saveSettings(), which lets a successful poll
erase a recent save failure; introduce separate error state(s) (e.g., saveError
vs pollError) or tag errors by source so saveSettings sets and reads a dedicated
saveError instead of the shared error, and update references to setError/get
error reads in loadData, saveSettings, and any UI render logic (including use of
setError in the catch blocks and the finally blocks noted) to use the
appropriate new state or conditional clearing so a successful poll does not
clear a saveSettings failure.
- Around line 213-215: The onChange handler in VoiceDebugPanel currently uses
Number(e.target.value) || 0.002 which treats an explicit 0 as falsy and
overwrites it; update the handler that calls updateSetting('silence_threshold',
...) to accept 0 by parsing the input (e.g., parseFloat or Number) and only fall
back to 0.002 when the parsed value is NaN or the input is empty—refer to the
onChange lambda in VoiceDebugPanel and the updateSetting call to implement this
change (use Number.isNaN(parsed) or an explicit input === '' check instead of
||).
---
Duplicate comments:
In `@app/src/components/settings/panels/AutocompleteDebugPanel.tsx`:
- Around line 148-173: The load() flow currently seeds UI fields (setDebounceMs,
setMaxChars, setOverlayTtlMs, setStyleInstructions, setStyleExamplesText) from
DEFAULT_CONFIG without tracking whether openhumanGetConfig() actually hydrated
values, allowing Start/Save to run with defaults; add a boolean state/ref (e.g.,
isHydrated or configLoaded) initialized false, set it false at start of load(),
set it true only after parseAutocompleteConfig(...) succeeds and fields are set,
and leave it false on error; then update the Start and Save controls (the
buttons or handlers that call start/save actions) to be disabled unless
isHydrated is true; apply the same change pattern to the other affected blocks
noted (lines 239-265, 393-428, 490-503, 625-631) to ensure no actions run before
backend config hydration.
In `@app/src/components/settings/panels/LocalModelDebugPanel.tsx`:
- Around line 135-148: The poll currently swallows errors in loadStatus, leaving
stale UI state; add a dedicated poll error state (e.g., pollError and
setPollError) and update loadStatus to clear pollError on success (after setting
setStatus/setAssets/setDownloads) and set pollError with the caught error in the
catch block; ensure this new pollError is exposed/used alongside statusError in
the component UI so polling failures are surfaced to the user.
- Around line 151-157: The polling loop using setInterval in the useEffect
should be serialized to avoid concurrent loadStatus() calls and stale state
updates: modify the effect that currently calls loadStatus() and sets timer via
setInterval so it instead uses a self-scheduling pattern (setTimeout) or an
in-flight guard/request-id around loadStatus() to ensure only one request runs
at a time; also ensure any pending async work checks a mounted flag or the
request-id before calling setState on status/assets/downloads to avoid updating
state after unmount. Target the useEffect and loadStatus symbols and replace the
setInterval/clearInterval logic with a serialized loop or guard as described.
---
Nitpick comments:
In `@app/src/components/settings/panels/AutocompletePanel.tsx`:
- Around line 16-57: Extract DEFAULT_CONFIG and parseAutocompleteConfig into a
small shared module and have both AutocompletePanel and AutocompleteDebugPanel
import them; specifically, create and export the constants/functions (preserving
the AutocompleteConfig type, DEFAULT_CONFIG, and parseAutocompleteConfig
behavior) in a new helper, replace the duplicate definitions in
AutocompletePanel and remove the duplicate in AutocompleteDebugPanel, and update
both components to import DEFAULT_CONFIG and parseAutocompleteConfig from the
shared helper so schema/default changes stay in one place.
In `@app/src/components/settings/panels/ScreenAwarenessDebugPanel.tsx`:
- Around line 9-13: Extract the inline prop type for DebugSection into a named
interface: create an interface (e.g., DebugSectionProps) that defines state:
ComponentProps<typeof ScreenIntelligenceDebugPanel>['state'], then update the
DebugSection function signature to accept props: DebugSectionProps. Reference
the DebugSection component and the ScreenIntelligenceDebugPanel type when making
the change.
🪄 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: 8c786abe-4646-4cd6-9b0e-47728d9dfc6b
📒 Files selected for processing (5)
app/src/components/settings/panels/AutocompleteDebugPanel.tsxapp/src/components/settings/panels/AutocompletePanel.tsxapp/src/components/settings/panels/LocalModelDebugPanel.tsxapp/src/components/settings/panels/ScreenAwarenessDebugPanel.tsxapp/src/components/settings/panels/VoiceDebugPanel.tsx
Comment on lines
+244
to
+247
| const debounce = Number(debounceMs); | ||
| appendUiLog(`start requested (debounce=${String(debounce)}ms)`); | ||
| const response = await openhumanAutocompleteStart({ | ||
| debounce_ms: Number.isFinite(debounce) ? Math.min(Math.max(debounce, 50), 2000) : 120, |
Contributor
There was a problem hiding this comment.
Treat cleared numeric inputs as empty, not as 0.
Number('') evaluates to 0, so if the user clears one of these fields you currently clamp and persist the minimum (50, 32, 300) instead of falling back or blocking the action. Check for value.trim() === '' before coercion so an empty field cannot silently rewrite the config.
Possible fix
+ const parseClampedNumber = (
+ raw: string,
+ min: number,
+ max: number,
+ fallback: number
+ ) => {
+ if (raw.trim() === '') return fallback;
+ const value = Number(raw);
+ return Number.isFinite(value) ? Math.min(Math.max(value, min), max) : fallback;
+ };
+
const start = async () => {
if (!isTauri()) return;
setError(null);
setMessage(null);
try {
- const debounce = Number(debounceMs);
+ const debounce = parseClampedNumber(debounceMs, 50, 2000, 120);
appendUiLog(`start requested (debounce=${String(debounce)}ms)`);
const response = await openhumanAutocompleteStart({
- debounce_ms: Number.isFinite(debounce) ? Math.min(Math.max(debounce, 50), 2000) : 120,
+ debounce_ms: debounce,
});- const debounce = Number(debounceMs);
- const max = Number(maxChars);
- const ttl = Number(overlayTtlMs);
+ const debounce = parseClampedNumber(debounceMs, 50, 2000, 120);
+ const max = parseClampedNumber(maxChars, 32, 1200, 384);
+ const ttl = parseClampedNumber(overlayTtlMs, 300, 10000, 1100);
const response = await openhumanAutocompleteSetStyle({
- debounce_ms: Number.isFinite(debounce) ? Math.min(Math.max(debounce, 50), 2000) : 120,
- max_chars: Number.isFinite(max) ? Math.min(Math.max(max, 32), 1200) : 384,
- overlay_ttl_ms: Number.isFinite(ttl) ? Math.min(Math.max(ttl, 300), 10000) : 1100,
+ debounce_ms: debounce,
+ max_chars: max,
+ overlay_ttl_ms: ttl,Also applies to: 400-406
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/settings/panels/AutocompleteDebugPanel.tsx` around lines
244 - 247, The code currently treats cleared numeric inputs like debounceMs as 0
because it uses Number(debounceMs); update the conversion before calling
openhumanAutocompleteStart (and the similar conversions around the other fields
referenced) to first check if the raw input string is empty or only whitespace
(e.g. value.trim() === '') and in that case use the intended fallback (e.g.
undefined or the existing default like 120) or prevent the submit, otherwise
coerce to Number and then clamp with Number.isFinite(...) ?
Math.min(Math.max(Number(value), min, ), max) : default; apply this change to
the debounceMs handling and the other numeric inputs used in the
openhumanAutocompleteStart call so cleared inputs are not treated as 0 and do
not silently overwrite config.
Comment on lines
+67
to
+79
| const saveConfig = async () => { | ||
| if (!isTauri()) return; | ||
| setConfigError(null); | ||
| setIsSavingConfig(true); | ||
| try { | ||
| const fps = Number(baselineFps); | ||
| await openhumanUpdateScreenIntelligenceSettings({ | ||
| enabled: status?.config.enabled ?? false, | ||
| policy_mode: | ||
| status?.config.policy_mode === 'whitelist_only' | ||
| ? 'whitelist_only' | ||
| : 'all_except_blacklist', | ||
| baseline_fps: Number.isFinite(fps) && fps > 0 ? fps : 1, |
Contributor
There was a problem hiding this comment.
Block save until config is loaded to prevent accidental policy resets
At Line 74 and Line 76, fallback values are submitted when status?.config is still undefined. Since Line 178 only disables on isSavingConfig, users can save too early and overwrite backend settings with defaults.
Suggested fix
+ const isConfigReady = Boolean(status?.config);
+
const saveConfig = async () => {
- if (!isTauri()) return;
+ if (!isTauri() || !status?.config) return;
setConfigError(null);
setIsSavingConfig(true);
try {
const fps = Number(baselineFps);
await openhumanUpdateScreenIntelligenceSettings({
- enabled: status?.config.enabled ?? false,
+ enabled: status.config.enabled ?? false,
policy_mode:
- status?.config.policy_mode === 'whitelist_only'
+ status.config.policy_mode === 'whitelist_only'
? 'whitelist_only'
: 'all_except_blacklist',
baseline_fps: Number.isFinite(fps) && fps > 0 ? fps : 1,
use_vision_model: useVisionModel,
keep_screenshots: keepScreenshots,
@@
<button
type="button"
onClick={() => void saveConfig()}
- disabled={isSavingConfig}
+ disabled={isSavingConfig || !isConfigReady}
className="rounded-lg border border-primary-400 bg-primary-50 px-3 py-2 text-sm text-primary-700 disabled:opacity-50">
{isSavingConfig ? 'Saving…' : 'Save Screen Intelligence Settings'}
</button>Also applies to: 175-179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/settings/panels/ScreenAwarenessDebugPanel.tsx` around
lines 67 - 79, The saveConfig function is allowing submits when status?.config
is undefined which causes fallback defaults to be sent and can overwrite backend
settings; update saveConfig (and the UI enable/disable logic that currently uses
isSavingConfig) to guard against saving until the existing config is loaded:
check a loaded flag or require status?.config to be non-null at the start of
saveConfig and return early (or set a user-facing error) if it's missing, and
also change the UI button disabling condition (where isSavingConfig is used) to
also include a check like !status?.config (or a dedicated isConfigLoaded
boolean) so the save action and openhumanUpdateScreenIntelligenceSettings call
only run when status.config is available.
Comment on lines
+63
to
+67
| setError(null); | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : 'Failed to load voice debug data'; | ||
| setError(message); | ||
| } finally { |
Contributor
There was a problem hiding this comment.
Polling success can erase save failures too quickly.
loadData() and saveSettings() both write to the same error state. A successful poll can clear a recent save error, which makes failed saves easy to miss.
Suggested fix
- const [error, setError] = useState<string | null>(null);
+ const [pollError, setPollError] = useState<string | null>(null);
+ const [actionError, setActionError] = useState<string | null>(null);
...
- setError(null);
+ setPollError(null);
...
- setError(message);
+ setPollError(message);
...
- setError(null);
+ setActionError(null);
...
- setError(message);
+ setActionError(message);
...
- {error && (
+ {(actionError || pollError) && (
<div className="rounded-md border border-red-200 bg-red-50 p-3 text-xs text-red-600">
- {error}
+ {actionError ?? pollError}
</div>
)}Also applies to: 72-76, 91-107, 222-226
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/settings/panels/VoiceDebugPanel.tsx` around lines 63 - 67,
The polling success path currently clears the shared error state used by both
loadData() and saveSettings(), which lets a successful poll erase a recent save
failure; introduce separate error state(s) (e.g., saveError vs pollError) or tag
errors by source so saveSettings sets and reads a dedicated saveError instead of
the shared error, and update references to setError/get error reads in loadData,
saveSettings, and any UI render logic (including use of setError in the catch
blocks and the finally blocks noted) to use the appropriate new state or
conditional clearing so a successful poll does not clear a saveSettings failure.
Comment on lines
+213
to
+215
| onChange={e => | ||
| updateSetting('silence_threshold', Number(e.target.value) || 0.002) | ||
| } |
Contributor
There was a problem hiding this comment.
Allow an explicit 0 value for silence threshold.
Line 214 uses Number(e.target.value) || 0.002, which converts a valid 0 input into 0.002.
Suggested fix
- onChange={e =>
- updateSetting('silence_threshold', Number(e.target.value) || 0.002)
- }
+ onChange={e => {
+ const raw = e.target.value;
+ const next = raw === '' ? 0.002 : Number(raw);
+ updateSetting('silence_threshold', Number.isFinite(next) ? next : 0.002);
+ }}📝 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.
Suggested change
| onChange={e => | |
| updateSetting('silence_threshold', Number(e.target.value) || 0.002) | |
| } | |
| onChange={e => { | |
| const raw = e.target.value; | |
| const next = raw === '' ? 0.002 : Number(raw); | |
| updateSetting('silence_threshold', Number.isFinite(next) ? next : 0.002); | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/settings/panels/VoiceDebugPanel.tsx` around lines 213 -
215, The onChange handler in VoiceDebugPanel currently uses
Number(e.target.value) || 0.002 which treats an explicit 0 as falsy and
overwrites it; update the handler that calls updateSetting('silence_threshold',
...) to accept 0 by parsing the input (e.g., parseFloat or Number) and only fall
back to 0.002 when the parsed value is NaN or the input is empty—refer to the
onChange lambda in VoiceDebugPanel and the updateSetting call to implement this
change (use Number.isNaN(parsed) or an explicit input === '' check instead of
||).
- Update ScreenIntelligencePanel test for renamed title, button text, and platform message - Rewrite AutocompletePanel test for simplified panel (dev options moved to AutocompleteDebugPanel)
This was referenced Apr 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Before → After
What moved to Developer Options
Test plan
yarn typecheckpasses with 0 errorsyarn lintpasses with 0 errors (6 pre-existing warnings)Summary by CodeRabbit
New Features
Refactor
Removed