Merged
Conversation
- Introduced `overlay_ttl_ms` parameter to the Autocomplete configuration, allowing users to set the overlay display duration. - Updated AutocompletePanel to include a new input field for adjusting the overlay TTL in milliseconds. - Enhanced parsing and saving logic to handle the new configuration parameter. - Added corresponding tests to ensure functionality and validate the new overlay TTL feature. This update improves user control over the autocomplete overlay behavior, enhancing the overall user experience.
- Simplified log statements in `precompile_helper_background` for better clarity. - Reformatted `detect_input_monitoring_permission` check in `keys.rs` for enhanced readability. - Rearranged imports in `mod.rs` to maintain consistent structure. - Improved formatting of `ElementBounds` initialization across multiple test cases in `overlay.rs` and `types.rs` for better visual alignment. - Enhanced test context creation in `types.rs` for improved clarity. These changes enhance code maintainability and readability across the accessibility module.
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 28 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughComprehensive system for accessibility overlay management with parent-child core RPC routing, background helper thread refactoring with request/response pairing, macOS input monitoring permission caching, and autocomplete overlay TTL configuration UI. Added tab hint display in overlays, voice status tracking in debug mode, and expanded test coverage across text/overlay/type utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant Overlay as Overlay App
participant TauriLib as Tauri (lib.rs)
participant ParentCore as Parent Core<br/>(Desktop Sidecar)
participant EmbeddedCore as Embedded Core<br/>(if standalone)
Note over Overlay,EmbeddedCore: RPC Routing Decision
Overlay->>TauriLib: invoke("overlay_parent_rpc_url")
TauriLib-->>Overlay: returns parentRpcUrl or None
alt parentRpcUrl is set (child process mode)
Overlay->>Overlay: initialize rpc() helper with parentRpcUrl
Overlay->>ParentCore: fetch("/rpc", {jsonrpc, id, method, params})
ParentCore-->>Overlay: {jsonrpc, id, result/error}
Overlay->>Overlay: validate ID match & error, return result
else parentRpcUrl is None (standalone mode)
Overlay->>TauriLib: invoke("core_rpc", {method, params})
TauriLib->>EmbeddedCore: forward to embedded server
EmbeddedCore-->>TauriLib: {result/error}
TauriLib-->>Overlay: return response
end
Note over Overlay: RPC calls use unified helper for voice/globe/transcription
sequenceDiagram
participant App as AutocompleteEngine
participant Helper as macOS Helper Process
participant Stdout as Stdout Reader<br/>(background thread)
participant RxChannel as RESPONSE_RX<br/>(mpsc channel)
Note over App,RxChannel: Request-Response Pairing Flow
App->>App: increment HELPER_REQ_ID to "123"
App->>App: build JSON request {id: "123", method, params}
App->>Helper: write JSON to stdin (brief lock on UNIFIED_HELPER)
Helper->>Stdout: Helper outputs responses to stdout
Stdout->>Stdout: trim non-empty lines
Stdout->>RxChannel: forward each line as response
par Response Polling
App->>RxChannel: poll with HELPER_RECV_TIMEOUT
RxChannel-->>App: receive response {id: "X", ...}
App->>App: check if id == "123"
alt ID matches
App->>App: return result, clear timeout
else ID mismatch
App->>App: log debug, discard, continue polling
end
end
Note over App: On timeout or quit: drop RESPONSE_RX to signal reader thread exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 6
🧹 Nitpick comments (2)
src/openhuman/accessibility/text_util.rs (1)
120-182: Add boundary tests for thei32overflow guards inparse_ax_number.
parse_ax_numberhas expliciti32::MIN/MAXrange checks, but the new tests don’t currently exercise those branches. Adding just 2–3 cases (just inside/outside bounds) would lock this behavior down and prevent regressions.✅ Suggested test additions
#[test] fn parse_ax_number_trims_surrounding_whitespace() { assert_eq!(parse_ax_number(" 10 "), Some(10)); } + + #[test] + fn parse_ax_number_i32_bounds() { + assert_eq!(parse_ax_number("2147483647"), Some(2147483647)); + assert_eq!(parse_ax_number("-2147483648"), Some(-2147483648)); + } + + #[test] + fn parse_ax_number_out_of_i32_range_returns_none() { + assert_eq!(parse_ax_number("2147483648"), None); + assert_eq!(parse_ax_number("-2147483649"), None); + } }Based on learnings: “Keep functionality validation in Rust tests for
rust-core.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/accessibility/text_util.rs` around lines 120 - 182, Add unit tests around parse_ax_number to exercise the i32 overflow guards: assert that strings for i32::MAX ("2147483647") and i32::MIN ("-2147483648") parse to Some(2147483647) and Some(-2147483648) respectively, and that values just outside the bounds ("2147483648" and "-2147483649") return None; also add one rounding edge case such as "2147483646.6" which should round to i32::MAX (Some(2147483647)) to ensure the rounding+clamp logic in parse_ax_number behaves correctly.src/openhuman/autocomplete/core/text.rs (1)
111-123: Avoid hardcoded max-length literals in tests.Line 112/115/121 duplicate
64instead of deriving fromMAX_SUGGESTION_CHARS, which makes these tests brittle if the constant changes.♻️ Suggested update
#[test] fn sanitize_suggestion_truncates_to_max_chars() { - // MAX_SUGGESTION_CHARS is 64 — a 70-char string should be cut to 64. - let long = "a".repeat(70); + let long = "a".repeat(MAX_SUGGESTION_CHARS + 6); let result = sanitize_suggestion(&long); - assert_eq!(result.len(), 64); + assert_eq!(result.chars().count(), MAX_SUGGESTION_CHARS); assert!(result.chars().all(|c| c == 'a')); } #[test] fn sanitize_suggestion_exactly_max_chars_unchanged() { - let exact = "b".repeat(64); + let exact = "b".repeat(MAX_SUGGESTION_CHARS); assert_eq!(sanitize_suggestion(&exact), exact); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/autocomplete/core/text.rs` around lines 111 - 123, Replace hardcoded 64 literals in the tests with the MAX_SUGGESTION_CHARS constant so they track the implementation; update sanitize_suggestion_truncates_to_max_chars and sanitize_suggestion_exactly_max_chars_unchanged to use MAX_SUGGESTION_CHARS when building the test strings and when asserting lengths/results against sanitize_suggestion, referencing the existing sanitize_suggestion function and MAX_SUGGESTION_CHARS constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/accessibility/helper.rs`:
- Around line 242-256: precompile_helper_background spawns a thread that calls
ensure_helper_binary() concurrently with other code (e.g.,
ensure_helper_running()), causing races/double-builds when both try to write the
same cached source/binary; serialize access by introducing a process-wide guard
(e.g., a static Mutex or Once) and use it around ensure_helper_binary() so all
callers (precompile_helper_background and the first-use compile path invoked
from ensure_helper_running()/first-use code) acquire the same guard before
invoking ensure_helper_binary(); update precompile_helper_background to lock the
guard inside the spawned thread and update the first-use compile path to use the
same guard so only one compilation/writer runs at a time.
- Around line 681-687: The overlay always shows the "Tab ↵" hint even when tab
acceptance is disabled; update the code that creates the NSTextField named hint
so its visibility/text is driven by the Rust-side setting
(config.autocomplete.accept_with_tab) instead of hardcoding. Pass a boolean
(e.g. accept_with_tab) from Rust into the UI creation call (the same place that
calls try_accept_via_tab()), then in the helper set hint.isHidden = true (or
hint.stringValue = "") when accept_with_tab is false and only show the label
when true; reference the NSTextField instance named hint and the
try_accept_via_tab()/config.autocomplete.accept_with_tab symbols to locate where
to wire this flag. Ensure the flag is propagated from Rust into the UI
initializer and used immediately when creating/configuring hint.
- Around line 70-107: The transport sends anonymous requests so responses can be
mis-delivered; modify helper_send_receive to generate a unique request id (e.g.
UUID or incrementing counter) included in the outgoing JSON, and then read from
RESPONSE_RX in a loop until a response with the same id is received (use
HELPER_RECV_TIMEOUT for overall timeout). When a received message has a
non-matching id, stash it in a shared buffer guarded by a mutex (create a
RESPONSE_BUFFER similar to RECV_SERIALISER/RESPONSE_RX) so other callers can
later consume those messages; only return the matching serde_json::Value to the
caller. Keep existing locks (RECV_SERIALISER, UNIFIED_HELPER) semantics but
change the read path to match by id rather than returning the first line.
In `@src/openhuman/accessibility/keys.rs`:
- Around line 15-30: The static Lazy INPUT_MONITORING_GRANTED caches a single
negative result so Tab/Escape detection stays disabled until restart; replace
this with a writable AtomicBool and an accessor that re-checks
detect_input_monitoring_permission() when the flag is false and sets it to true
if PermissionState::Granted (so once granted it stays true), then update callers
that read INPUT_MONITORING_GRANTED to call the new is_input_monitoring_granted()
accessor (which uses detect_input_monitoring_permission and PermissionState)
instead of reading the frozen Lazy value.
In `@src/openhuman/autocomplete/core/engine.rs`:
- Around line 632-635: The confidence is currently hardcoded to 0.0 in
AutocompleteSuggestion (state.suggestion) which makes downstream consumers treat
every suggestion as 0% certain; replace the literal 0.0 with a meaningful
fallback constant (e.g., DEFAULT_FALLBACK_CONFIDENCE = 0.6) and use that when
local_ai::inline_complete cannot provide a real score, update the TODO to
mention using the real score when available, and ensure the constant is a named
f32 so it can be tuned centrally and tested.
In `@src/openhuman/autocomplete/core/overlay.rs`:
- Around line 39-47: The current dedupe logic for LAST_OVERFLOW_BADGE mutates
and suppresses identical signatures indefinitely because it never checks now_ms;
change it to only suppress identical overlay events within a short time window
by comparing now_ms with the stored timestamp before returning. Update the
branch in the block that reads LAST_OVERFLOW_BADGE (where it checks if
*last_signature == signature) to also check that now_ms - stored_timestamp <=
OVERLAY_DEDUP_WINDOW_MS (or a newly defined constant), and if the window has
expired treat the event as new and overwrite the stored tuple with (signature,
now_ms). Ensure you reference LAST_OVERFLOW_BADGE, signature, and now_ms when
making the comparison and updating the guard.
---
Nitpick comments:
In `@src/openhuman/accessibility/text_util.rs`:
- Around line 120-182: Add unit tests around parse_ax_number to exercise the i32
overflow guards: assert that strings for i32::MAX ("2147483647") and i32::MIN
("-2147483648") parse to Some(2147483647) and Some(-2147483648) respectively,
and that values just outside the bounds ("2147483648" and "-2147483649") return
None; also add one rounding edge case such as "2147483646.6" which should round
to i32::MAX (Some(2147483647)) to ensure the rounding+clamp logic in
parse_ax_number behaves correctly.
In `@src/openhuman/autocomplete/core/text.rs`:
- Around line 111-123: Replace hardcoded 64 literals in the tests with the
MAX_SUGGESTION_CHARS constant so they track the implementation; update
sanitize_suggestion_truncates_to_max_chars and
sanitize_suggestion_exactly_max_chars_unchanged to use MAX_SUGGESTION_CHARS when
building the test strings and when asserting lengths/results against
sanitize_suggestion, referencing the existing sanitize_suggestion function and
MAX_SUGGESTION_CHARS constant.
🪄 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: dd164e74-8527-4a75-8179-cd3a6d6e7831
📒 Files selected for processing (15)
app/src/components/settings/panels/AutocompletePanel.tsxapp/src/components/settings/panels/__tests__/AutocompletePanel.test.tsxapp/src/utils/tauriCommands.tssrc/openhuman/accessibility/helper.rssrc/openhuman/accessibility/keys.rssrc/openhuman/accessibility/mod.rssrc/openhuman/accessibility/overlay.rssrc/openhuman/accessibility/paste.rssrc/openhuman/accessibility/text_util.rssrc/openhuman/accessibility/types.rssrc/openhuman/autocomplete/core/engine.rssrc/openhuman/autocomplete/core/overlay.rssrc/openhuman/autocomplete/core/text.rssrc/openhuman/autocomplete/core/types.rssrc/openhuman/config/schema/autocomplete.rs
…ai#342 - Pass OPENHUMAN_OVERLAY_PARENT_RPC_URL from sidecar spawn and strip inherited OPENHUMAN_CORE_PORT so the overlay no longer fights for the parent listen port. - Overlay UI uses HTTP JSON-RPC to the parent sidecar for globe, debug, and voice STT so state matches the main app; add parentCoreRpc helper mirroring legacy method aliases. - Skip embedded JSON-RPC server when parent URL is set; use OPENHUMAN_OVERLAY_EMBEDDED_CORE_PORT (default 7799) for standalone dev. - Fix screen intelligence status method name; add voice_status polling, STT section, collapsible debug summary, and connection banner when core is unreachable. - Voice capture switch gates the mic with real STT availability from voice_status. Closes tinyhumansai#342 Made-with: Cursor
…ai#378) - helper: correlate JSON-RPC replies with monotonic request ids; discard mismatched lines until deadline (fixes stale response after timeout). - helper: serialize Swift compile via HELPER_COMPILE_LOCK (precompile vs first use). - Overlay Tab hint: pass tab_hint from Rust from accept_with_tab; Swift hides hint when empty. - keys: re-check Input Monitoring on an interval when denied so grant without restart works. - engine: use non-zero confidence placeholder (0.75) until inline_complete returns scores. - overlay dedupe: suppress identical badge only within 400ms, not for process lifetime. Co-authored-by: Code review feedback <noreply@github.com> Made-with: Cursor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
overlay/src/App.tsx (1)
590-616:⚠️ Potential issue | 🟠 MajorKeep the stop path reachable while recording.
Right now
voiceBlockedalso disables the mic button whilestatus === "listening", and the early returns at Lines 591-600 run beforestopRecording(). If the user turns voice capture off mid-recording—orvoice_statusflipsstt_availableto false—the overlay can remove its own stop control.🛠 Proposed fix
const handleMainButton = useCallback(() => { + if (status === "listening") { + logOverlay("main button toggled to stop listening"); + stopRecording(); + return; + } + if (!voiceCaptureEnabled) { setMessage("Turn on voice capture to use the microphone"); return; } if (debugSnapshot.voice && !debugSnapshot.voice.stt_available) { setMessage( "Speech-to-text is not available. Configure Local AI / voice in the main OpenHuman app.", ); return; } - - if (status === "listening") { - logOverlay("main button toggled to stop listening"); - stopRecording(); - return; - } logOverlay("main button toggled to start listening", { priorStatus: status }); void startRecording(); }, [ debugSnapshot.voice, startRecording, status, stopRecording, voiceCaptureEnabled, ]); @@ - disabled={waitingForCoreConfig || voiceBlocked} + disabled={waitingForCoreConfig || (status !== "listening" && voiceBlocked)}Also applies to: 729-730
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@overlay/src/App.tsx` around lines 590 - 616, The main issue is that early returns for voiceCaptureEnabled and debugSnapshot.voice.stt_available run before the stop path, which can prevent stopRecording() from being called while status === "listening"; to fix, reorder the checks inside handleMainButton so you first check if status === "listening" and call stopRecording() (and return) before any checks that block the UI (voiceCaptureEnabled or stt availability), ensuring the stop path is always reachable; apply the same reorder/fix to the other equivalent button handler that performs similar voice/voiceBlocked checks so it also always allows stopRecording() when status === "listening".src/core/jsonrpc.rs (1)
617-628:⚠️ Potential issue | 🟠 MajorBuild the overlay URL from the bound listener, not the requested host/port.
format!("http://{host}:{port}/rpc")bakes the bind address into a client URL. That breaks valid configs like IPv6 (needs brackets) and ephemeral port0, and it also propagates wildcard binds (0.0.0.0/::) to the child overlay even though those are bind targets, not a concrete loopback destination. In those cases the core can listen successfully while sidecar-mode overlay RPC points at the wrong endpoint.🛠 Proposed fix
- let parent_core_rpc_url = format!("http://{host}:{port}/rpc"); + let local_addr = listener.local_addr()?; + let parent_addr = match local_addr { + std::net::SocketAddr::V4(addr) if addr.ip().is_unspecified() => { + std::net::SocketAddr::from((std::net::Ipv4Addr::LOCALHOST, addr.port())) + } + std::net::SocketAddr::V6(addr) if addr.ip().is_unspecified() => { + std::net::SocketAddr::from((std::net::Ipv6Addr::LOCALHOST, addr.port())) + } + addr => addr, + }; + let parent_core_rpc_url = format!("http://{parent_addr}/rpc");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/jsonrpc.rs` around lines 617 - 628, The code currently constructs parent_core_rpc_url from the requested host/port (format!("http://{host}:{port}/rpc")), which can be incorrect for IPv6, ephemeral port 0, or wildcard binds; instead obtain the actual bound socket address from the listener and build the URL from that. Change the code so the closure uses the listener's local_addr() (SocketAddr) to derive host and port, replace unspecified addresses (is_unspecified) with the appropriate loopback (127.0.0.1 or ::1), and format the URL with IPv6 bracketed notation when needed; then pass that computed parent_core_rpc_url into spawn_overlay and other callers (refer to parent_core_rpc_url, spawn_overlay, the tokio::spawn async block, and the listener.local_addr() call) so the overlay connects to the real bound endpoint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@overlay/src-tauri/src/lib.rs`:
- Around line 141-150: The embedded core path currently calls
openhuman_core::core::jsonrpc::run_server(None, Some(port), true) but the
jsonrpc server still auto-spawns the overlay; update the run_server API to
accept an explicit embedded_core (bool) flag and propagate it to the server
logic, then modify the auto-spawn block in src/core/jsonrpc.rs (the code around
lines that currently spawn the overlay when config.overlay_enabled) to skip
spawning when embedded_core is true; finally change this call to pass the
embedded_core=true flag so the embedded server does not auto-launch a second
overlay.
In `@overlay/src/App.tsx`:
- Around line 250-255: The useEffect in App.tsx that calls
invoke("overlay_parent_rpc_url") lacks rejection handling and a cleanup, so if
invoke rejects parentRpcUrl stays undefined; update the effect around
invoke<string | null>("overlay_parent_rpc_url") to add a .catch(() =>
setParentRpcUrl(null)) (or equivalent) and implement a cleanup guard (e.g., an
isMounted flag or AbortController) before calling setParentRpcUrl to avoid state
updates after unmount; ensure you still trim and set null for empty strings and
reference the setParentRpcUrl setter and the invoke call so the behavior is
identical on success or failure.
In `@overlay/src/parentCoreRpc.ts`:
- Around line 42-76: callParentCoreRpc can hang because fetch has no timeout;
wrap the fetch in an AbortController with a configurable timeout (e.g., default
10s) so stalled requests are aborted and the caller receives an error. In
callParentCoreRpc, create an AbortController, pass controller.signal into fetch,
start a timer that calls controller.abort() after the timeout, and clear the
timer after fetch completes (both on success and error) to avoid leaks; surface
a clear error when aborted. Keep the rest of the logic (normalizeLegacyMethod,
payload creation, response handling, unwrapCliCompatibleJson) unchanged.
---
Outside diff comments:
In `@overlay/src/App.tsx`:
- Around line 590-616: The main issue is that early returns for
voiceCaptureEnabled and debugSnapshot.voice.stt_available run before the stop
path, which can prevent stopRecording() from being called while status ===
"listening"; to fix, reorder the checks inside handleMainButton so you first
check if status === "listening" and call stopRecording() (and return) before any
checks that block the UI (voiceCaptureEnabled or stt availability), ensuring the
stop path is always reachable; apply the same reorder/fix to the other
equivalent button handler that performs similar voice/voiceBlocked checks so it
also always allows stopRecording() when status === "listening".
In `@src/core/jsonrpc.rs`:
- Around line 617-628: The code currently constructs parent_core_rpc_url from
the requested host/port (format!("http://{host}:{port}/rpc")), which can be
incorrect for IPv6, ephemeral port 0, or wildcard binds; instead obtain the
actual bound socket address from the listener and build the URL from that.
Change the code so the closure uses the listener's local_addr() (SocketAddr) to
derive host and port, replace unspecified addresses (is_unspecified) with the
appropriate loopback (127.0.0.1 or ::1), and format the URL with IPv6 bracketed
notation when needed; then pass that computed parent_core_rpc_url into
spawn_overlay and other callers (refer to parent_core_rpc_url, spawn_overlay,
the tokio::spawn async block, and the listener.local_addr() call) so the overlay
connects to the real bound endpoint.
🪄 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: c98a47a8-ecbe-43ce-afac-16caa6266a12
📒 Files selected for processing (6)
overlay/src-tauri/src/lib.rsoverlay/src-tauri/tauri.conf.jsonoverlay/src/App.tsxoverlay/src/parentCoreRpc.tssrc/core/jsonrpc.rssrc/openhuman/overlay/process.rs
✅ Files skipped from review due to trivial changes (1)
- overlay/src-tauri/tauri.conf.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/autocomplete/core/engine.rs (1)
110-121:⚠️ Potential issue | 🟡 MinorGate helper warm-up behind the
enabledcheck.Right now
precompile_helper_background()runs beforeConfig::load_or_init()and the guard at Lines 120-121, so the firststart()still kicks off a Swift compile even when autocomplete is disabled or config loading fails. That makes the opt-out/error path do background work every launch.Suggested change
- // Kick off Swift helper compilation in the background so the first - // suggestion request does not stall waiting for `swiftc`. - static PRECOMPILE_ONCE: Once = Once::new(); - PRECOMPILE_ONCE.call_once(|| { - crate::openhuman::accessibility::precompile_helper_background(); - }); - let config = Config::load_or_init() .await .map_err(|e| format!("failed to load config: {e}"))?; if !config.autocomplete.enabled { return Ok(AutocompleteStartResult { started: false }); } + + // Kick off Swift helper compilation in the background so the first + // suggestion request does not stall waiting for `swiftc`. + static PRECOMPILE_ONCE: Once = Once::new(); + PRECOMPILE_ONCE.call_once(|| { + crate::openhuman::accessibility::precompile_helper_background(); + });🤖 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 110 - 121, The PRECOMPILE_ONCE call that invokes crate::openhuman::accessibility::precompile_helper_background() must be moved so it runs only after Config::load_or_init() succeeds and after checking config.autocomplete.enabled; update engine.rs to call PRECOMPILE_ONCE.call_once(...) only if the loaded Config indicates autocomplete.enabled is true and after mapping errors from Config::load_or_init(), ensuring no background precompile is started when config loading fails or autocomplete is disabled (refer to PRECOMPILE_ONCE, precompile_helper_background(), Config::load_or_init(), and the autocomplete.enabled check/AutocompleteStartResult).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/accessibility/helper.rs`:
- Around line 128-137: The recv_timeout call on RESPONSE_RX is currently
propagated with ? which makes any 500ms Timeout abort the whole helper call;
change the logic in the loop around RESPONSE_RX.recv_timeout(chunk) (the block
producing response_line) to match on the recv result instead of using ?,
treating RecvTimeoutError::Timeout as a non-fatal event that decrements
remaining and continues the outer loop until HELPER_RECV_TIMEOUT is exhausted,
while still returning an error for RecvTimeoutError::Disconnected (or channel
closed). Keep references to RESPONSE_RX, recv_timeout, remaining, chunk and
HELPER_RECV_TIMEOUT so the change is applied in the same receive loop context.
---
Outside diff comments:
In `@src/openhuman/autocomplete/core/engine.rs`:
- Around line 110-121: The PRECOMPILE_ONCE call that invokes
crate::openhuman::accessibility::precompile_helper_background() must be moved so
it runs only after Config::load_or_init() succeeds and after checking
config.autocomplete.enabled; update engine.rs to call
PRECOMPILE_ONCE.call_once(...) only if the loaded Config indicates
autocomplete.enabled is true and after mapping errors from
Config::load_or_init(), ensuring no background precompile is started when config
loading fails or autocomplete is disabled (refer to PRECOMPILE_ONCE,
precompile_helper_background(), Config::load_or_init(), and the
autocomplete.enabled check/AutocompleteStartResult).
🪄 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: e10d109a-8e0e-484f-b333-b43de45903b3
📒 Files selected for processing (5)
src/openhuman/accessibility/helper.rssrc/openhuman/accessibility/keys.rssrc/openhuman/accessibility/overlay.rssrc/openhuman/autocomplete/core/engine.rssrc/openhuman/autocomplete/core/overlay.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/openhuman/accessibility/keys.rs
- src/openhuman/accessibility/overlay.rs
- src/openhuman/autocomplete/core/overlay.rs
- Use match on anchor_bounds in autocomplete overlay (avoid unnecessary unwrap) - Drop unused test imports in registry_ops and rpc dispatch - Prefix unused notion_doc_id in subconscious integration test Made-with: Cursor
- Updated the useEffect hook to manage the parent RPC URL more robustly by introducing a mounted flag to prevent state updates on unmounted components. - Added error handling to set the parent RPC URL to null in case of invocation failure, enhancing the reliability of the component's behavior.
- Introduced a default timeout for parent core RPC requests, enhancing reliability by preventing indefinite waiting for responses. - Added an AbortController to manage request timeouts, throwing a specific error message when a timeout occurs. - Updated the `callParentCoreRpc` function to accept a customizable timeout parameter, improving flexibility for RPC calls.
- Updated the main button handler in the App component to always permit stopping an active recording when the status is "listening", improving user experience and control over the recording process. - Removed redundant code that previously checked the status before stopping the recording, streamlining the logic.
- Added new packages including `alsa`, `alsa-sys`, `arboard`, `block`, `cocoa`, `core-foundation`, `core-graphics`, `coreaudio-rs`, `coreaudio-sys`, `cpal`, `crunchy`, and `dasp_sample` to enhance functionality and support for audio processing and system interactions. - Updated existing dependencies to their latest versions for improved performance and compatibility. - Modified the `show_overlay` function in `ops.rs` to include an additional parameter, enhancing the overlay display functionality.
…rfaces - Introduced a new optional parameter `overlay_ttl_ms` to both `AutocompleteSetStyleParams` and `AutocompleteConfig` interfaces, allowing for customizable overlay timeout settings. - This enhancement improves the flexibility of the autocomplete feature by enabling developers to specify how long the overlay should remain visible.
This was referenced Apr 7, 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
2 participants
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
Problem
Solution
Submission Checklist
app/) and/orcargo test(core) for logic you add or changeapp/test/e2e, mock backend,tests/json_rpc_e2e.rsas appropriate)//////!(Rust), JSDoc or brief file/module headers (TS) on public APIs and non-obvious modules(Any feature related checklist can go in here)
Impact
Related
Summary by CodeRabbit
Release Notes
New Features
Improvements