Improve autocomplete observability and runtime controls in settings + JSON-RPC#308
Conversation
- Introduced MAX_LOG_ENTRIES constant to limit log entries to 200. - Updated log formatting to include timestamps with milliseconds for better precision. - Added UI logging for various actions (e.g., saving settings, starting/stopping autocomplete) to improve traceability. - Enhanced error handling in refreshStatus, acceptSuggestion, and other functions to log specific failure messages. - Added unit tests for AutocompletePanel to ensure functionality and logging behavior. This update improves the overall user experience by providing clearer logs and better error handling in the Autocomplete feature.
📝 WalkthroughWalkthroughEnhanced autocomplete: improved UI logging and lifecycle flows, added millisecond timestamps and capped logs, preserved suggestions on timeouts, added skip_apply to accept, increased timeouts, richer RPC logs, and new frontend and macOS E2E tests covering the full autocomplete lifecycle. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI (AutocompletePanel)
participant RPC as RPC Server
participant Engine as Autocomplete Engine
participant OS as Accessibility / Focus
UI->>RPC: openhuman.autocomplete_set_style(params)
RPC->>Engine: set_style(params)
Engine-->>RPC: result + logs
RPC-->>UI: response + logs
UI->>RPC: openhuman.autocomplete_start()
RPC->>Engine: start()
Engine-->>RPC: started + status logs
RPC-->>UI: status + logs
UI->>RPC: openhuman.autocomplete_current(context)
RPC->>Engine: current(context)
Engine->>OS: validate_focused_target()
OS-->>Engine: focus info
Engine-->>RPC: suggestion + logs
RPC-->>UI: suggestion + logs
UI->>RPC: openhuman.autocomplete_accept { suggestion, skip_apply? }
RPC->>Engine: accept(suggestion, skip_apply)
Engine->>OS: try_apply_to_focused_field (if not skip_apply)
OS-->>Engine: apply result / error
Engine-->>RPC: accept result + logs
RPC-->>UI: accept response + logs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
… handling and logging - Updated to log role changes as debug information instead of returning an error, allowing for more flexible handling of role fluctuations. - Increased the timeout for autocomplete refresh operations from 15 seconds to 120 seconds to accommodate longer processing times, enhancing reliability. - Improved error handling in the autocomplete engine to preserve previous suggestions and provide clearer error messages when operations are aborted. These changes enhance the user experience by providing better logging and more robust handling of focus and autocomplete functionalities.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/AutocompletePanel.tsx`:
- Around line 195-205: refreshStatus is currently appending every
autocomplete_status log (appendLogs(response.logs)) even when called by the 1.2s
poller; change it so only manual/action-triggered refreshes append raw logs—use
the showSpinner boolean to gate appendLogs (call appendLogs(response.logs) only
when showSpinner is true) while leaving trackStatusChanges(response.result) and
setStatus(response.result) unchanged so polling relies on trackStatusChanges
without drowning the log buffer.
- Around line 355-356: The history RPC call after accepting a completion races
the backend write because AutocompleteEngine::accept() persists in a detached
tokio::spawn; update the client-side flow in AutocompletePanel.tsx to avoid
missing the just-accepted entry by replacing the single await loadHistory() call
with a short retry/poll loop (e.g., call loadHistory(), if the expected entry
isn't present sleep a small interval and retry up to N times) after await
refreshStatus(); alternatively, if you prefer server-side change, make
AutocompleteEngine::accept() await the primary history save before returning so
loadHistory() is consistent — reference the loadHistory() and refreshStatus()
calls in AutocompletePanel.tsx and the AutocompleteEngine::accept() persistence
behavior when implementing the fix.
- Around line 345-354: The settings-panel accept path must call
openhumanAutocompleteAccept with skip_apply: true to avoid backend attempting an
AX insertion; update the openhumanAutocompleteAccept invocation (where
appendUiLog is called and suggestion is passed via status?.suggestion?.value) to
include skip_apply: true, keep appendLogs(response.logs) as-is, and change the
success check to use response.result.accepted (and response.result.value) rather
than response.result.applied when deciding the setMessage text and fallback
reason.
In `@src/openhuman/accessibility/focus.rs`:
- Around line 355-361: The current validate_focused_target logic treats any role
mismatch as inconclusive; change it to only relax mismatches between editable
text roles (e.g., "AXTextArea" and "AXTextField") while keeping all other role
changes fatal: inside validate_focused_target, replace the unconditional "role
changed ... proceeding" branch with a conditional that checks if both expected
and actual are text-editable roles (use the exact role string comparisons for
"AXTextArea" and "AXTextField" or a small helper like
is_text_editable_role(expected/actual)); if both are editable, log the debug and
continue, otherwise return the existing error/abort path so callers such as
accept() and try_accept_via_tab() will stop on non-text role mismatches.
In `@src/openhuman/autocomplete/core/engine.rs`:
- Around line 145-149: The timeout branch around time::timeout(...) that calls
engine.refresh(None) can leave a stale `suggestion` that mismatches new focus
metadata; update the timeout handling in the refresh block (and the similar
branch around lines 186-197) to either validate the preserved `suggestion`
against a snapshot of `{context, app, role}` taken before refresh or simply
clear `self.suggestion` when the timeout fires so `accept()` cannot apply an
outdated completion; locate references to engine.refresh(None), the `suggestion`
field, and the accept() path and ensure you only keep the suggestion when the
stored snapshot matches the post-refresh metadata, otherwise set suggestion to
None and avoid writing accepted-history with mismatched metadata.
🪄 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: 65feccd0-0f40-4246-89b6-d39ccac0bd37
📒 Files selected for processing (8)
app/src/components/settings/panels/AutocompletePanel.tsxapp/src/components/settings/panels/__tests__/AutocompletePanel.test.tsxsrc/openhuman/accessibility/focus.rssrc/openhuman/autocomplete/core/engine.rssrc/openhuman/autocomplete/ops.rssrc/openhuman/autocomplete/schemas.rssrc/openhuman/local_ai/service/bootstrap.rstests/json_rpc_e2e.rs
- Refactored loadHistory to return an empty array when Tauri is not available, improving error handling. - Introduced waitForAcceptedHistoryEntry to ensure the history is loaded before accepting suggestions, enhancing user experience. - Updated the accept suggestion logic to use the new waitForAcceptedHistoryEntry function, ensuring the correct suggestion is applied. - Modified tests to reflect changes in the accept suggestion API, ensuring accurate functionality. These changes improve the reliability and responsiveness of the autocomplete feature.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/autocomplete/core/engine.rs (1)
145-224:⚠️ Potential issue | 🟡 MinorSnapshot comparison may race with partial state updates during refresh.
The pre-refresh snapshot is captured before
refresh(None)is called, butrefresh()updatesapp_name,target_role, andupdated_at_msat line 555-557 before inference completes. If inference times out after these updates, the post-snapshot will differ even though the suggestion (if any) was generated for the new context, not the pre-snapshot context.This could cause unnecessary clearing of valid suggestions or, conversely, retain suggestions that should be cleared. Consider snapshotting after the context capture inside
refresh()returns, or accepting that the conservative "clear on any drift" is acceptable for a 120s edge case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/autocomplete/core/engine.rs` around lines 145 - 224, The pre_refresh_snapshot comparison can race with partial updates because pre_refresh_snapshot is taken before engine.refresh(None) while refresh() mutates state.app_name and state.target_role early; as a fix, capture the "pre" snapshot immediately before calling refresh and capture the "post" snapshot after refresh returns (i.e., move where post_refresh_snapshot is taken to after the timeout branch when refresh completes/errs) or alternatively adjust the logic to compare against the snapshot of metadata that refresh actually began with by exposing/returning the refresh-start metadata from engine.refresh (e.g., have engine.refresh return its initial context/app_name/target_role or provide a method like refresh_start_snapshot) and use those values instead of reading state midway; update references around pre_refresh_snapshot, post_refresh_snapshot, engine.refresh, and state.suggestion/state.app_name/target_role accordingly.
🧹 Nitpick comments (1)
app/src/components/settings/panels/__tests__/AutocompletePanel.test.tsx (1)
208-293: Comprehensive happy-path test covering the full autocomplete lifecycle.The test validates:
- Initial status display
- Settings save with updated debounce/max_chars/accept_with_tab
- Start and running state reflection
- Get suggestion with context override
- Accept with
skip_apply: true(matching the schema change)- Log display and clear behavior
This provides good end-to-end coverage for the panel's primary flow.
Consider adding a second test case for error/edge paths (e.g., start when
enabled=false, accept with no suggestion) to increase resilience against regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/settings/panels/__tests__/AutocompletePanel.test.tsx` around lines 208 - 293, Add a second test in AutocompletePanel.test.tsx to cover error/edge paths: create a new it(...) that simulates starting when the autocomplete feature is disabled (e.g., ensure Platform supported or Running toggles to false/disabled) and verifies openhumanAutocompleteStart is not called and the UI shows the correct error/disabled state, and test accepting with no current suggestion to confirm openhumanAutocompleteAccept is not called and the UI shows an appropriate message; reuse the existing helpers/selectors (e.g., buttons named 'Start', 'Get Suggestion', 'Accept Suggestion', and functions openhumanAutocompleteStart/openhumanAutocompleteAccept/openhumanAutocompleteCurrent) to drive and assert behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/openhuman/autocomplete/core/engine.rs`:
- Around line 145-224: The pre_refresh_snapshot comparison can race with partial
updates because pre_refresh_snapshot is taken before engine.refresh(None) while
refresh() mutates state.app_name and state.target_role early; as a fix, capture
the "pre" snapshot immediately before calling refresh and capture the "post"
snapshot after refresh returns (i.e., move where post_refresh_snapshot is taken
to after the timeout branch when refresh completes/errs) or alternatively adjust
the logic to compare against the snapshot of metadata that refresh actually
began with by exposing/returning the refresh-start metadata from engine.refresh
(e.g., have engine.refresh return its initial context/app_name/target_role or
provide a method like refresh_start_snapshot) and use those values instead of
reading state midway; update references around pre_refresh_snapshot,
post_refresh_snapshot, engine.refresh, and
state.suggestion/state.app_name/target_role accordingly.
---
Nitpick comments:
In `@app/src/components/settings/panels/__tests__/AutocompletePanel.test.tsx`:
- Around line 208-293: Add a second test in AutocompletePanel.test.tsx to cover
error/edge paths: create a new it(...) that simulates starting when the
autocomplete feature is disabled (e.g., ensure Platform supported or Running
toggles to false/disabled) and verifies openhumanAutocompleteStart is not called
and the UI shows the correct error/disabled state, and test accepting with no
current suggestion to confirm openhumanAutocompleteAccept is not called and the
UI shows an appropriate message; reuse the existing helpers/selectors (e.g.,
buttons named 'Start', 'Get Suggestion', 'Accept Suggestion', and functions
openhumanAutocompleteStart/openhumanAutocompleteAccept/openhumanAutocompleteCurrent)
to drive and assert behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bcc0b070-5ed2-4519-958b-c9b95314912b
📒 Files selected for processing (4)
app/src/components/settings/panels/AutocompletePanel.tsxapp/src/components/settings/panels/__tests__/AutocompletePanel.test.tsxsrc/openhuman/accessibility/focus.rssrc/openhuman/autocomplete/core/engine.rs
Brings in Discord server/channel picker (tinyhumansai#349) and autocomplete observability improvements (tinyhumansai#308) from main. Resolves conflict in tests/json_rpc_e2e.rs by keeping both the screen intelligence tests (this branch) and the autocomplete runtime settings test (main). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Improves Inline Autocomplete diagnostics and control flow across UI and core RPC by adding structured logs, better action/error feedback, and end-to-end coverage for runtime settings and accept behavior.
Changes
Add
MAX_LOG_ENTRIEScap (200) and millisecond-precision timestamps in AutocompletePanel logsAdd
[ui-flow]logs for key UI actions:Improve start/stop UX states:
Harden panel error handling with specific failure logs/messages:
Accept flow improvements:
openhumanAutocompleteAcceptAdd
skip_applyto autocomplete accept schema metadataExpand Rust RPC logs:
[autocomplete]logs for endpointsAdd tests:
set_style → start → status → current → accept(skip_apply) → stopProblem
Solution
Add structured observability and improved runtime feedback across UI and core.
UI (AutocompletePanel)
Core (
src/openhuman/autocomplete/ops.rs)Replace single-line logs with richer
RpcOutcome::new(...)log sets for:Include structured fields:
Schema (
schemas.rs)skip_applyinput field toautocomplete.acceptmetadataTests
skip_applySubmission Checklist
Unit Tests
Vitest
app/src/components/settings/panels/__tests__/AutocompletePanel.test.tsxRust
E2E / Integration
Added:
tests/json_rpc_e2e.rsjson_rpc_autocomplete_runtime_settings_and_logs_flowCovers:
skip_applyaccept behaviorDoc Comments
Inline Comments
Impact
Improved debuggability for autocomplete lifecycle and failures
More predictable settings panel runtime controls
Increased confidence via:
Low risk:
Related
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests