feat(heartbeat): deliver durable proactive meeting/reminder notifications with dedup + category controls#1369
Merged
senamakel merged 7 commits intotinyhumansai:mainfrom May 9, 2026
Conversation
…minders, and important events
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds meetings, reminders, and important notification categories across frontend and backend; implements a heartbeat planner that collects cron reminders, calendar meetings, and relevant notifications, deduplicates via a SQLite store, persists integration notifications, exposes RPCs/schemas, and integrates planner ticks into the heartbeat loop. Heartbeat Proactive Notifications Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 8
🧹 Nitpick comments (1)
src/openhuman/heartbeat/rpc.rs (1)
36-92: ⚡ Quick winAdd stable debug/trace logs around the new heartbeat RPC entrypoints.
These handlers currently rely on the returned
RpcOutcome, but the repo guidance for new RPC flows is stronger than that. A couple of[heartbeat][rpc]entry/exit/error logs aroundsettings_get,settings_set, andtick_nowwould make support and incident triage much easier without logging sensitive payloads.As per coding guidelines,
src/**/*.rs: “Uselog/tracingatdebugortracelevel on RPC entry/exit, error paths, state transitions... include stable prefixes.”🤖 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/heartbeat/rpc.rs` around lines 36 - 92, Add stable debug/trace logs with the prefix "[heartbeat][rpc]" at entry and exit (and on errors) for settings_get, settings_set, and tick_now: log when each RPC is entered (function name + normalized non-sensitive markers), log successful exit with outcome summary (no sensitive config fields), and log failures around config::rpc::load_config_with_timeout(), config.save(), and planner::evaluate_and_dispatch() including the error string; place logs inside settings_get, settings_set, and tick_now near the calls to config::rpc::load_config_with_timeout, config.save, and planner::evaluate_and_dispatch so you capture start/finish/error for each operation without emitting sensitive payloads.
🤖 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 `@app/src/store/notificationSlice.ts`:
- Around line 50-58: Add a migrate step to notificationPersistConfig (or handle
in a REHYDRATE extraReducer) that backfills missing keys on rehydration so older
persisted preferences gain meetings, reminders, and important; specifically,
implement a migrate(oldState) that returns Object.assign({}, oldState, {
preferences: Object.assign({ messages: true, agents: true, skills: true, system:
true, meetings: true, reminders: true, important: true }, oldState?.preferences
|| {}) }) (or the equivalent logic in the REHYDRATE handler) so
notificationPersistConfig and the code that checks
state.preferences[item.category] see those keys present after rehydration.
In `@src/openhuman/heartbeat/engine.rs`:
- Around line 45-46: The planner loop currently always calls
run_event_planner_tick() even when config.heartbeat.enabled == false; update the
loop to reload/read the current config and short-circuit (continue/return) when
config.heartbeat.enabled is false so the tick is skipped, i.e., check
config.heartbeat.enabled before invoking run_event_planner_tick(); apply the
same guard wherever run_event_planner_tick() is called later in this file (the
other invocation region around the block noted at 94-111) so planner ticks stop
immediately when heartbeat is disabled.
In `@src/openhuman/heartbeat/planner/mod.rs`:
- Around line 134-179: mark_sent is currently called before
persist_heartbeat_alert, which lets dedupe rows suppress future retries even if
the durable notification write fails; move the durable write ahead of marking.
Specifically: call persist_heartbeat_alert (and ensure it returns an error on
failure) and only call store::mark_sent (or store::mark_sent in the other
similar block) after persist_heartbeat_alert succeeds; on
persist_heartbeat_alert failure do not call mark_sent, increment
summary.deliveries_failed (or appropriate counter) and continue/retry behavior
instead of creating the dedupe marker, and keep publish_core_notification
executed only after both durable persist and mark_sent succeed (or guard
publish_core_notification behind the same success check). Ensure both code paths
referenced (the shown block and the 635-680 duplicate) are updated accordingly.
- Around line 470-499: collect_relevant_notifications currently calls
notifications_store::list with a score cutoff (Some(0.8)) which filters out
low/no-score items before we can check triage_action ("escalate" or "react") or
raw_payload.urgent; change the list call to not apply the importance_score
threshold (pass None for the last argument instead of Some(0.8)) so all
candidate notifications are returned, then keep the existing downstream filters
(provider != "heartbeat", status == Unread, triage_action/raw_payload checks,
and recent received_at) so the planner can surface urgent or action-worthy items
regardless of their score.
- Around line 119-124: The dedupe key is currently source-scoped because it uses
event.fingerprint; replace that with an overlap-scoped identifier so identical
underlying events from multiple sources collide. Add or use a function like
compute_overlap_key(event) (or event.overlap_id / event.occurrence_id if
available) and build the stable_key from format!("{}|{}|{}",
event.category.as_str(), compute_overlap_key(event), plan.stage) instead of
event.fingerprint; apply the same change to the other identical key
constructions (the other occurrences of the same pattern) so deduplication is
based on the overlapping event identity rather than the source-specific
fingerprint.
- Around line 373-407: The parser currently treats all-day events as timed
events because extract_datetime_from_map falls back from start.dateTime to
start.date, producing a 00:00 UTC starts_at; update the logic to skip all-day
entries before creating a PendingEvent for HeartbeatCategory::Meetings by
detecting a start.date field (or by changing extract_datetime_from_map to only
return Some for dateTime and return None for all-day dates). In practice either
(a) modify extract_datetime_from_map to refuse date-only values so starts_at is
None for all-day events, or (b) add an explicit check where starts_at is used
(before constructing PendingEvent in this block and the similar block at
425-467) to continue/skip if map["start"] contains a "date" key, ensuring only
timed events are promoted to HeartbeatCategory::Meetings and anchor_at uses a
real dateTime.
- Around line 18-681: Summary: mod.rs is too large and should be split into
export-focused sibling modules; move collectors, planning, persistence, and
helpers into smaller files and re-export them from mod.rs. Fix: create
collectors.rs and move collect_cron_reminders, collect_calendar_meetings,
collect_relevant_notifications, extract_calendar_events,
collect_calendar_events_recursive, extract_datetime_from_map,
extract_title_from_map, parse_datetime there; create planner.rs (or
planner_core.rs) and move evaluate_and_dispatch, plan_delivery_for_event,
PendingEvent, PlannedDelivery, HeartbeatCategory and PlannerRunSummary there;
create persistence.rs and move persist_heartbeat_alert, stable_key and the
notifications_store/store:: calls there; create utils.rs for sanitize_preview
and any small helpers; update mod.rs to only pub mod and pub use the moved
symbols (e.g., pub use planner::{evaluate_and_dispatch, PlannerRunSummary}; pub
use collectors::collect_*; etc.), and adjust internal imports/paths so functions
like evaluate_and_dispatch still call store::mark_sent and notifications_store
via the new modules; ensure Cargo/Rust visibility (pub(crate)/pub) and
tests/imports are updated accordingly.
In `@src/openhuman/heartbeat/rpc.rs`:
- Around line 52-54: The RPC handler currently writes interval_minutes down to 1
(using interval_minutes.max(1)), which contradicts HeartbeatConfig's documented
5-minute minimum and HeartbeatEngine::run's enforcement; change the assignment
in the patch handling so that when patch.interval_minutes is Some, you store
config.heartbeat.interval_minutes = interval_minutes.max(5) (clamping to 5)
instead of .max(1) so persisted settings match the runtime behavior and
settings_get reports a valid cadence.
---
Nitpick comments:
In `@src/openhuman/heartbeat/rpc.rs`:
- Around line 36-92: Add stable debug/trace logs with the prefix
"[heartbeat][rpc]" at entry and exit (and on errors) for settings_get,
settings_set, and tick_now: log when each RPC is entered (function name +
normalized non-sensitive markers), log successful exit with outcome summary (no
sensitive config fields), and log failures around
config::rpc::load_config_with_timeout(), config.save(), and
planner::evaluate_and_dispatch() including the error string; place logs inside
settings_get, settings_set, and tick_now near the calls to
config::rpc::load_config_with_timeout, config.save, and
planner::evaluate_and_dispatch so you capture start/finish/error for each
operation without emitting sensitive payloads.
🪄 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: ba0287be-5762-4f39-bcbd-f3e793ef20b2
📒 Files selected for processing (13)
app/src/components/settings/panels/NotificationsPanel.tsxapp/src/lib/notificationRouter.tsapp/src/pages/Notifications.tsxapp/src/store/__tests__/settingsSlice.test.tsapp/src/store/notificationSlice.tssrc/openhuman/config/schema/heartbeat_cron.rssrc/openhuman/heartbeat/engine.rssrc/openhuman/heartbeat/mod.rssrc/openhuman/heartbeat/planner/mod.rssrc/openhuman/heartbeat/planner/store.rssrc/openhuman/heartbeat/rpc.rssrc/openhuman/heartbeat/schemas.rssrc/openhuman/notifications/types.rs
…tegories Add three missing test cases for the new notification categories introduced in the previous commit. Covers lines 91-92, 94-95, 97-98 of notificationRouter.ts which were flagged at 0% by diff-cover, bringing changed-line coverage above 80%.
The dedupe key was previously derived from event.fingerprint, which is source-specific (cron job ID, calendar connection/event ID, notification ID). This meant the same underlying event surfaced by two different sources (e.g. a meeting visible in both a cron reminder and a Google Calendar connection) produced distinct dedupe keys and could generate duplicate notifications. Add an overlap_key to PendingEvent computed from category + normalized title + 15-minute time bucket. Use overlap_key instead of fingerprint when building the final dedupe hash in evaluate_and_dispatch so identical events from multiple sources collide on the same key. Also adds compute_overlap_key() helper to utils.rs and two new unit tests that verify same-bucket/cross-source collision and different-title/time separation. Fixes @coderabbitai comment on planner/mod.rs:124.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/openhuman/heartbeat/planner/plan.rs (1)
88-107: 💤 Low valueConsider adding a lower bound for Important events.
The condition
now.signed_duration_since(event.anchor_at) <= Duration::minutes(10)only checks the upper bound. An event withanchor_atfrom hours/days ago would still match. While collectors filter by recency (30 minutes for notifications), this function could be more defensive.💡 Optional defensive bound
HeartbeatCategory::Important => { - if now.signed_duration_since(event.anchor_at) <= Duration::minutes(10) { + let since_anchor = now.signed_duration_since(event.anchor_at); + if since_anchor >= Duration::zero() && since_anchor <= Duration::minutes(10) { return Some(PlannedDelivery {🤖 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/heartbeat/planner/plan.rs` around lines 88 - 107, The Important branch currently only checks an upper bound using now.signed_duration_since(event.anchor_at) which allows very old anchors to match; compute the signed duration once (e.g., let delta = now.signed_duration_since(event.anchor_at)) and require a lower and upper bound (for example delta >= Duration::minutes(0) && delta <= Duration::minutes(10)) before returning the PlannedDelivery in HeartbeatCategory::Important so only items within the recent window are considered; update the condition around the PlannedDelivery creation accordingly.
🤖 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 `@app/src/store/notificationSlice.ts`:
- Line 1: This file fails Prettier formatting; run the project formatter (e.g.
npm run format from the repo root or prettier --write the source file) to fix
style issues in the notification slice (imports referencing createSlice and type
PayloadAction and the notificationSlice code) and re-commit the formatted file.
- Around line 156-175: The REHYDRATE handler in builder.addCase is incorrectly
checking rehydrateAction.meta?.arg; update the type assertion for the action and
change the filter to use action.key === 'notifications' (the redux-persist
persist key) instead of meta?.arg so the backfill runs; keep the existing logic
that merges payload.preferences into state.preferences (using
initialState.preferences and payload.preferences) so missing keys are filled
(refer to REHYDRATE, rehydrateAction, state.preferences and
initialState.preferences).
In `@src/openhuman/heartbeat/planner/mod.rs`:
- Around line 390-434: The test functions
overlap_key_same_for_cross_source_same_event and
overlap_key_differs_for_different_titles_or_times are misformatted; run rustfmt
(cargo fmt) across the repo or on the Rust sources, reformat the code so
compute_overlap_key usages and the Duration::minutes calls align with project
style, then re-run cargo fmt and cargo check and commit the reformatted changes
(ensuring tests referencing compute_overlap_key and HeartbeatCategory::Meetings
remain unchanged functionally).
In `@src/openhuman/heartbeat/planner/utils.rs`:
- Around line 43-44: The two consecutive assignments to normalized_title need
rustfmt formatting; run `cargo fmt` (or reformat the expression) so the chain is
idiomatically formatted — for example collapse the operations into a single
statement on normalized_title (use
title.to_ascii_lowercase().split_whitespace().collect::<Vec<_>>().join(" ")) and
then run `cargo fmt` and `cargo check` to ensure CI passes; locate the code
around the normalized_title variable in utils.rs to apply this change.
---
Nitpick comments:
In `@src/openhuman/heartbeat/planner/plan.rs`:
- Around line 88-107: The Important branch currently only checks an upper bound
using now.signed_duration_since(event.anchor_at) which allows very old anchors
to match; compute the signed duration once (e.g., let delta =
now.signed_duration_since(event.anchor_at)) and require a lower and upper bound
(for example delta >= Duration::minutes(0) && delta <= Duration::minutes(10))
before returning the PlannedDelivery in HeartbeatCategory::Important so only
items within the recent window are considered; update the condition around the
PlannedDelivery creation accordingly.
🪄 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: 390034fb-4b2c-49bc-be69-7f6ad033e43a
📒 Files selected for processing (9)
app/src/store/notificationSlice.tssrc/openhuman/heartbeat/engine.rssrc/openhuman/heartbeat/planner/collectors.rssrc/openhuman/heartbeat/planner/mod.rssrc/openhuman/heartbeat/planner/persistence.rssrc/openhuman/heartbeat/planner/plan.rssrc/openhuman/heartbeat/planner/types.rssrc/openhuman/heartbeat/planner/utils.rssrc/openhuman/heartbeat/rpc.rs
✅ Files skipped from review due to trivial changes (1)
- src/openhuman/heartbeat/planner/types.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/openhuman/heartbeat/rpc.rs
… notificationSlice redux-persist sets action.key, not action.meta.arg, so the previous guard always evaluated undefined !== 'notifications' (true) and returned early, making the preferences backfill for meetings/reminders/important a no-op for existing users.
… cases for diff-cover lines 161,167-170
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
Added a heartbeat event planner:
Added persistent dedup state:
heartbeat_state.dbIntegrated calendar-aware detection:
Added heartbeat notification settings + RPC/controller APIs:
settings_getsettings_settick_nowExtended notification categories:
Made heartbeat alerts durable:
Problem
Existing heartbeat/notification primitives lacked a complete proactive event workflow
Upcoming meetings/reminders could be missed:
No dedicated dedup layer:
Users lacked controls for proactive notification categories
Solution
Heartbeat Planner
Implemented a dedicated planner that:
Collects event candidates from:
Computes staged delivery windows:
Records send markers in SQLite:
INSERT OR IGNOREPublishes via:
core_notificationHeartbeat Configuration
Added new
HeartbeatConfigsettings:notify_meetingsnotify_remindersnotify_relevant_eventsexternal_delivery_enabledmeeting_lookahead_minutesreminder_lookahead_minutesRPC / Controller Surface
Added heartbeat endpoints:
Notification Model Updates
Extended notification categories and settings UI to support:
Reliability Improvements
Tradeoffs
Calendar detection depends on:
Does NOT use:
Favors:
Submission Checklist
Tests
docs/TESTING-STRATEGY.mdCoverage
.github/workflows/coverage.ymlpnpm test:coveragepnpm test:rustCoverage Matrix
docs/TEST-COVERAGE-MATRIX.mdFeature IDs
## RelatedNetwork Dependencies
Manual Smoke Checklist
docs/RELEASE-MANUAL-SMOKE.mdLinked Issues
Closes #NNNRelated
Closes #NNNImpact
Platform / Runtime
Performance
Small additional heartbeat tick work:
Overhead bounded by:
Security / Privacy
Logging remains metadata-focused:
Avoids full private payload logging
External proactive delivery requires:
Compatibility / Migration
Backward compatible config evolution via defaults
Existing notification preferences gain new categories:
Related
Summary by CodeRabbit
New Features
Tests