fix(autocomplete): reliability bugs and in-app ghost text (#107)#179
Conversation
reqwest::Client::new() creates a client with no timeout, causing inference calls to block indefinitely when Ollama is slow or unreachable. Admin endpoints had explicit timeouts but inference did not. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ifications - Wrap engine loop refresh() in 15s tokio timeout to prevent blocking - Handle AX error -1728 (Electron apps) gracefully as idle, not error - Clear stale state (last_error, suggestion, context) on stop() - Add skip_apply flag to AutocompleteAcceptParams so frontend can accept without triggering AppleScript text insertion - Skip AX-based Tab accept and overlay when OpenHuman is focused - Guard all show_overflow_badge calls with app_name check to suppress Script Editor notifications when typing in the chat composer - Cache inference results: skip Ollama call when context is unchanged and a suggestion already exists Closes tinyhumansai#107 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Frontend passes skip_apply: true when accepting ghost text so the backend only persists for personalization without AppleScript insertion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix getInlineCompletionSuffix to handle backend's continuation suffix format (not just full-string format) - Add 120s safety timeout to clear isSending if no response arrives - Match textarea CSS to overlay div (leading-normal, whitespace-pre-wrap, break-words, font-sans) so ghost text aligns on multi-line wrap - Pass skip_apply: true when accepting to prevent double text insertion - Add Right Arrow key acceptance when cursor is at end of input Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 Walkthrough🚥 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)
⚔️ Resolve merge conflicts
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/openhuman/local_ai/service/bootstrap.rs (1)
44-47: Log the HTTP client fallback path.If client construction fails here we silently boot with a different HTTP path in the middle of a timeout/reliability fix. A warn/debug log with the build error would make these failures much easier to trace when local AI requests start behaving unexpectedly.
As per coding guidelines: "Use log/tracing at debug or trace level in Rust for critical checkpoints (entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, error handling paths)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/local_ai/service/bootstrap.rs` around lines 44 - 47, The current reqwest client construction uses .build().unwrap_or_else(|_| reqwest::Client::new()) which swallows the build error; change the unwrap_or_else closure to capture the error and log it (e.g., using tracing::warn! or debug!) before returning the fallback reqwest::Client::new(). Update the closure around reqwest::Client::builder().timeout(...).build() to accept the build error, emit a clear log message including the error and context (e.g., "reqwest client build failed, falling back to default client"), then return the fallback client.
🤖 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/pages/Conversations.tsx`:
- Around line 505-512: The timeout handler currently only flips UI state
(sendingTimeoutRef, setIsSending, dispatch(setActiveThread(null))) but does not
cancel or mark the in-flight send, nor do successful local-model completes clear
the timer; fix by (1) associating each send with a unique sendId and storing it
(e.g., currentSendIdRef) so late responses can be ignored if their sendId !==
currentSendIdRef, (2) when the timeout fires, mark the active send as
cancelled/expired (set a flag or update a cancel map) so socket/local-model
response handlers drop results, and (3) ensure all normal completion paths for
socket and local-model sends clearTimeout(sendingTimeoutRef.current) and reset
sendingTimeoutRef.current = null so no stale fallback remains; update the
socket/local-model response handlers to check the sendId/cancel flag before
appending messages.
- Around line 69-77: The current getInlineCompletionSuffix function incorrectly
returns the raw suggestion when it doesn't start with the current input, which
can surface stale/ghost inline text; change it so only a true continuation is
accepted: if suggestion.startsWith(input) return suggestion.slice(input.length)
(or ''), otherwise return an empty string (do not fall back to returning
suggestion). Update the logic in getInlineCompletionSuffix to drop non-prefix
suggestions (use input and suggestion variables and the suggestion.startsWith
check) so stale inlineSuggestionValue cannot be shown.
In `@src/openhuman/autocomplete/core.rs`:
- Around line 592-595: In the conditional inside
src/openhuman/autocomplete/core.rs where errors are being downgraded (the clause
using is_no_text_candidate_error(err) and err.contains(...)), stop broadly
matching "AXFocusedUIElement"; instead only downgrade the specific AX error you
intend (the -1728 case). Update the condition that currently checks
err.contains("AXFocusedUIElement") to either remove that check entirely or
replace it with a strict match that includes the -1728 code (e.g., check for the
exact error token that pairs AXFocusedUIElement with ERROR:-1728), so only the
exact AX -1728 message is suppressed and other AXFocusedUIElement failures are
allowed to surface.
- Around line 462-464: The accept() path still triggers the in-app notification
via show_overflow_badge("accepted", ...) because callers call
openhumanAutocompleteAccept() without skip_apply; fix by ensuring the client
call supplies skip_apply: true (i.e., call openhumanAutocompleteAccept({ ...,
skip_apply: true })) and/or make accept() respect the flag by checking
skip_apply before invoking show_overflow_badge (change the condition to if
should_apply && !skip_apply { show_overflow_badge(...) }), referencing the
openhumanAutocompleteAccept callsite and the accept()/show_overflow_badge
symbols to locate the changes.
- Around line 660-667: The cache-hit branch in the block that locks self.inner
and checks state.context/state.suggestion returns early without refreshing
metadata or considering the frontmost app, causing stale app_name/phase (e.g.,
phase set to capturing_context and app_name left as "OpenHuman"); change the
cache check in that branch (the code using self.inner.lock().await and checking
state.context and state.suggestion) to include the frontmost app/source in the
cache key (compare both context and current_frontmost_app) and before returning
the cached suggestion update the mutable state fields (phase and
app_name/frontmost source) from the current environment so try_accept_via_tab()
sees fresh metadata; ensure you still short-circuit only when both context and
frontmost app match and the cached suggestion is valid.
---
Nitpick comments:
In `@src/openhuman/local_ai/service/bootstrap.rs`:
- Around line 44-47: The current reqwest client construction uses
.build().unwrap_or_else(|_| reqwest::Client::new()) which swallows the build
error; change the unwrap_or_else closure to capture the error and log it (e.g.,
using tracing::warn! or debug!) before returning the fallback
reqwest::Client::new(). Update the closure around
reqwest::Client::builder().timeout(...).build() to accept the build error, emit
a clear log message including the error and context (e.g., "reqwest client build
failed, falling back to default client"), then return the fallback client.
🪄 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: 845ac7aa-3921-4460-8faa-2dd02cedc079
📒 Files selected for processing (4)
app/src/pages/Conversations.tsxapp/src/utils/tauriCommands.tssrc/openhuman/autocomplete/core.rssrc/openhuman/local_ai/service/bootstrap.rs
| function getInlineCompletionSuffix(input: string, suggestion: string): string { | ||
| const normalizedInput = input; | ||
| const normalizedSuggestion = suggestion; | ||
|
|
||
| if (!normalizedInput || !normalizedSuggestion) { | ||
| return ''; | ||
| } | ||
|
|
||
| if (normalizedSuggestion === normalizedInput) { | ||
| return ''; | ||
| } | ||
|
|
||
| if (normalizedSuggestion.startsWith(normalizedInput)) { | ||
| return normalizedSuggestion.slice(normalizedInput.length); | ||
| if (!input || !suggestion) return ''; | ||
| // If backend returns full string (starts with input), extract the suffix portion. | ||
| if (suggestion.startsWith(input)) { | ||
| const suffix = suggestion.slice(input.length); | ||
| return suffix || ''; | ||
| } | ||
|
|
||
| return ''; | ||
| // Backend's inline_complete returns continuation suffix directly — use as-is. | ||
| return suggestion; |
There was a problem hiding this comment.
Don't treat every non-prefix completion as a valid suffix.
Returning the raw suggestion here is only safe if the response is guaranteed to match the current input. The autocomplete poller can still leave an older inlineSuggestionValue around during send/mode transitions, and this fallback turns those stale responses into visible ghost text instead of dropping them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/Conversations.tsx` around lines 69 - 77, The current
getInlineCompletionSuffix function incorrectly returns the raw suggestion when
it doesn't start with the current input, which can surface stale/ghost inline
text; change it so only a true continuation is accepted: if
suggestion.startsWith(input) return suggestion.slice(input.length) (or ''),
otherwise return an empty string (do not fall back to returning suggestion).
Update the logic in getInlineCompletionSuffix to drop non-prefix suggestions
(use input and suggestion variables and the suggestion.startsWith check) so
stale inlineSuggestionValue cannot be shown.
| // Safety: auto-clear isSending if no response arrives within 120s | ||
| if (sendingTimeoutRef.current) clearTimeout(sendingTimeoutRef.current); | ||
| sendingTimeoutRef.current = setTimeout(() => { | ||
| console.warn('[chat] safety timeout: clearing isSending after 120s with no response'); | ||
| setIsSending(false); | ||
| dispatch(setActiveThread(null)); | ||
| sendingTimeoutRef.current = null; | ||
| }, 120_000); |
There was a problem hiding this comment.
The 120s timer unlocks the composer without retiring the send.
This handler only clears UI state; it does not cancel or orphan the in-flight socket/local-model request. After it fires, the user can submit again and the late response from the first send will still append to the thread out of order. The local-model path also never clears this timer on normal completion, so successful local sends keep a stale fallback timer alive until another send replaces it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/Conversations.tsx` around lines 505 - 512, The timeout handler
currently only flips UI state (sendingTimeoutRef, setIsSending,
dispatch(setActiveThread(null))) but does not cancel or mark the in-flight send,
nor do successful local-model completes clear the timer; fix by (1) associating
each send with a unique sendId and storing it (e.g., currentSendIdRef) so late
responses can be ignored if their sendId !== currentSendIdRef, (2) when the
timeout fires, mark the active send as cancelled/expired (set a flag or update a
cancel map) so socket/local-model response handlers drop results, and (3) ensure
all normal completion paths for socket and local-model sends
clearTimeout(sendingTimeoutRef.current) and reset sendingTimeoutRef.current =
null so no stale fallback remains; update the socket/local-model response
handlers to check the sendId/cancel flag before appending messages.
| if should_apply { | ||
| show_overflow_badge("accepted", Some(&cleaned), None, None, None); | ||
| } |
There was a problem hiding this comment.
accept() still hits the in-app notification path.
app/src/components/settings/panels/AutocompletePanel.tsx:285-300 still calls openhumanAutocompleteAccept() without skip_apply: true. This branch will therefore keep invoking show_overflow_badge("accepted", ...) while OpenHuman is frontmost, which is the Script Editor notification this PR is trying to suppress.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/autocomplete/core.rs` around lines 462 - 464, The accept() path
still triggers the in-app notification via show_overflow_badge("accepted", ...)
because callers call openhumanAutocompleteAccept() without skip_apply; fix by
ensuring the client call supplies skip_apply: true (i.e., call
openhumanAutocompleteAccept({ ..., skip_apply: true })) and/or make accept()
respect the flag by checking skip_apply before invoking show_overflow_badge
(change the condition to if should_apply && !skip_apply {
show_overflow_badge(...) }), referencing the openhumanAutocompleteAccept
callsite and the accept()/show_overflow_badge symbols to locate the changes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/pages/Conversations.tsx (1)
140-140:⚠️ Potential issue | 🟠 MajorBind the safety timeout to the send that created it.
sendingTimeoutRefis global. After Line 507 reopens the composer, a latechat:donefrom the older request can still runonDone, append the stale reply, and clear whatever timer now belongs to a newer send. The local-model branch also never retires this timer on normal completion. Track a send id / cancelled flag per send, ignore stale callbacks, and clear the timer in the localfinally.Also applies to: 344-347, 505-512
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/Conversations.tsx` at line 140, The global sendingTimeoutRef causes late chat:done callbacks to affect newer sends; modify the send flow (the function that sets sendingTimeoutRef and the onDone handler referenced by chat:done) to create a per-send identifier or cancelled flag (e.g., a sendId variable captured by the closure) when starting a send, store that id with the timeout, and have onDone and the timeout handler check that the callback's sendId matches the current active send before appending replies or clearing timers; also ensure the local-model branch's finally block clears the per-send timer and marks the send as finished/cancelled so normal completion retires the timer and stale callbacks are ignored (update usages around sendingTimeoutRef, onDone, the chat:done listener, and the local send finally).
🧹 Nitpick comments (1)
app/src/pages/Conversations.tsx (1)
475-767: Consider extracting the send/autocomplete state machine out of this component.These lines now mix send orchestration, timeout ownership, local/cloud branching, and inline-accept keyboard handling in one place. Pulling that into a couple of hooks/modules would make the next reliability changes much safer.
As per coding guidelines, "Keep source files at ≤ ~500 lines per file; split modules when growing larger".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/Conversations.tsx` around lines 475 - 767, The component is doing send/autocomplete orchestration in-here; extract that logic into focused hooks/modules: create a useChatController hook (or similar) to own handleSendMessage, sendingTimeoutRef, isSending state, setSendError, dispatching addMessageLocal/addInferenceResponse, the local-vs-cloud branching (isLocalModelActiveRef, openhumanLocalAiChat, deliverLocalResponse, chatSend) and activeThread management (dispatch(setActiveThread/setActiveThread(null))); create a useVoiceTranscribe hook to own transcribeAndSendAudio and microphone lifecycle (transcribe via openhumanLocalAiTranscribeBytes and call back into useChatController.handleSendMessage); and split inline/autocomplete keyboard handling into a useInlineCompletion hook that exposes getInlineCompletionSuffix, acceptSuggestion, and a handler for handleInputKeyDown to keep UI wiring thin while preserving existing APIs (handleSendMessage, handleVoiceRecordToggle, reply TTS via openhumanLocalAiTts remain callable). Ensure the new hooks accept/return the same event handlers and state setters used here so callers need minimal changes.
🤖 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/pages/Conversations.tsx`:
- Around line 395-400: Move the global send cleanup to run before the
thread-specific guard in the onError handler: always clear sendingTimeoutRef
(clearTimeout + set to null) and reset the send state (e.g., setIsSending(false)
or isSendingRef) first, then check selectedThreadIdRef.current to decide whether
to bail out of showing thread-local error UI; update the onError function so
cleanup of sendingTimeoutRef and isSending happens unconditionally at the top
and only the thread-local UI/return is gated by selectedThreadIdRef.current.
---
Duplicate comments:
In `@app/src/pages/Conversations.tsx`:
- Line 140: The global sendingTimeoutRef causes late chat:done callbacks to
affect newer sends; modify the send flow (the function that sets
sendingTimeoutRef and the onDone handler referenced by chat:done) to create a
per-send identifier or cancelled flag (e.g., a sendId variable captured by the
closure) when starting a send, store that id with the timeout, and have onDone
and the timeout handler check that the callback's sendId matches the current
active send before appending replies or clearing timers; also ensure the
local-model branch's finally block clears the per-send timer and marks the send
as finished/cancelled so normal completion retires the timer and stale callbacks
are ignored (update usages around sendingTimeoutRef, onDone, the chat:done
listener, and the local send finally).
---
Nitpick comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 475-767: The component is doing send/autocomplete orchestration
in-here; extract that logic into focused hooks/modules: create a
useChatController hook (or similar) to own handleSendMessage, sendingTimeoutRef,
isSending state, setSendError, dispatching addMessageLocal/addInferenceResponse,
the local-vs-cloud branching (isLocalModelActiveRef, openhumanLocalAiChat,
deliverLocalResponse, chatSend) and activeThread management
(dispatch(setActiveThread/setActiveThread(null))); create a useVoiceTranscribe
hook to own transcribeAndSendAudio and microphone lifecycle (transcribe via
openhumanLocalAiTranscribeBytes and call back into
useChatController.handleSendMessage); and split inline/autocomplete keyboard
handling into a useInlineCompletion hook that exposes getInlineCompletionSuffix,
acceptSuggestion, and a handler for handleInputKeyDown to keep UI wiring thin
while preserving existing APIs (handleSendMessage, handleVoiceRecordToggle,
reply TTS via openhumanLocalAiTts remain callable). Ensure the new hooks
accept/return the same event handlers and state setters used here so callers
need minimal changes.
🪄 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: 93abf576-1512-4827-afff-ea634604e2ac
📒 Files selected for processing (3)
app/src/pages/Conversations.tsxsrc/openhuman/autocomplete/core.rssrc/openhuman/local_ai/service/bootstrap.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/openhuman/local_ai/service/bootstrap.rs
- src/openhuman/autocomplete/core.rs
| onError: event => { | ||
| if (event.thread_id !== selectedThreadIdRef.current) return; | ||
| if (sendingTimeoutRef.current) { | ||
| clearTimeout(sendingTimeoutRef.current); | ||
| sendingTimeoutRef.current = null; | ||
| } |
There was a problem hiding this comment.
Move the global send cleanup before the selected-thread guard.
If the user switches threads while a request is in flight, Line 396 returns before clearing sendingTimeoutRef or resetting isSending. That leaves the composer locked until the 120s fallback fires. Do the timeout / sending-state cleanup first, then gate only the thread-local error UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/Conversations.tsx` around lines 395 - 400, Move the global send
cleanup to run before the thread-specific guard in the onError handler: always
clear sendingTimeoutRef (clearTimeout + set to null) and reset the send state
(e.g., setIsSending(false) or isSendingRef) first, then check
selectedThreadIdRef.current to decide whether to bail out of showing
thread-local error UI; update the onError function so cleanup of
sendingTimeoutRef and isSending happens unconditionally at the top and only the
thread-local UI/return is gated by selectedThreadIdRef.current.
…ject overview - Added installation commands for MacOS/Linux and Windows users. - Removed redundant download section and streamlined the project description. - Updated the GitHub star section to be commented out for clarity. This update improves user onboarding by providing clear installation steps and a concise project overview.
…ai#107) (tinyhumansai#179) * fix(autocomplete): add 30s timeout to inference HTTP client reqwest::Client::new() creates a client with no timeout, causing inference calls to block indefinitely when Ollama is slow or unreachable. Admin endpoints had explicit timeouts but inference did not. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(autocomplete): improve engine reliability and suppress in-app notifications - Wrap engine loop refresh() in 15s tokio timeout to prevent blocking - Handle AX error -1728 (Electron apps) gracefully as idle, not error - Clear stale state (last_error, suggestion, context) on stop() - Add skip_apply flag to AutocompleteAcceptParams so frontend can accept without triggering AppleScript text insertion - Skip AX-based Tab accept and overlay when OpenHuman is focused - Guard all show_overflow_badge calls with app_name check to suppress Script Editor notifications when typing in the chat composer - Cache inference results: skip Ollama call when context is unchanged and a suggestion already exists Closes tinyhumansai#107 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(autocomplete): add skip_apply to AutocompleteAcceptParams type Frontend passes skip_apply: true when accepting ghost text so the backend only persists for personalization without AppleScript insertion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(autocomplete): fix ghost text display, alignment, and acceptance - Fix getInlineCompletionSuffix to handle backend's continuation suffix format (not just full-string format) - Add 120s safety timeout to clear isSending if no response arrives - Match textarea CSS to overlay div (leading-normal, whitespace-pre-wrap, break-words, font-sans) so ghost text aligns on multi-line wrap - Pass skip_apply: true when accepting to prevent double text insertion - Add Right Arrow key acceptance when cursor is at end of input Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: apply prettier + cargo fmt formatting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * format * Update README.md to enhance installation instructions and clarify project overview - Added installation commands for MacOS/Linux and Windows users. - Removed redundant download section and streamlined the project description. - Updated the GitHub star section to be commented out for clarity. This update improves user onboarding by providing clear installation steps and a concise project overview. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
Summary
Problem
getInlineCompletionSuffixexpected full-string format but backend returns continuation suffix onlyapp_name: Nonebypassing the skip checkleading-normal whitespace-pre-wrapbut textarea did not)reqwest::Clienthad no timeout, causing inference calls to block indefinitelystop()left stale state (last_error, suggestion, context)Solution
Conversations.tsx): Fixed suffix extraction logic, matched textarea/overlay CSS classes, added ArrowRight handler with cursor-at-end check, addedsendingTimeoutRefcleared in all done/error/catch pathstauriCommands.ts): Addedskip_applytoAutocompleteAcceptParamsso accept RPC only persists for personalization without AppleScript insertioncore.rs): Addedis_in_appflag derived fromcontext_override.is_some(), setapp_name: "OpenHuman"in context_override path, guarded all 5show_overflow_badgecall sites with app_name check, added inference cache (skip when context unchanged + suggestion exists), wrapped engine loop refresh in 15s tokio timeout, handle AX -1728 gracefully, clear stale state on stopbootstrap.rs): Added 30s default timeout to reqwest client builderSubmission Checklist
cargo test --test autocomplete_memory_e2e(4 tests pass),cargo checkcleanis_in_appflag, cache check, and notification suppression logicImpact
skip_applyfield is optional with serde defaultRelated
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Performance
Style
Documentation