Skip to content

feat(core): desktop companion domain — session, pipeline, pointing (#1909)#2025

Merged
senamakel merged 13 commits into
tinyhumansai:mainfrom
yuvrxj-afk:feat/desktop-companion-core
May 19, 2026
Merged

feat(core): desktop companion domain — session, pipeline, pointing (#1909)#2025
senamakel merged 13 commits into
tinyhumansai:mainfrom
yuvrxj-afk:feat/desktop-companion-core

Conversation

@yuvrxj-afk
Copy link
Copy Markdown
Contributor

@yuvrxj-afk yuvrxj-afk commented May 18, 2026

Summary

New desktop_companion Rust domain for #1909 — Clicky-style interaction loop reusing existing voice, meet_agent, screen_intelligence, and provider_surfaces modules.

  • Session lifecycle with consent gate, TTL, state machine, conversation history
  • Pipeline: STT -> LLM -> TTS with cancellation support
  • [POINT:x,y:label:screenN] parser + multi-monitor coordinate mapping
  • Provider-surface handoff
  • 5 RPC controllers, 3 domain events, broadcast bus
  • JSON-RPC E2E tests
  • Architecture doc + capability catalog

Test plan

  • 66 tests (65 unit + 1 JSON-RPC E2E)
  • cargo check + cargo fmt clean

PR 2 will add Tauri bridge + React UI.

Closes #1909 (partially)

Summary by CodeRabbit

  • New Features

    • Desktop Companion: consented session lifecycle with TTL, single-session state machine, hotkey/audio/text turns, cancellation and recoveries
    • On-screen visual pointing via embedded POINT tags with coordinate mapping and overlay
    • STT input, TTS output, and provider-surface handoff detection
    • JSON-RPC controls/schemas for start/stop/status/config
  • Documentation

    • New Desktop Companion architecture guide
  • Tests

    • Unit, integration, and e2e coverage for pipeline, session, pointing, handoff, and RPC flows

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 18, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new "Desktop Companion" domain: serializable types and RPC contracts, a process-global session lifecycle with TTL/consent and events, turn orchestration (text/audio → STT/LLM → POINT parsing → TTS/pointing), provider handoff detection, JSON-RPC schemas/handlers, module wiring, tests, and architecture documentation.

Changes

Desktop Companion

Layer / File(s) Summary
Data model & RPC contracts
src/openhuman/desktop_companion/types.rs
CompanionState, ConversationTurn, CompanionConfig, RPC params/results, CompanionSessionStatus, and CompanionStateChangedEvent are defined and serde-enabled.
Session lifecycle & state machine
src/openhuman/desktop_companion/session.rs, src/openhuman/desktop_companion/session_tests.rs
Process-global singleton session with start/stop (consent + TTL), stale-session auto-expiry, validated transitions (allowlist + Error recovery), conversation history capping, event publishing, reset_for_test, and comprehensive state/history tests.
Pointing parser & provider handoff
src/openhuman/desktop_companion/pointing.rs, src/openhuman/desktop_companion/handoff.rs, src/openhuman/desktop_companion/pointing_tests.rs
POINT tag extraction and map-to-absolute multi-monitor coordinates (clamping/fallback), plus check_handoff that token/substring-matches provider keywords and emits HandoffEvent(s); both include unit tests.
Turn orchestration pipeline
src/openhuman/desktop_companion/pipeline.rs, src/openhuman/desktop_companion/pipeline_tests.rs
Exports TurnResult and async run_text_turn/run_audio_turn: validation, cancellation checks, session state transitions, rolling history, optional screen context, backend LLM call, POINT parsing/stripping, handoff detection, history persistence, conditional TTS, adapters (cloud STT/TTS), helpers, and tests.
JSON-RPC schemas & handlers
src/openhuman/desktop_companion/schemas.rs, src/core/all.rs
Controller schema enumerator and RegisteredController entries for start_session/stop_session/status/config_get/config_set with handler impls parsing params and invoking session APIs; integrates schemas/controllers into global registry and namespace description.
Event bus & DomainEvent routing
src/openhuman/desktop_companion/bus.rs, src/core/event_bus/events.rs
Process-global Tokio broadcast channel for CompanionStateChangedEvent with subscribe_state_changed/publish_state_changed APIs and tests; DomainEvent gains companion-specific variants and maps them to "companion".
Module wiring & capability catalog
src/openhuman/desktop_companion/mod.rs, src/openhuman/mod.rs, src/openhuman/about_app/catalog.rs
New desktop_companion module exposing submodules and re-exporting schema helpers; public openhuman::desktop_companion export and capability catalog entries for companion.session and companion.pointing.
Unit, integration, and RPC e2e tests
tests/json_rpc_e2e.rs, src/openhuman/desktop_companion/*_tests.rs
Module-level unit tests (bus/handoff/pointing/session/pipeline), integration-style pipeline tests, and an RPC e2e test exercising the companion session lifecycle over /rpc.
Architecture documentation
gitbooks/developing/architecture/desktop-companion.md
New GitBook page documenting the Desktop Companion interaction loop, modules, state machine, RPC/event interfaces, pointing/handoff behavior, platform scope, and test plans.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CompanionSession
  participant ScreenService
  participant LLMBackend
  participant ProviderQueue
  participant TTSService

  User->>CompanionSession: invoke (hotkey / audio)
  CompanionSession->>ScreenService: gather foreground/screen context
  CompanionSession->>LLMBackend: POST chat/completions (system + history + prompt)
  LLMBackend-->>CompanionSession: assistant text (may include [POINT:...] tags)
  CompanionSession->>ProviderQueue: check_handoff(cleaned_text)
  ProviderQueue-->>CompanionSession: matching queue items (HandoffEvent)
  CompanionSession->>TTSService: synthesize(cleaned_text) [optional]
  CompanionSession-->>User: speak / point / return TurnResult
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

"A rabbit whispered from the key, I hopped to wire the loop,
I stitched consent and session thread and taught the bot to stoop.
It listens, maps a POINT, and speaks with gentle art,
Hands off to providers when the reply should chart.
Hop, click, and whisper — companion work is smart."

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding a new desktop companion domain with session, pipeline, and pointing functionality.
Linked Issues check ✅ Passed All core coding requirements from #1909 are met: session lifecycle with consent and TTL [#1909], LLM pipeline with STT/TTS [#1909], POINT tag parsing and multi-monitor mapping [#1909], provider handoff [#1909], state machine [#1909], RPC controllers [#1909], and comprehensive test coverage including E2E [#1909].
Out of Scope Changes check ✅ Passed All changes are scoped to the desktop companion domain implementation as specified in #1909; no unrelated modifications to other systems were introduced.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yuvrxj-afk yuvrxj-afk changed the title WIP: feat(core): desktop companion domain — session, pipeline, pointing (#1909) feat(core): desktop companion domain — session, pipeline, pointing (#1909) May 18, 2026
@yuvrxj-afk yuvrxj-afk marked this pull request as ready for review May 18, 2026 02:45
@yuvrxj-afk yuvrxj-afk requested a review from a team May 18, 2026 02:45
@senamakel
Copy link
Copy Markdown
Member

fuck yeah.... great PR can't wait to merge this

@coderabbitai coderabbitai Bot added the working A PR that is being worked on by the team. label May 18, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Nitpick comments (1)
tests/json_rpc_e2e.rs (1)

5981-5988: ⚡ Quick win

Add companion_config_set round-trip coverage in this lifecycle E2E.

This test validates start_session, stop_session, status, and config_get, but not openhuman.companion_config_set. Please add a config_set call plus a follow-up config_get assertion to lock the full UI-facing RPC contract in one place.

As per coding guidelines: "Extend tests/json_rpc_e2e.rs and scripts/test-rust-with-mock.sh for new RPC methods to verify they match what the UI will call before surfacing in the frontend."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/json_rpc_e2e.rs` around lines 5981 - 5988, Add a round-trip test for
the new RPC by calling post_json_rpc(&rpc_base, <new_id>,
"openhuman.companion_config_set", json!({...})) (use the next sequential request
id after the existing 106) and assert success with assert_no_jsonrpc_error (same
pattern as existing calls), then immediately call post_json_rpc(&rpc_base,
<next_id>, "openhuman.companion_config_get", json!({})) and use
assert_no_jsonrpc_error to fetch the result and assert the changed field (e.g.,
compare the "hotkey" or other config key) is present/updated in the returned
config_body; follow the existing variables and helpers (rpc_base, post_json_rpc,
assert_no_jsonrpc_error, config_body) and mirror the style of the surrounding
test block to keep ordering and IDs consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gitbooks/developing/architecture/desktop-companion.md`:
- Line 58: Update the sentence "Activation — state transitions to Listening
(driven by Tauri hotkey bridge)" to make the Tauri bridge status explicit as not
yet implemented (e.g., "Activation — state transitions to Listening (triggered
by a future Tauri hotkey bridge)" or "Activation — state transitions to
Listening (platform trigger/entrypoint)"). Edit the phrase so it no longer
implies the bridge exists now; reference the original string "Activation — state
transitions to Listening (driven by Tauri hotkey bridge)" when locating and
replacing the text.
- Around line 23-33: The fenced code blocks in
gitbooks/developing/architecture/desktop-companion.md are missing language
identifiers (causing MD040); update both blocks that show the file tree snippet
(the block starting with "src/openhuman/desktop_companion/") and the state
diagram block (the block starting with "Idle -> Listening...") to include a
language tag (e.g., change ``` to ```text) so markdownlint treats them as code
blocks; ensure you also update the other occurrence mentioned (lines 37-44)
similarly.

In `@src/core/all.rs`:
- Around line 238-241: The companion namespace was registered via
crate::openhuman::desktop_companion::all_desktop_companion_registered_controllers()
(seen in the controllers.extend call) but no entry was added to the
namespace_description lookup; add a description string for the "companion"
namespace into the same structure/collection where other namespace_description
entries are defined (ensure the key is "companion" and the value is a concise
help/CLI description), and mirror the same change for the other occurrence
referenced (around the second controllers.extend
call/all_desktop_companion_registered_controllers invocation) so help/CLI
lookups return the new description.

In `@src/openhuman/desktop_companion/bus.rs`:
- Around line 49-62: The test publish_is_received_by_subscriber is flaky because
STATE_BUS is global and rx.recv() may return events from other tests; update the
test to keep calling rx.recv().await until you get the
CompanionStateChangedEvent with session_id "test-session" (or otherwise a unique
marker), i.e., after calling publish_state_changed(...) loop over received
events from subscribe_state_changed() and assert on the event whose session_id
matches the one you published (refer to publish_state_changed,
subscribe_state_changed, CompanionStateChangedEvent, and STATE_BUS).

In `@src/openhuman/desktop_companion/handoff.rs`:
- Around line 61-87: The loop over PROVIDER_KEYWORDS emits
duplicate/false-positive HandoffEvent entries due to substring matching and
multiple keywords per provider; update the check to use token-aware matching
(e.g., split or regex word-boundary match against `lower` rather than
`lower.contains(keyword)`) and also deduplicate by provider before pushing (skip
if an existing `HandoffEvent` with the same `provider` is already in `events`).
Adjust the provider matching/filtering for `RespondQueueItem` (the `matching`
Vec built from `queue_items` using `item.provider.to_lowercase() ==
provider_id`) to use the same normalized form used for dedupe, and only push one
`HandoffEvent` per provider with the aggregated `matching_items` and
`response_text`.

In `@src/openhuman/desktop_companion/pipeline.rs`:
- Around line 71-73: The early cancellation checks that return
cancelled_result(trimmed) after transitioning the session to Thinking leave the
session stuck in Thinking; before each early return (the if
cancel.is_cancelled() branches that call cancelled_result(trimmed) at the sites
shown) explicitly transition the session back to Idle (e.g., call the
session/state transition method used elsewhere to set SessionState::Idle or
invoke the same helper used for normal completion) so the session state is
restored before returning; apply this change to all occurrences (the branches at
the shown diff and the similar blocks around the other noted locations).
- Around line 193-214: run_audio_turn currently calls stt(...) even when the
session has been cancelled, wasting work and delaying interruption; add an early
cancellation check immediately before invoking stt: inspect the session
cancellation/state (e.g., check if the session is in a cancelled state or a
cancellation flag exposed by the session API) and if cancelled, call
session::transition_state(CompanionState::Idle or CompanionState::Error as
appropriate), and return a TurnResult with cancelled: true (and empty
transcript/response/targets/ttt_synthesized/handoff_events) instead of calling
stt; ensure you update the same code path that currently calls stt(...) so the
stt function is only invoked when not cancelled.

In `@src/openhuman/desktop_companion/schemas.rs`:
- Around line 147-183: Handlers handle_start_session, handle_stop_session,
handle_status, handle_config_get and handle_config_set lack RPC entry/exit and
error-path diagnostics; update each to emit tracing/log (use tracing::debug! or
trace!) at function entry with relevant request params (e.g., serialize or
summarize parsed
StartCompanionSessionParams/StopCompanionSessionParams/EmptyRequest), log
successful exit with the result value, and log errors with the error details
before returning (include the error string from parse_params, session::*,
serde_json::to_value, or the custom Err in handle_config_set) so failures can be
triaged; keep logs concise and contextual (function name + params +
error/result).

In `@src/openhuman/desktop_companion/session.rs`:
- Around line 197-207: The error returns for missing session and invalid state
transitions lack logs; before each early return add a tracing/log statement
(using log::debug or tracing::debug) that includes context: when
guard.as_mut().ok_or("no active companion session") would return Err, log that
there is "no active companion session" along with any identifying
session/request id available and relevant vars; similarly, before the
is_valid_transition check returns Err, emit a debug/trace that records the
attempted transition and values (previous and new_state) and any session id or
guard metadata; update the code around the use of guard, inner.state, and the
is_valid_transition call to include these debug logs immediately prior to
returning the formatted Err.
- Around line 47-50: ACTIVE_SESSION being checked in start_session() rejects any
Some() value even if that session has expired, because TTL cleanup currently
only runs inside session_status(); modify start_session() to perform the same
expiration check/cleanup as session_status() before returning an error.
Specifically, when holding the ACTIVE_SESSION mutex in start_session(), inspect
the contained session's expiry/ttl (use the same expiry logic used in
session_status() around lines 145-150 or extract that logic to a helper like
cleanup_expired_session()), and if the session is expired, clear ACTIVE_SESSION
(set to None) and allow start_session() to proceed; otherwise keep the existing
error path. Ensure the same change (or extracted helper) is used to avoid
duplicated logic.
- Line 57: The current expression Some(now_ms + (ttl_secs as i64 * 1000)) can
overflow/truncate when casting ttl_secs (u64) to i64; replace the direct cast
with checked arithmetic: compute ttl_ms = ttl_secs.checked_mul(1000), convert
ttl_ms to i64 safely (e.g., i64::try_from(ttl_ms).ok()), then use
now_ms.checked_add(ms_i64) and return Some(result) only if all checks succeed,
otherwise return None or saturate—apply this change around the expires_at_ms
computation that uses now_ms and ttl_secs.

---

Nitpick comments:
In `@tests/json_rpc_e2e.rs`:
- Around line 5981-5988: Add a round-trip test for the new RPC by calling
post_json_rpc(&rpc_base, <new_id>, "openhuman.companion_config_set",
json!({...})) (use the next sequential request id after the existing 106) and
assert success with assert_no_jsonrpc_error (same pattern as existing calls),
then immediately call post_json_rpc(&rpc_base, <next_id>,
"openhuman.companion_config_get", json!({})) and use assert_no_jsonrpc_error to
fetch the result and assert the changed field (e.g., compare the "hotkey" or
other config key) is present/updated in the returned config_body; follow the
existing variables and helpers (rpc_base, post_json_rpc,
assert_no_jsonrpc_error, config_body) and mirror the style of the surrounding
test block to keep ordering and IDs consistent.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f4588f13-f7b4-4e82-b8d8-073c4d165c0e

📥 Commits

Reviewing files that changed from the base of the PR and between db99318 and 45e6d8a.

📒 Files selected for processing (17)
  • gitbooks/developing/architecture/desktop-companion.md
  • src/core/all.rs
  • src/core/event_bus/events.rs
  • src/openhuman/about_app/catalog.rs
  • src/openhuman/desktop_companion/bus.rs
  • src/openhuman/desktop_companion/handoff.rs
  • src/openhuman/desktop_companion/mod.rs
  • src/openhuman/desktop_companion/pipeline.rs
  • src/openhuman/desktop_companion/pipeline_tests.rs
  • src/openhuman/desktop_companion/pointing.rs
  • src/openhuman/desktop_companion/pointing_tests.rs
  • src/openhuman/desktop_companion/schemas.rs
  • src/openhuman/desktop_companion/session.rs
  • src/openhuman/desktop_companion/session_tests.rs
  • src/openhuman/desktop_companion/types.rs
  • src/openhuman/mod.rs
  • tests/json_rpc_e2e.rs

Comment thread gitbooks/developing/architecture/desktop-companion.md Outdated
Comment thread gitbooks/developing/architecture/desktop-companion.md Outdated
Comment thread src/core/all.rs
Comment thread src/openhuman/desktop_companion/bus.rs
Comment thread src/openhuman/desktop_companion/handoff.rs
Comment thread src/openhuman/desktop_companion/pipeline.rs
Comment thread src/openhuman/desktop_companion/schemas.rs
Comment thread src/openhuman/desktop_companion/session.rs
Comment thread src/openhuman/desktop_companion/session.rs Outdated
Comment thread src/openhuman/desktop_companion/session.rs Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
gitbooks/developing/architecture/desktop-companion.md (1)

23-23: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add language identifiers to fenced code blocks.

The code fences at lines 23 and 37 are missing language identifiers (e.g., ```text), which triggers markdownlint MD040 warnings.

📝 Proposed fix
-```
+```text
 src/openhuman/desktop_companion/

And:

-```
+```text
 Idle -> Listening -> Thinking -> Speaking -> Pointing -> Idle

Also applies to: 37-37

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gitbooks/developing/architecture/desktop-companion.md` at line 23, The two
fenced code blocks showing the directory path "src/openhuman/desktop_companion/"
and the state sequence "Idle -> Listening -> Thinking -> Speaking -> Pointing ->
Idle" are missing language identifiers; update each opening fence to include a
language (e.g., change ``` to ```text) so the blocks become ```text and
eliminate the markdownlint MD040 warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gitbooks/developing/architecture/desktop-companion.md`:
- Line 61: Update the inconsistent conversation history cap between the two
mentions by verifying the actual implementation of the rolling conversation
history and then setting both occurrences in the document to the real limit;
specifically check the LLM usage via BackendOAuthClient (system prompt, screen
context, rolling conversation history) and change the "capped at 20 turns" and
"capped at 50 turns" references so they match the implemented cap (use the
implemented value).

---

Duplicate comments:
In `@gitbooks/developing/architecture/desktop-companion.md`:
- Line 23: The two fenced code blocks showing the directory path
"src/openhuman/desktop_companion/" and the state sequence "Idle -> Listening ->
Thinking -> Speaking -> Pointing -> Idle" are missing language identifiers;
update each opening fence to include a language (e.g., change ``` to ```text) so
the blocks become ```text and eliminate the markdownlint MD040 warnings.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0ee4956a-5454-4bc6-976f-845100801b86

📥 Commits

Reviewing files that changed from the base of the PR and between 45e6d8a and ce14d7c.

📒 Files selected for processing (8)
  • gitbooks/developing/architecture/desktop-companion.md
  • src/core/all.rs
  • src/openhuman/desktop_companion/bus.rs
  • src/openhuman/desktop_companion/handoff.rs
  • src/openhuman/desktop_companion/pipeline.rs
  • src/openhuman/desktop_companion/schemas.rs
  • src/openhuman/desktop_companion/session.rs
  • tests/json_rpc_e2e.rs
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/openhuman/desktop_companion/bus.rs
  • src/core/all.rs
  • tests/json_rpc_e2e.rs
  • src/openhuman/desktop_companion/schemas.rs
  • src/openhuman/desktop_companion/pipeline.rs
  • src/openhuman/desktop_companion/handoff.rs
  • src/openhuman/desktop_companion/session.rs

Comment thread gitbooks/developing/architecture/desktop-companion.md Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/openhuman/desktop_companion/handoff.rs (2)

52-57: ⚡ Quick win

Tests currently can’t validate the positive handoff path.

check_handoff reads from global store directly, so current tests only hit empty-queue behavior. Please extract a pure helper that accepts queue items so tests can assert real emission/dedupe behavior.

Proposed refactor
 pub fn check_handoff(response_text: &str) -> Vec<HandoffEvent> {
-    if response_text.is_empty() {
-        return Vec::new();
-    }
-
-    let lower = response_text.to_lowercase();
     let queue_items = store::list_queue_items();
+    check_handoff_with_items(response_text, &queue_items)
+}
+
+fn check_handoff_with_items(response_text: &str, queue_items: &[RespondQueueItem]) -> Vec<HandoffEvent> {
+    if response_text.is_empty() {
+        return Vec::new();
+    }
+    let lower = response_text.to_lowercase();
 
     if queue_items.is_empty() {
         debug!("{LOG_PREFIX} no items in provider queue, skipping handoff check");
         return Vec::new();
     }
@@
-        let matching: Vec<RespondQueueItem> = queue_items
+        let matching: Vec<RespondQueueItem> = queue_items
             .iter()
             .filter(|item| item.provider.to_lowercase() == provider_id)
             .cloned()
             .collect();

Also applies to: 130-135

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/desktop_companion/handoff.rs` around lines 52 - 57, The
check_handoff function currently reads store::list_queue_items() directly which
prevents tests from exercising the positive path; extract a pure helper (e.g.,
check_handoff_items or emit_handoff_events) that takes the queue items (Vec or
slice of QueueItem) and returns the Vec of emitted handoff events (performing
emission/dedupe logic currently in check_handoff), then have check_handoff
simply call store::list_queue_items() and forward that result into the new
helper; also apply the same extraction for the duplicate logic referenced around
the other occurrence (lines ~130-135) so tests can invoke the helper with
crafted queue items to assert emission and dedup behavior.

148-152: ⚡ Quick win

This test bypasses the production matching logic.

provider_keywords_case_insensitive_match only checks lower.contains(kw), which can pass even if check_handoff token/phrase behavior regresses. Please route this test through the same matcher helper used by production.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/desktop_companion/handoff.rs` around lines 148 - 152, The test
provider_keywords_case_insensitive_match currently asserts by doing
lower.contains(kw) and therefore bypasses production logic; update the test to
call the actual production matcher (check_handoff) instead of doing a simple
substring check so it uses the same token/phrase behavior and normalization as
production. Specifically, iterate PROVIDER_KEYWORDS and for each kw build a
representative input (e.g., "Check your {kw} messages" or the original text
variable), then call check_handoff(text) (or the module's handoff-matching
helper used in production) and assert that it returns the expected match for
that provider; keep the test name and use PROVIDER_KEYWORDS as the source of
keywords.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/openhuman/desktop_companion/handoff.rs`:
- Around line 52-57: The check_handoff function currently reads
store::list_queue_items() directly which prevents tests from exercising the
positive path; extract a pure helper (e.g., check_handoff_items or
emit_handoff_events) that takes the queue items (Vec or slice of QueueItem) and
returns the Vec of emitted handoff events (performing emission/dedupe logic
currently in check_handoff), then have check_handoff simply call
store::list_queue_items() and forward that result into the new helper; also
apply the same extraction for the duplicate logic referenced around the other
occurrence (lines ~130-135) so tests can invoke the helper with crafted queue
items to assert emission and dedup behavior.
- Around line 148-152: The test provider_keywords_case_insensitive_match
currently asserts by doing lower.contains(kw) and therefore bypasses production
logic; update the test to call the actual production matcher (check_handoff)
instead of doing a simple substring check so it uses the same token/phrase
behavior and normalization as production. Specifically, iterate
PROVIDER_KEYWORDS and for each kw build a representative input (e.g., "Check
your {kw} messages" or the original text variable), then call
check_handoff(text) (or the module's handoff-matching helper used in production)
and assert that it returns the expected match for that provider; keep the test
name and use PROVIDER_KEYWORDS as the source of keywords.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 12c2b5e3-0e94-4f2d-8318-6989406bed65

📥 Commits

Reviewing files that changed from the base of the PR and between ce14d7c and 898939e.

📒 Files selected for processing (3)
  • gitbooks/developing/architecture/desktop-companion.md
  • src/openhuman/desktop_companion/handoff.rs
  • src/openhuman/desktop_companion/session.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/openhuman/desktop_companion/session.rs

@yuvrxj-afk
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 18, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 18, 2026
Copy link
Copy Markdown
Contributor

@graycyrus graycyrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Walkthrough

Solid new desktop_companion domain — well-structured module layout, clean state machine, thorough test coverage (66 tests), and the session/pipeline/pointing/handoff/bus decomposition is thoughtful. Good job addressing all of CodeRabbit's feedback across the fixup commits. The PR honestly scopes itself as "PR 1 of 2" which is appreciated.

Three pattern-conformance issues to address before this ships:

Change Summary

File Change type Description
desktop-companion.md New Architecture doc — module layout, state machine, pipeline, RPC surface, testing
src/core/all.rs Modified Register 5 companion controllers + namespace description
src/core/event_bus/events.rs Modified Add 3 CompanionSession* DomainEvent variants
src/openhuman/about_app/catalog.rs Modified Add 2 companion capabilities (session + pointing)
src/openhuman/desktop_companion/bus.rs New Broadcast bus for state changes (same pattern as overlay)
src/openhuman/desktop_companion/handoff.rs New Provider-surface keyword matching with token-aware dedup
src/openhuman/desktop_companion/mod.rs New Module exports (thin, export-focused)
src/openhuman/desktop_companion/pipeline.rs New STT→LLM→TTS orchestration with cancellation
src/openhuman/desktop_companion/pointing.rs New [POINT:x,y:label:screenN] parser + multi-monitor mapping
src/openhuman/desktop_companion/schemas.rs New 5 RPC controllers
src/openhuman/desktop_companion/session.rs New Session lifecycle, state machine, TTL, conversation history
src/openhuman/desktop_companion/types.rs New Shared types — CompanionState, Config, Turn, params/results
src/openhuman/mod.rs Modified Register pub mod desktop_companion
tests/json_rpc_e2e.rs Modified Companion lifecycle RPC round-trip test
*_tests.rs (3 files) New 65 unit tests across session, pipeline, pointing

Pattern conformance

types.rs — missing PartialEq on domain types

CompanionConfig, ConversationTurn, CompanionSessionStatus, StartCompanionSessionParams, StopCompanionSessionParams, StartCompanionSessionResult, StopCompanionSessionResult don't derive PartialEq. The sibling provider_surfaces::types derives PartialEq on all domain types. This matters for assertion-based testing and will bite you in PR 2 when you want to assert on status/result values.

Suggestion: add PartialEq to all public types in types.rs.

types.rs — missing #[serde(deny_unknown_fields)] on inbound params

StartCompanionSessionParams and StopCompanionSessionParams are deserialized from RPC input but don't have #[serde(deny_unknown_fields)]. The provider_surfaces module applies this to all inbound types to catch stale or malformed client requests early.

Suggestion: add #[serde(deny_unknown_fields)] to both param structs.

Comment thread src/openhuman/desktop_companion/session.rs
Comment thread src/openhuman/desktop_companion/pipeline.rs
@yuvrxj-afk yuvrxj-afk force-pushed the feat/desktop-companion-core branch from 5d991a1 to 1996611 Compare May 18, 2026 10:22
@yuvrxj-afk yuvrxj-afk requested a review from graycyrus May 18, 2026 11:04
@senamakel
Copy link
Copy Markdown
Member

really cool

@senamakel senamakel merged commit 504d10e into tinyhumansai:main May 19, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build a Clicky-style desktop companion loop from OpenHuman’s existing screen-intelligence and voice foundations

3 participants