feat(notifications): core→shell DomainEvent bridge (Phase 1d of #395)#782
Merged
senamakel merged 5 commits intotinyhumansai:mainfrom Apr 23, 2026
Merged
Conversation
…nsai#395) Adds persistent notification center that captures events from the existing webview notification surface shipped in tinyhumansai#741. - notificationSlice (Redux + redux-persist) with categories + preferences - /notifications route + Notification Center page (read/unread, click-to-navigate, mark all, clear) - Bell tab in BottomTabBar with unread badge - NotificationsPanel in Settings → Features for per-category toggles (messages/agents/skills/system) - Webview notification handler dispatches into the center in addition to bumping account unread - Tests: 7 reducer cases covering prepend, preference gating, read flips, cap, unread count Next: PR B wires core-side native banners for agent/skill/channel events (tauri-plugin-notification).
…se 1c of tinyhumansai#395) Stacked on tinyhumansai#769. Adds OS-level notifications for events the frontend already sees over the existing socket.io + Tauri event surface, so the notification center earns its keep even when the app is backgrounded. - Tauri command `show_native_notification(title, body, tag?)` using the already-installed `tauri-plugin-notification` - Frontend `nativeNotifications` service subscribes to socket events: - `chat_done` → agents category - `chat_error` → system category - `disconnect` → system category - Only fires the OS banner when the window is NOT focused — in-app center covers the focused case - Respects per-category preferences from the settings panel - Vitest: 5 cases (dispatch, truncation, preference gating, focus gate, unfocused banner fire) Skipped the core→shell DomainEvent bridge for this PR: the Rust core runs in a separate process so piping DomainEvents into the Tauri shell would need a new event-relay transport. Existing socket events already carry agent completions, so this PR banner-izes them without new Rust plumbing. Bridge lands in a follow-up if we need cron/webhook/skill-sync banners too.
…umansai#395) Stacked on tinyhumansai#780. Adds a core-side notification bus that subscribes to selected DomainEvents and fans them out over the existing Socket.IO transport as `core_notification` / `core:notification` events — no new wire-level transport or RPC surface needed. New module `src/openhuman/notifications/`: - `types.rs` — `CoreNotificationEvent` (id, category, title, body, deep_link, timestamp_ms) matching the frontend slice shape - `bus.rs` — broadcast channel + `NotificationBridgeSubscriber` implementing `EventHandler`. Pure `event_to_notification(&DomainEvent)` translator, unit-tested. Surfaces: - `DomainEvent::CronJobCompleted` → agents category (success + failure variants) - `DomainEvent::WebhookProcessed` → system category (only when HTTP status ≥ 400 or an error was set — successful webhooks are silent) - `DomainEvent::SubagentCompleted` → agents category Wiring: - `src/core/socketio.rs::spawn_web_channel_bridge` adds a 5th bridge task that subscribes to `notifications::subscribe_core_notifications` and broadcasts to all connected sockets (mirrors the overlay / dictation bridges). - `src/core/jsonrpc.rs::register_domain_subscribers` registers the bridge subscriber alongside the existing webhook / channel / health ones, Once-guarded. Frontend (`app/src/lib/nativeNotifications/service.ts`): - New listener on the `core_notification` socket event dispatches into `notificationSlice` with the category + deep link delivered by the core. Banner path reuses the same focus/pref gating as chat events. Tests: - Rust: 6 unit cases over `event_to_notification` (cron pass, cron fail, successful webhook silent, failed webhook, subagent completed, unrelated event). - Vitest: 2 new cases for core_notification routing (normal + missing-fields filter) — 14/14 total across the service + slice.
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 31 minutes and 36 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 (4)
📝 WalkthroughWalkthroughA new core notification system has been implemented that bridges domain events to frontend clients via Socket.IO. The backend translates domain events into Changes
Sequence Diagram(s)sequenceDiagram
participant DE as Domain Event Source
participant NB as Notification Bus
participant SIO as Socket.IO Bridge
participant Client as Frontend Client
DE->>NB: emit DomainEvent (cron done, webhook done, etc.)
activate NB
NB->>NB: translate to CoreNotificationEvent
NB->>SIO: broadcast CoreNotificationEvent
deactivate NB
activate SIO
SIO->>Client: emit "core_notification" event
deactivate SIO
activate Client
Client->>Client: validate (id, title required)
Client->>Client: dispatch notification
Client->>Client: maybe show banner
deactivate Client
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Merged main into feat/notification-bridge. Resolved five conflict sites: - app/src/lib/webviewNotifications/service.ts: use cached `now` variable from main rather than two calls to Date.now(). - app/src/pages/Settings.tsx: add NotificationRoutingPanel import and route that main introduced alongside this PR's changes. - app/src/store/index.ts: add notificationsSlice / integrationNotifications reducer that main introduced alongside this PR's changes. - app/src/components/settings/panels/NotificationsPanel.tsx: keep main's DND (Do Not Disturb) superset over the PR's simpler category-only version. - app/src/lib/nativeNotifications/service.ts + test: merge PR's core_notification socket listener with main's listener-reference pattern for proper teardown via socketService.off(). - src/openhuman/notifications/types.rs + mod.rs: keep both the PR's CoreNotificationEvent bridge types and main's IntegrationNotification triage-pipeline types; expose both the bus and rpc/schemas submodules. All checks pass: tsc --noEmit clean, ESLint 0 errors, Prettier unchanged, cargo check clean (core + tauri), Vitest 548/548 tests green.
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/core/socketio.rs (1)
239-245:⚠️ Potential issue | 🟡 MinorUpdate bridge count/list in function doc now that a fifth bridge was added.
spawn_web_channel_bridgedoc comment still says "four bridges" and omits the new core_notification bridge. Also, the inline numbering becomes1, 2, 3, 5, 4because the new block at line 327 is labeled5.while the transcription bridge below remains4.— easy to misread when scanning.✏️ Proposed fix
-/// This function sets up four bridges: +/// This function sets up five bridges: /// 1. **Web Channel Bridge**: Forwards chat-related events (messages, tool calls) to specific clients. /// 2. **Dictation Bridge**: Forwards hotkey events to all clients. /// 3. **Overlay Bridge**: Forwards attention bubble events to all clients. -/// 4. **Transcription Bridge**: Forwards real-time speech-to-text results to all clients. +/// 4. **Core Notification Bridge**: Forwards core domain notifications to all clients. +/// 5. **Transcription Bridge**: Forwards real-time speech-to-text results to all clients.Either renumber the transcription block to
5.and the new block to4., or reorder them. Either way, keep the numbering monotonic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/socketio.rs` around lines 239 - 245, The doc comment for spawn_web_channel_bridge is out of sync: it still says "four bridges" and the numbered list misorders the new core_notification bridge (introducing 1,2,3,5,4). Update the comment to reflect five bridges and make the numbering monotonic (e.g., renumber the transcription bridge to 5 and the new core_notification block to 4, or reorder the blocks) so the list reads 1–5 correctly and includes the new core_notification bridge.src/core/jsonrpc.rs (1)
839-841:⚠️ Potential issue | 🟡 MinorUpdate subscriber list log to include
notifications.Line 813 registers the new notification bridge subscriber, but the summary log on line 840 still lists only the pre-existing subscribers. For grep-friendly tracing, add
notificationsto the list.✏️ Proposed fix
log::info!( - "[event_bus] domain subscribers registered (webhook, channel, health, conversation, composio, restart, proactive, agent)" + "[event_bus] domain subscribers registered (webhook, channel, health, notifications, conversation, composio, restart, proactive, agent)" );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/jsonrpc.rs` around lines 839 - 841, The event bus summary log currently printed by the log::info! call that lists registered domain subscribers is missing the newly added "notifications" subscriber; update the log message string used in that log::info! invocation to include "notifications" alongside the existing items (e.g., "...conversation, composio, restart, proactive, agent, notifications") so the summary accurately reflects the notification bridge subscriber registered elsewhere in the file.
🧹 Nitpick comments (2)
src/openhuman/notifications/bus.rs (1)
133-149: Add a tracing log on matched events for grep-friendly diagnostics.
handle()silently swallows non-matching events (intentional) and forwards matched ones topublish_core_notification, which logs at debug. A short trace/debug at the match site would help when tracing "why didn't this DomainEvent surface as a notification?" without cross-referencingpublish_core_notification's log.✏️ Optional addition
async fn handle(&self, event: &DomainEvent) { if let Some(notification) = event_to_notification(event) { + log::trace!( + "{LOG_PREFIX} translated domain event → notification id={} category={:?}", + notification.id, + notification.category, + ); publish_core_notification(notification); } }As per coding guidelines: "add substantial development-oriented logs at entry/exit points, branch decisions, external calls, retries, state transitions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/notifications/bus.rs` around lines 133 - 149, Add a grep-friendly trace/debug log inside NotificationBridgeSubscriber::handle when event_to_notification(event) yields Some(notification); specifically, log a short message including the DomainEvent type/id or a succinct identifier and that it matched before calling publish_core_notification, so you can trace matched events without digging into publish_core_notification; update the handle method to call the tracer (trace! or debug!) right after the if-let Some(...) match and include event and/or notification identifying fields.app/src/lib/nativeNotifications/service.ts (1)
106-177: Consider extracting shared core-notification handling to avoid duplication.
coreNotificationListener(Lines 106-115) and__handleCoreNotificationForTests(Lines 169-177) are near-identical implementations. If truncation limits, field mapping, or validation logic changes, both need to be updated in lockstep.♻️ Proposed refactor
+ function handleCoreNotification(payload: CoreNotificationPayload): void { + if (!payload.id || !payload.title) return; + dispatchAndMaybeBanner(payload.category, { + id: payload.id, + title: truncate(payload.title, 120), + body: truncate(payload.body ?? '', 160), + deepLink: payload.deep_link ?? undefined, + }); + } + coreNotificationListener = (...args: unknown[]) => { - const p = (args[0] ?? {}) as CoreNotificationPayload; - if (!p.id || !p.title) return; - dispatchAndMaybeBanner(p.category, { - id: p.id, - title: truncate(p.title, 120), - body: truncate(p.body ?? '', 160), - deepLink: p.deep_link ?? undefined, - }); + handleCoreNotification((args[0] ?? {}) as CoreNotificationPayload); };Then
__handleCoreNotificationForTestsjust delegates tohandleCoreNotification. Note the helper would need to be module-scoped (or the listener can be extracted similarly to how the test helper exists).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/lib/nativeNotifications/service.ts` around lines 106 - 177, coreNotificationListener and __handleCoreNotificationForTests duplicate the same validation, truncation and dispatch logic; extract a single module-scoped helper (e.g., handleCoreNotification) that accepts a CoreNotificationPayload, performs the id/title checks, truncates title/body, maps deep_link to deepLink, and calls dispatchAndMaybeBanner, then replace both coreNotificationListener and __handleCoreNotificationForTests to delegate to that helper (ensure the socketService.on registration still passes args[0] into the helper).
🤖 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/notifications/bus.rs`:
- Around line 73-131: The id generation in event_to_notification embeds now_ms()
making IDs change per publish; change to produce a stable logical id per event
kind instead: for DomainEvent::CronJobCompleted use format!("cron:{}:{}",
job_id, if *success {"ok"} else {"err"}) (no ts), for
DomainEvent::WebhookProcessed use the correlation_id (e.g. format!("webhook:{}",
correlation_id)) and if correlation_id is None fall back to
format!("webhook:{}", skill_id), and for DomainEvent::SubagentCompleted use
format!("subagent:{}:{}", parent_session, task_id) or at minimum
format!("subagent:{}", task_id); keep timestamp_ms = now_ms() as the event time
but remove ts from the id so frontend de-dup by stable ids.
---
Outside diff comments:
In `@src/core/jsonrpc.rs`:
- Around line 839-841: The event bus summary log currently printed by the
log::info! call that lists registered domain subscribers is missing the newly
added "notifications" subscriber; update the log message string used in that
log::info! invocation to include "notifications" alongside the existing items
(e.g., "...conversation, composio, restart, proactive, agent, notifications") so
the summary accurately reflects the notification bridge subscriber registered
elsewhere in the file.
In `@src/core/socketio.rs`:
- Around line 239-245: The doc comment for spawn_web_channel_bridge is out of
sync: it still says "four bridges" and the numbered list misorders the new
core_notification bridge (introducing 1,2,3,5,4). Update the comment to reflect
five bridges and make the numbering monotonic (e.g., renumber the transcription
bridge to 5 and the new core_notification block to 4, or reorder the blocks) so
the list reads 1–5 correctly and includes the new core_notification bridge.
---
Nitpick comments:
In `@app/src/lib/nativeNotifications/service.ts`:
- Around line 106-177: coreNotificationListener and
__handleCoreNotificationForTests duplicate the same validation, truncation and
dispatch logic; extract a single module-scoped helper (e.g.,
handleCoreNotification) that accepts a CoreNotificationPayload, performs the
id/title checks, truncates title/body, maps deep_link to deepLink, and calls
dispatchAndMaybeBanner, then replace both coreNotificationListener and
__handleCoreNotificationForTests to delegate to that helper (ensure the
socketService.on registration still passes args[0] into the helper).
In `@src/openhuman/notifications/bus.rs`:
- Around line 133-149: Add a grep-friendly trace/debug log inside
NotificationBridgeSubscriber::handle when event_to_notification(event) yields
Some(notification); specifically, log a short message including the DomainEvent
type/id or a succinct identifier and that it matched before calling
publish_core_notification, so you can trace matched events without digging into
publish_core_notification; update the handle method to call the tracer (trace!
or debug!) right after the if-let Some(...) match and include event and/or
notification identifying fields.
🪄 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: c8b9c8b9-cd76-47c9-acec-5e8c0d8000e0
📒 Files selected for processing (7)
app/src/lib/nativeNotifications/__tests__/service.test.tsapp/src/lib/nativeNotifications/service.tssrc/core/jsonrpc.rssrc/core/socketio.rssrc/openhuman/notifications/bus.rssrc/openhuman/notifications/mod.rssrc/openhuman/notifications/types.rs
Comment on lines
+73
to
+131
| pub fn event_to_notification(event: &DomainEvent) -> Option<CoreNotificationEvent> { | ||
| let ts = now_ms(); | ||
| match event { | ||
| DomainEvent::CronJobCompleted { job_id, success } => Some(CoreNotificationEvent { | ||
| id: format!("cron:{}:{}", job_id, ts), | ||
| category: CoreNotificationCategory::Agents, | ||
| title: if *success { | ||
| "Cron job completed".into() | ||
| } else { | ||
| "Cron job failed".into() | ||
| }, | ||
| body: format!("Job {job_id} finished."), | ||
| deep_link: Some("/settings/cron-jobs".into()), | ||
| timestamp_ms: ts, | ||
| }), | ||
| DomainEvent::WebhookProcessed { | ||
| skill_id, | ||
| status_code, | ||
| elapsed_ms, | ||
| error, | ||
| .. | ||
| } => { | ||
| // Only surface failures — successful webhooks are noisy. | ||
| if error.is_none() && *status_code < 400 { | ||
| return None; | ||
| } | ||
| Some(CoreNotificationEvent { | ||
| id: format!("webhook:{}:{}", skill_id, ts), | ||
| category: CoreNotificationCategory::System, | ||
| title: "Webhook error".into(), | ||
| body: match error { | ||
| Some(err) => { | ||
| format!("{skill_id} webhook failed after {elapsed_ms}ms: {err}") | ||
| } | ||
| None => format!( | ||
| "{skill_id} webhook returned HTTP {status_code} after {elapsed_ms}ms" | ||
| ), | ||
| }, | ||
| deep_link: Some("/webhooks".into()), | ||
| timestamp_ms: ts, | ||
| }) | ||
| } | ||
| DomainEvent::SubagentCompleted { | ||
| parent_session, | ||
| task_id, | ||
| agent_id, | ||
| output_chars, | ||
| .. | ||
| } => Some(CoreNotificationEvent { | ||
| id: format!("subagent:{}:{}:{}", parent_session, task_id, ts), | ||
| category: CoreNotificationCategory::Agents, | ||
| title: "Sub-agent finished".into(), | ||
| body: format!("{agent_id} produced {output_chars} chars of output."), | ||
| deep_link: Some("/chat".into()), | ||
| timestamp_ms: ts, | ||
| }), | ||
| _ => None, | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Stable-id semantics don't match the doc comment.
The id format is <kind>:<subject>:<ts> where ts is the current wall-clock milliseconds at translation time. Because ts is embedded, two publishes of the same logical event (e.g., a cron job retried, or the same webhook replayed) get different ids and therefore won't be de-duplicated on the frontend.
This contradicts the doc in src/openhuman/notifications/types.rs lines 25-27:
Stable id used for de-duplication in the center (e.g.
"cron:<job_id>:<ts>"). The frontend keys by this id so repeated publishes for the same logical event don't pile up.
Two options:
- Keep the current behavior and fix the doc to say the id is unique per publish (not per logical event).
- Drop
tsfrom the id and derive a stable id from the event's natural key (e.g.cron:<job_id>+ success/failure,webhook:<correlation_id>,subagent:<task_id>). The frontend slice can then replace-by-id for true de-dup.
Given the PR comment says the frontend keys by id, option 2 is probably the intended behavior. correlation_id is already available on WebhookProcessed and task_id on SubagentCompleted — both are stronger stability guarantees than a timestamp suffix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/notifications/bus.rs` around lines 73 - 131, The id generation
in event_to_notification embeds now_ms() making IDs change per publish; change
to produce a stable logical id per event kind instead: for
DomainEvent::CronJobCompleted use format!("cron:{}:{}", job_id, if *success
{"ok"} else {"err"}) (no ts), for DomainEvent::WebhookProcessed use the
correlation_id (e.g. format!("webhook:{}", correlation_id)) and if
correlation_id is None fall back to format!("webhook:{}", skill_id), and for
DomainEvent::SubagentCompleted use format!("subagent:{}:{}", parent_session,
task_id) or at minimum format!("subagent:{}", task_id); keep timestamp_ms =
now_ms() as the event time but remove ts from the id so frontend de-dup by
stable ids.
Contributor
Author
There was a problem hiding this comment.
Deferring to follow-up — not blocking v1.
Contributor
There was a problem hiding this comment.
@jwalin-shah, understood — happy to defer this to a follow-up. Would you like me to open a GitHub issue to track the stable-id de-duplication fix (dropping ts from the id and using natural event keys like cron:<job_id>:<ok|err>, webhook:<correlation_id>, subagent:<task_id>) so it doesn't get lost before Phase 2 polish?
(╯°□°）╯
🧠 Learnings used
Learnt from: CR
Repo: tinyhumansai/openhuman PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-23T06:30:00.454Z
Learning: Applies to src/openhuman/*/bus.rs : Each domain owns its `bus.rs` file with `EventHandler` implementations; register subscribers at startup via the singleton API in the domain's startup or initialization code
Learnt from: CR
Repo: tinyhumansai/openhuman PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-23T06:30:00.454Z
Learning: Applies to src/core/event_bus/**/*.rs : Use `publish_global(event)` for fire-and-forget notifications (one publisher, many subscribers); use `request_native_global(method, req)` for typed one-to-one request/response with zero serialization
Learnt from: CR
Repo: tinyhumansai/openhuman PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T21:14:11.082Z
Learning: Applies to src/openhuman/*/bus.rs : Each domain should own a `bus.rs` with `EventHandler` impls. Convention: `<Purpose>Subscriber` + `name()` returning `"<domain>::<purpose>"`
Learnt from: CR
Repo: tinyhumansai/openhuman PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T21:14:11.082Z
Learning: New/changed behavior must ship with matching rustdoc / code comments; update AGENTS.md or architecture docs when rules or user-visible behavior change
Learnt from: CR
Repo: tinyhumansai/openhuman PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-23T06:30:00.454Z
Learning: Applies to src/openhuman/**/*.rs : When adding or changing domain behavior, ship matching documentation (rustdoc, code comments, or updates to `AGENTS.md` / architecture docs) before handoff
Learnt from: CR
Repo: tinyhumansai/openhuman PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-23T06:30:00.454Z
Learning: Applies to src/openhuman/**/*.rs : Implement features in this order: specify against the codebase → implement in Rust → JSON-RPC E2E tests → UI in the Tauri app → app unit tests → app E2E
Learnt from: sanil-23
Repo: tinyhumansai/openhuman PR: 416
File: src/openhuman/memory/relex.rs:441-464
Timestamp: 2026-04-07T15:49:51.275Z
Learning: When using the `ort` Rust crate v2.x with the `load-dynamic` feature enabled, don’t require individual execution-provider feature flags (e.g., `directml`, `coreml`, `cuda`) alongside `load-dynamic` to get EP registration code. The `ort` crate already compiles EP registration via `#[cfg(any(feature = "load-dynamic", feature = "<ep_name>"))]` guards, and adding per-EP feature flags can pull in static-linking dependencies that conflict with the dynamic loading approach. At runtime, EP availability is determined by what the dynamically loaded ONNX Runtime library (`onnxruntime.dll`/`.so`/`.dylib`) supports; ort docs indicate providers like `directml`/`xnnpack`/`coreml` are available in builds when the platform supports them.
Learnt from: sanil-23
Repo: tinyhumansai/openhuman PR: 416
File: src/openhuman/memory/relex.rs:441-464
Timestamp: 2026-04-07T15:49:51.275Z
Learning: When integrating the `ort` Rust crate v2.x with the `load-dynamic` feature enabled, do NOT also require/enable individual provider EP Cargo features like `directml`, `coreml`, or `cuda`. In `ort` v2.x, EP registration for providers (e.g., DirectML, CoreML, CUDA, etc.) is already compiled in under source-level `#[cfg(any(feature = "load-dynamic", feature = "<provider>"))]` guards, such as in `ep/directml.rs`. Adding provider feature flags alongside `load-dynamic` can pull in static-linking dependencies that conflict with the dynamic-loading approach. Provider availability should be treated as runtime-determined by what the loaded `onnxruntime` library (`onnxruntime.dll`/`libonnxruntime.so`/`libonnxruntime.dylib`) actually supports.
Learnt from: oxoxDev
Repo: tinyhumansai/openhuman PR: 571
File: src/openhuman/local_ai/service/whisper_engine.rs:69-80
Timestamp: 2026-04-14T19:59:04.826Z
Learning: When reviewing Rust code in this repo that uses the upstream `whisper-rs` crate (v0.16.0), do not report `WhisperContextParameters::use_gpu(...)` or `WhisperContextParameters::flash_attn(...)` as missing/invalid APIs. These builder-style methods exist upstream and return `&mut Self`; they are not limited to `WhisperVadContextParams`.
- Differentiate CronJobCompleted body for success vs failure paths - Add SubagentFailed DomainEvent handler (was silently dropped) - Add missing unit test for subagent_failed_produces_agents_notification - Fix bridge numbering in socketio.rs: core_notification=4, transcription=5 and update spawn_web_channel_bridge doc to say "five bridges" - Use server timestamp_ms in native notifications service when available - #[derive(Default)] on NotificationBridgeSubscriber; remove manual impl - Add SAFETY comment near mem::forget in register_notification_bridge_subscriber - Fix types.rs doc comment: id is unique-per-publish, not a stable dedup key (addresses @coderabbitai stable-id semantics mismatch)
Contributor
Author
|
@senamakel — green + MERGEABLE, ready for a look when you have a sec. |
VectorJet
pushed a commit
to VectorJet/openhuman
that referenced
this pull request
Apr 24, 2026
…umansai#395) (tinyhumansai#782) Co-authored-by: Jwalin Shah <jshah1331@gmail.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
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
3 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
Stacked on #780 and #769. Closes the last open chunk of the notification work in #395: piping Rust-core DomainEvents into the in-app notification center, so cron completions / webhook failures / sub-agent completions surface alongside the chat + webview events already wired.
Design
Reuses the existing Socket.IO transport — no new RPC surface. Matches the overlay / dictation bridge pattern that already broadcasts to all clients.
Rust (
src/openhuman/notifications/)types.rs—CoreNotificationEvent { id, category, title, body, deep_link, timestamp_ms }, shape matches frontendNotificationItembus.rs— broadcast channel +NotificationBridgeSubscriberimplementingEventHandler. Pureevent_to_notification(&DomainEvent)translator, unit-testedcore::socketio::spawn_web_channel_bridgeas a 5th bridge taskregister_domain_subscribers()(Once-guarded)Variants surfaced
CronJobCompletedWebhookProcessedstatus >= 400orerror.is_some()— successful webhooks are noisySubagentCompletedFrontend (
app/src/lib/nativeNotifications/service.ts)core_notificationsocket eventnotificationSlicewith the server-provided category + deep linkMerge order
Stacked: #769 → #780 → this. After each merges the next will auto-rebase.
Test plan
cargo test openhuman::notificationsgreentsc --noEmit+eslint+cargo checkcleanRemaining for #395
Phase 2 polish — DND / quiet hours, sound customization, Windows/Linux-specific tweaks. Not blocking; punt until we see how 1a–1d feel in use.
Summary by CodeRabbit
Release Notes