Add background loop controls and usage diagnostics#1965
Conversation
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds per-tick calendar connection caps, Tauri heartbeat RPC wrappers/tests, config/schema and RPC updates, engine scheduler and lifecycle changes, and a new BackgroundLoopControls diagnostics UI in the AI settings panel. ChangesBackground loop rate-limiting and diagnostics
Sequence DiagramsequenceDiagram
participant AIPanel
participant TauriRPC as openhumanHeartbeat*
participant RPC as heartbeat.settings_set/view/tick
participant Config
participant Engine as HeartbeatEngine
AIPanel->>TauriRPC: openhumanHeartbeatSettingsSet(patch)
TauriRPC->>RPC: callCoreRpc("openhuman.heartbeat_settings_set", params: patch)
RPC->>Config: persist patched heartbeat config (max_calendar_connections_per_tick, enabled)
RPC->>Engine: bootstrap_after_login() or stop_heartbeat_loop()
Engine->>Config: reload per-iteration config
Engine->>Engine: run_event_planner_tick_for_config(config) -> evaluate_and_dispatch()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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. 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 |
|
Second pass pushed in 84ff054. Added real calendar fanout budget control (max_calendar_connections_per_tick), exposed meeting/reminder lookahead controls, and expanded AI settings diagnostics with weekly heartbeat ticks, calendar planner calls, Composio scan counts, background API read budget, background wakeups, rows-left budget math, sample burn rate, and projected exhaustion from recent ledger rows. Validation: format/check via pre-push, app compile, app unit heartbeat test, Rust heartbeat default test, rust check. |
|
Fixed the failing frontend tests from the latest run. Root cause was an ambiguous /Learning/ text match in AIPanel: the existing workload row and the new background-loop row both rendered Learning. Renamed the new loop row to Reflection rebuild and removed the extra learning word from its helper copy.\n\nLocal validation after fix:\n- npm --prefix app run test:unit -- src/components/settings/panels/tests/AIPanel.test.tsx\n- npm --prefix app run compile\n- git diff --check\n- pre-push format/lint/compile/rust:check passed with existing warnings only\n\nPushed commit: 3d98b94 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/openhuman/heartbeat/rpc.rs (1)
99-105: ⚡ Quick winAdd explicit debug logs for enable/disable lifecycle branch decisions.
This new state-transition path should log which branch executed (enable/bootstrap vs disable/stop) for operability and incident triage.
As per coding guidelines, "Add substantial, development-oriented logs ... at ... branch decisions, ... state transitions, and error handling paths."
🤖 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 99 - 105, Add explicit debug logs for the heartbeat lifecycle branch: before calling crate::openhuman::subconscious::global::bootstrap_after_login() log a debug message like "heartbeat: enabling - starting bootstrap_after_login()" and after a successful bootstrap optionally log success; before calling crate::openhuman::subconscious::global::stop_heartbeat_loop() log a debug message like "heartbeat: disabling - stopping heartbeat loop()". Keep the existing warn! on Err(error) but ensure the debug logs precede the calls so branch decisions (config.heartbeat.enabled true/false) are always recorded for triage.app/src/components/settings/panels/AIPanel.tsx (1)
514-1317: 🏗️ Heavy liftExtract
BackgroundLoopControls(and its budget helpers) into separate modules.This addition substantially increases
AIPanel.tsxcomplexity and makes future changes/testability harder; splitting into focused files would reduce coupling and review risk.As per coding guidelines: "Frontend component files should generally not exceed ~500 lines; split growing modules into smaller units."
🤖 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 `@app/src/components/settings/panels/AIPanel.tsx` around lines 514 - 1317, The file grew too large—extract BackgroundLoopControls and its helper utilities (constants USD, WEEK_MINUTES, COMPOSIO_PERIODIC_TICK_MINUTES, LEARNING_REBUILD_MINUTES, MEMORY_WORKERS, MEMORY_POLL_SECONDS; helper functions formatUsd, spendAmount, formatCount, formatDateTime, activeConnection, normalizedToolkit, isCalendarConnection, summarizeSpendByAction, summarizeSpendByHour, summarizeSpendSample, describeProvider; and small presentational subcomponents LoopToggle, MetricTile, FormulaRow) into their own module(s) so AIPanel.tsx only imports BackgroundLoopControls and the helpers it needs; move the BackgroundLoopControls component and any state/logic (refresh, applyHeartbeatPatch, runPlannerNow, computed budget math and loops array) into a new file (e.g., backgroundLoopControls.tsx) and export the component and any pure helpers you want to test, then update AIPanel to import those symbols—this keeps component presentation in AIPanel and moves heavy logic and formatting into focused, testable modules.app/src/utils/tauriCommands/heartbeat.test.ts (1)
22-58: ⚡ Quick winAdd coverage for
openhumanHeartbeatTickNowand non-Tauri rejection path.This suite validates get/set, but misses the third newly-added command and the
isTauri() === falseerror behavior.🤖 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 `@app/src/utils/tauriCommands/heartbeat.test.ts` around lines 22 - 58, Add tests for the missing command and the non-Tauri rejection: import and call openhumanHeartbeatTickNow from ./heartbeat and mock mockCallCoreRpc to resolve; assert it calls mockCallCoreRpc with { method: 'openhuman.heartbeat_tick_now' } and that the function resolves/returns the mocked result. Also add a test that stubs isTauri() to return false (mock the module that exports isTauri or jest.spyOn it) then import openhumanHeartbeatSettingsGet (or openhumanHeartbeatTickNow) and assert it rejects with the expected non-Tauri error (e.g., contains "not supported" or the existing error text) so the code path when isTauri() === false is covered.
🤖 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/components/settings/panels/AIPanel.tsx`:
- Around line 736-755: applyHeartbeatPatch can suffer from out-of-order
responses overwriting newer changes; make requests serializable or ignore stale
responses by attaching a monotonic request id and only applying the response if
the id matches the latest. Specifically, add a requestIdRef (useRef<number>)
that you increment before calling openhumanHeartbeatSettingsSet, capture the
current id in a local const, and then in both the success path (where you call
setSettings(response.result.settings)) and the error path (where you restore
previous), verify that the captured id === requestIdRef.current before mutating
state or clearing setSaving; keep setSaving keyed by that id so UI reflects the
correct inflight op, and remove settings from the useCallback dependency list
(use the ref/functional updates) so the closure doesn't capture stale values.
In `@src/openhuman/heartbeat/rpc.rs`:
- Around line 99-103: In settings_set, when config.heartbeat.enabled is true and
crate::openhuman::subconscious::global::bootstrap_after_login().await returns
Err, do not just warn and return success; instead propagate a failure back to
the caller (early return) by mapping the bootstrap error into the RPC function's
error type (or return an Err variant), so the API reports failure and the
settings change is not reported as successful; update the code around the
settings_set handler to return the mapped error (or alternatively roll back
config.heartbeat.enabled to false and persist that change before returning an
error) and reference the settings_set function and bootstrap_after_login() call
to locate where to implement the change.
In `@src/openhuman/subconscious/global.rs`:
- Around line 130-133: The heartbeat abort is currently fire-and-forget: in
stop_heartbeat_loop() after calling handle.abort() you must await the task's
JoinHandle to ensure the heartbeat task has fully stopped before resetting
BOOTSTRAPPED or allowing bootstrap_after_login() to run; locate the abort call
in heartbeat_slot().lock().await.take() handling, call .await on the JoinHandle
(handling/ignoring the JoinError from cancellation) and only then proceed to set
BOOTSTRAPPED = false or return, preventing engine.run_tick() from racing with a
new bootstrap.
---
Nitpick comments:
In `@app/src/components/settings/panels/AIPanel.tsx`:
- Around line 514-1317: The file grew too large—extract BackgroundLoopControls
and its helper utilities (constants USD, WEEK_MINUTES,
COMPOSIO_PERIODIC_TICK_MINUTES, LEARNING_REBUILD_MINUTES, MEMORY_WORKERS,
MEMORY_POLL_SECONDS; helper functions formatUsd, spendAmount, formatCount,
formatDateTime, activeConnection, normalizedToolkit, isCalendarConnection,
summarizeSpendByAction, summarizeSpendByHour, summarizeSpendSample,
describeProvider; and small presentational subcomponents LoopToggle, MetricTile,
FormulaRow) into their own module(s) so AIPanel.tsx only imports
BackgroundLoopControls and the helpers it needs; move the BackgroundLoopControls
component and any state/logic (refresh, applyHeartbeatPatch, runPlannerNow,
computed budget math and loops array) into a new file (e.g.,
backgroundLoopControls.tsx) and export the component and any pure helpers you
want to test, then update AIPanel to import those symbols—this keeps component
presentation in AIPanel and moves heavy logic and formatting into focused,
testable modules.
In `@app/src/utils/tauriCommands/heartbeat.test.ts`:
- Around line 22-58: Add tests for the missing command and the non-Tauri
rejection: import and call openhumanHeartbeatTickNow from ./heartbeat and mock
mockCallCoreRpc to resolve; assert it calls mockCallCoreRpc with { method:
'openhuman.heartbeat_tick_now' } and that the function resolves/returns the
mocked result. Also add a test that stubs isTauri() to return false (mock the
module that exports isTauri or jest.spyOn it) then import
openhumanHeartbeatSettingsGet (or openhumanHeartbeatTickNow) and assert it
rejects with the expected non-Tauri error (e.g., contains "not supported" or the
existing error text) so the code path when isTauri() === false is covered.
In `@src/openhuman/heartbeat/rpc.rs`:
- Around line 99-105: Add explicit debug logs for the heartbeat lifecycle
branch: before calling
crate::openhuman::subconscious::global::bootstrap_after_login() log a debug
message like "heartbeat: enabling - starting bootstrap_after_login()" and after
a successful bootstrap optionally log success; before calling
crate::openhuman::subconscious::global::stop_heartbeat_loop() log a debug
message like "heartbeat: disabling - stopping heartbeat loop()". Keep the
existing warn! on Err(error) but ensure the debug logs precede the calls so
branch decisions (config.heartbeat.enabled true/false) are always recorded for
triage.
🪄 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: d7e1033c-7369-4747-8919-bd621353eb51
📒 Files selected for processing (10)
app/src/components/settings/panels/AIPanel.tsxapp/src/utils/tauriCommands/heartbeat.test.tsapp/src/utils/tauriCommands/heartbeat.tsapp/src/utils/tauriCommands/index.tssrc/openhuman/config/schema/heartbeat_cron.rssrc/openhuman/heartbeat/engine.rssrc/openhuman/heartbeat/planner/collectors.rssrc/openhuman/heartbeat/rpc.rssrc/openhuman/heartbeat/schemas.rssrc/openhuman/subconscious/global.rs
- subconscious::global::stop_heartbeat_loop: await aborted JoinHandle before resetting BOOTSTRAPPED so a new bootstrap_after_login() cannot race with the old heartbeat task still executing engine.run_tick(). - heartbeat::rpc::settings_set: propagate bootstrap failure to the caller instead of warn-and-return-success, and log the enable/disable branch decision for triage. - AIPanel.BackgroundLoopControls.applyHeartbeatPatch: serialize patches via a monotonic request-id ref so an older response can't clobber a newer optimistic update; drop `settings` from the useCallback deps and use functional setState to avoid capturing a stale snapshot. - heartbeat tauri-command tests: add coverage for openhumanHeartbeatTickNow and the isTauri()===false rejection path on all three RPC wrappers.
| ); | ||
| }; | ||
|
|
||
| // ───────────────────────────────────────────────────────────────────────────── |
There was a problem hiding this comment.
Deferred: leaving the BackgroundLoopControls extraction (CodeRabbit 'heavy lift' nit on this range) for a follow-up PR. Pure refactor of ~800 lines with no behavioural delta — bundling it into this PR would obscure the review-fix diff and balloon the change. Tracking separately.
|
@coderabbitai commentns resolved. can u check again? |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/openhuman/heartbeat/planner/collectors.rs (1)
154-163: ⚡ Quick winLog the capped calendar fanout decision.
This new branch chooses a subset of eligible connections but never emits the configured cap or the eligible-vs-selected counts, which makes missed-event reports hard to correlate with the new budget control.
As per coding guidelines, "Ship heavy debug/trace level logs on new flows and critical checkpoints (entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, error paths) using
log/tracingwith grep-friendly 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/planner/collectors.rs` around lines 154 - 163, Add a log statement that records the eligible vs selected counts and the applied cap when capping calendar fanout: compute the count of eligible connections by filtering connections with is_active() and normalized_toolkit() matching "googlecalendar"/"google_calendar"/"calendar", then log the eligible_count, the calendar_connection_limit value, and the number actually taken (selected_count) before the .take(...) is applied; reference the existing symbols calendar_connection_limit, connections, and normalized_toolkit() and emit a grep-friendly tracing/log line (e.g., "calendar-fanout: eligible=%d cap=%d selected=%d") at debug/trace level at the branching point.src/openhuman/config/schema/heartbeat_cron.rs (1)
87-103: ⚡ Quick winAdd a deserialize regression test for omitted heartbeat keys.
HeartbeatConfig::default()only exercises the constructor. The regression here depends on serde filling missing fields in persisted/restored configs, so please also deserialize an empty or partial payload and assert the booleans stay opt-in andmax_calendar_connections_per_tickstays2.🤖 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/config/schema/heartbeat_cron.rs` around lines 87 - 103, Add a serde deserialization regression test alongside the existing HeartbeatConfig::default() test that deserializes an empty (e.g. "{}") and/or partial JSON payload into HeartbeatConfig and asserts the same opt-in booleans and defaults: enabled, inference_enabled, notify_meetings, notify_reminders, notify_relevant_events, external_delivery_enabled are all false and max_calendar_connections_per_tick == 2 (and interval_minutes == 5); use serde_json::from_str::<HeartbeatConfig>() to perform deserialization so the test verifies missing keys are filled with the struct defaults rather than only exercising HeartbeatConfig::default().app/src/components/settings/panels/AIPanel.tsx (1)
737-755: ⚡ Quick winRefactor to avoid side effects in state updater.
While the current code at lines 737-770 works correctly, it uses a non-idiomatic pattern of capturing state via a side effect inside the
setSettingsupdater. Prefer capturingsettingsfrom the closure directly:Suggested refactor
const applyHeartbeatPatch = useCallback(async (patch: HeartbeatSettingsPatch) => { const requestId = patchRequestIdRef.current + 1; patchRequestIdRef.current = requestId; const savingKey = Object.keys(patch).join(','); + const previous = settings; + if (!previous) return; - let previous: HeartbeatSettings | null = null; setError(''); setSaving(savingKey); - setSettings(current => { - if (!current) return current; - previous = current; - return { ...current, ...patch }; - }); - if (!previous) { - // No baseline to patch against — abandon this request. - if (patchRequestIdRef.current === requestId) { - setSaving(null); - } - return; - } + setSettings({ ...previous, ...patch }); try { const response = await openhumanHeartbeatSettingsSet(patch); if (patchRequestIdRef.current !== requestId) return; setSettings(response.result.settings); } catch (err) { if (patchRequestIdRef.current !== requestId) return; setSettings(previous); setError(err instanceof Error ? err.message : String(err)); } finally { if (patchRequestIdRef.current === requestId) { setSaving(null); } } -}, []); +}, [settings]);🤖 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 `@app/src/components/settings/panels/AIPanel.tsx` around lines 737 - 755, The setSettings updater currently uses a side effect to capture the previous settings (assigning to `previous` inside the `setSettings` updater), which is non-idiomatic; change `applyHeartbeatPatch` to read the current settings into a local variable before calling `setSettings` (e.g., get `previous` from the closure or a ref), then call `setSettings` with the merged object (`{ ...previous, ...patch }`) and handle the early-return when `previous` is null before mutating state; keep the existing `patchRequestIdRef`/`requestId`, `savingKey`, `setSaving`, and `setError` usage and ensure `setSaving(null)` is called only when `patchRequestIdRef.current === requestId`.
🤖 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/components/settings/panels/AIPanel.tsx`:
- Around line 601-614: The code currently assumes the first SPEND row is the
latest (latestSpend = spendRows[0]); instead, compute the latest spend from the
sample by using the timestamps in the SPEND rows (e.g., sort spend rows by
createdAt descending or pick the max createdAt via reduce) so latestSpend is the
transaction with the newest createdAt; update the code that uses latestSpend
(where it's derived from spendRows) to derive it from this explicit
latest-by-date logic (references: summarizeSpendSample, rows, times, spendRows,
latestSpend).
In `@src/openhuman/heartbeat/planner/collectors.rs`:
- Around line 154-163: The current .take(calendar_connection_limit) picks the
first N calendar connections in provider order and can starve later connections;
instead gather the filtered calendar connections into a Vec, compute a
rotation/shard offset (e.g., from a heartbeat tick counter or hash of the tick)
and then select calendar_connection_limit items starting at that offset with
wraparound so selection rotates across ticks; update the code around
connections.filter(...).take(...) to use this rotated selection (use the
existing symbols connections, normalized_toolkit, and
config.heartbeat.max_calendar_connections_per_tick) and ensure the tick counter
or seed used for offset is passed into or accessible by the collector so
selection changes each tick.
---
Nitpick comments:
In `@app/src/components/settings/panels/AIPanel.tsx`:
- Around line 737-755: The setSettings updater currently uses a side effect to
capture the previous settings (assigning to `previous` inside the `setSettings`
updater), which is non-idiomatic; change `applyHeartbeatPatch` to read the
current settings into a local variable before calling `setSettings` (e.g., get
`previous` from the closure or a ref), then call `setSettings` with the merged
object (`{ ...previous, ...patch }`) and handle the early-return when `previous`
is null before mutating state; keep the existing
`patchRequestIdRef`/`requestId`, `savingKey`, `setSaving`, and `setError` usage
and ensure `setSaving(null)` is called only when `patchRequestIdRef.current ===
requestId`.
In `@src/openhuman/config/schema/heartbeat_cron.rs`:
- Around line 87-103: Add a serde deserialization regression test alongside the
existing HeartbeatConfig::default() test that deserializes an empty (e.g. "{}")
and/or partial JSON payload into HeartbeatConfig and asserts the same opt-in
booleans and defaults: enabled, inference_enabled, notify_meetings,
notify_reminders, notify_relevant_events, external_delivery_enabled are all
false and max_calendar_connections_per_tick == 2 (and interval_minutes == 5);
use serde_json::from_str::<HeartbeatConfig>() to perform deserialization so the
test verifies missing keys are filled with the struct defaults rather than only
exercising HeartbeatConfig::default().
In `@src/openhuman/heartbeat/planner/collectors.rs`:
- Around line 154-163: Add a log statement that records the eligible vs selected
counts and the applied cap when capping calendar fanout: compute the count of
eligible connections by filtering connections with is_active() and
normalized_toolkit() matching "googlecalendar"/"google_calendar"/"calendar",
then log the eligible_count, the calendar_connection_limit value, and the number
actually taken (selected_count) before the .take(...) is applied; reference the
existing symbols calendar_connection_limit, connections, and
normalized_toolkit() and emit a grep-friendly tracing/log line (e.g.,
"calendar-fanout: eligible=%d cap=%d selected=%d") at debug/trace level at the
branching point.
🪄 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: 7a9201f9-692a-4ad3-86a6-613d79c14ea3
📒 Files selected for processing (10)
app/src/components/settings/panels/AIPanel.tsxapp/src/utils/tauriCommands/heartbeat.test.tsapp/src/utils/tauriCommands/heartbeat.tsapp/src/utils/tauriCommands/index.tssrc/openhuman/config/schema/heartbeat_cron.rssrc/openhuman/heartbeat/engine.rssrc/openhuman/heartbeat/planner/collectors.rssrc/openhuman/heartbeat/rpc.rssrc/openhuman/heartbeat/schemas.rssrc/openhuman/subconscious/global.rs
|
Fixed the latest CodeRabbit blockers in ac8613b.\n\n- Sorted SPEND rows before using latestSpend, so projections anchor to the newest ledger row instead of API order.\n- Replaced fixed calendar .take(N) with tick-bucket rotation across eligible active calendar connections, plus grep-friendly fanout log fields/message.\n- Removed the optimistic settings update side-effect pattern in applyHeartbeatPatch.\n- Added tests for heartbeat serde defaults, calendar selection rotation/filtering, AI panel diagnostics, optimistic controls, manual planner tick, and error states.\n\nLocal validation:\n- npm --prefix app run test:unit -- src/components/settings/panels/tests/AIPanel.test.tsx src/utils/tauriCommands/heartbeat.test.ts\n- npm --prefix app run compile\n- cargo test -p openhuman heartbeat_deserialization_fills_opt_in_defaults\n- cargo test -p openhuman calendar_selection_\n- diff-cover /tmp/lcov-app.info --compare-branch=origin/main --fail-under=80 => 94%\n- pre-push hook passed with existing warnings\n\n@coderabbitai comments resolved, please review again. |
|
(ノ◕ヮ◕)ノ*:・゚✧ ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/components/settings/panels/AIPanel.tsx`:
- Around line 643-649: The switch button in the LoopToggle (rendered in
AIPanel.tsx) lacks an accessible name; add an explicit accessible name by
setting either aria-label or aria-labelledby on the button element (e.g.,
aria-label={labelText} or aria-labelledby={labelId}) and thread a label prop or
id through the LoopToggle component so the switch announces its purpose to
screen readers; update the LoopToggle usage sites to pass a descriptive label
(or a matching label element id) and ensure aria-checked remains present.
In `@app/src/utils/tauriCommands/heartbeat.ts`:
- Around line 4-5: Change the import of CommandResponse to a type-only import to
follow TS guidelines: modify the import statement that currently brings in
CommandResponse alongside callCoreRpc and isTauri so that CommandResponse is
imported with `import type` while leaving callCoreRpc and isTauri as value
imports; update the import line referencing CommandResponse (used as a generic
and return type in functions such as the heartbeat call that uses callCoreRpc
and isTauri) 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: 80d01f3f-5838-4007-806b-39e3aa652fba
📒 Files selected for processing (11)
app/src/components/settings/panels/AIPanel.tsxapp/src/components/settings/panels/__tests__/AIPanel.test.tsxapp/src/utils/tauriCommands/heartbeat.test.tsapp/src/utils/tauriCommands/heartbeat.tsapp/src/utils/tauriCommands/index.tssrc/openhuman/config/schema/heartbeat_cron.rssrc/openhuman/heartbeat/engine.rssrc/openhuman/heartbeat/planner/collectors.rssrc/openhuman/heartbeat/rpc.rssrc/openhuman/heartbeat/schemas.rssrc/openhuman/subconscious/global.rs
|
Follow-up CodeRabbit nits fixed in 62308c9.\n\n- Added an accessible name to the background loop switch via aria-label.\n- Changed heartbeat command import to use an inline type-only CommandResponse import without tripping no-duplicate-imports.\n\nLocal validation:\n- npm --prefix app run test:unit -- src/components/settings/panels/tests/AIPanel.test.tsx src/utils/tauriCommands/heartbeat.test.ts\n- npm --prefix app run compile\n- eslint src/utils/tauriCommands/heartbeat.ts --cache\n- diff-cover /tmp/lcov-app.info --compare-branch=origin/main --fail-under=80 => 94%\n- pre-push hook passed with existing warnings\n\n@coderabbitai comments resolved, please review again. |
|
(ノ◕ヮ◕)ノ*:・゚✧ ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/openhuman/config/schema/heartbeat_cron.rs`:
- Around line 41-43: Normalize the max_calendar_connections_per_tick field on
deserialize so stored 0 can't slip through: add a custom deserializer (use
serde's deserialize_with) or implement Deserialize for the struct that reads the
incoming value for max_calendar_connections_per_tick and clamps/validates it
(e.g., if value < 1 then set to 1) instead of relying solely on #[serde(default
= "default_max_calendar_connections_per_tick")]; reference the existing field
name max_calendar_connections_per_tick and the default function
default_max_calendar_connections_per_tick when locating where to apply the
deserialize_with or custom Deserialize implementation.
In `@src/openhuman/heartbeat/planner/collectors.rs`:
- Around line 124-126: The rotation bucket uses a different minimum than the
scheduler sleep, causing misalignment; in the block computing
interval_seconds/tick_index/offset (referencing interval_seconds, tick_index,
offset) use the same effective minimum as HeartbeatEngine::run (i.e., clamp
interval_minutes with .max(5) instead of .max(1)) so the tick_index advances on
the actual 5-minute cadence and rotation stays aligned with the run loop.
🪄 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: 00ff9255-9ac6-4972-a15e-2015085e6d6e
📒 Files selected for processing (11)
app/src/components/settings/panels/AIPanel.tsxapp/src/components/settings/panels/__tests__/AIPanel.test.tsxapp/src/utils/tauriCommands/heartbeat.test.tsapp/src/utils/tauriCommands/heartbeat.tsapp/src/utils/tauriCommands/index.tssrc/openhuman/config/schema/heartbeat_cron.rssrc/openhuman/heartbeat/engine.rssrc/openhuman/heartbeat/planner/collectors.rssrc/openhuman/heartbeat/rpc.rssrc/openhuman/heartbeat/schemas.rssrc/openhuman/subconscious/global.rs
|
well done brother @vaddisrinivas great pr! |
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
Background
Users can hit the hosted weekly usage cap even when they do not remember sending enough chats to spend it. In the local repro, a fresh app install still showed the weekly cap exhausted because the cap lives in the backend ledger, not local app data. The recent ledger had many tiny
SPEND:USAGE_DEDUCTION:USERrows, but no feature/source tags, so the app could say “weekly limit hit” without showing which loop or tool path caused the spend.The risky part was background work. Heartbeat/proactive flows could start from an existing session, run immediately, poll calendar integrations, and run subconscious inference on a hosted route. Users had weak controls, no loop budget view, and no obvious way to pause the spend paths before credits were gone.
Summary
max_calendar_connections_per_tickRoot cause
Fresh installs bootstrapped heartbeat with planner categories and subconscious inference enabled. The loop ran immediately, kept a cloned config, and could continue even after settings changed. That made background calendar/subconscious work capable of burning hosted credits without a clear user-facing control or attribution view.
Separately, the hosted billing ledger does not yet include enough metadata to distinguish “chat spend” from “heartbeat calendar poll”, “subconscious tick”, “memory summarizer”, or “integration sync”. That means frontend users could not audit spend source from the app, and support/debugging had to infer from logs.
How this fixes it
GOOGLECALENDAR_EVENTS_LISTcalls per planner tick.Validation
Notes
The hosted billing ledger currently exposes action/time but not feature-level source tags. The UI calls that out and groups by action/hour until backend attribution metadata exists. A follow-up backend change should tag credit transactions with source loop, model/workload, request kind, provider/tool slug, and caller surface.
Summary by CodeRabbit
New Features
Behavior Changes
Tests