Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughExtended event retrieval windows from 28 to 100 days for both system and database queries in the Apple Calendar sync module. Removed four inline comments; execution order and behavior remain unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
1 issue found across 1 file • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| specific: ListEventFilterSpecific::DateRange { | ||
| start: Utc::now(), | ||
| end: Utc::now() + chrono::Duration::days(28), | ||
| end: Utc::now() + chrono::Duration::days(100), |
There was a problem hiding this comment.
The date-range has been extended from 28 days to 100 days while the limit field on the preceding ListEventFilterCommon is still hard-coded to 200. If more than 200 events exist in a 100-day window, newer events will be silently omitted, resulting in incomplete synchronisation data. Increase the limit or calculate it dynamically to match the wider range.
Prompt for AI agents
Address the following comment on plugins/apple-calendar/src/sync.rs at line 487:
<comment>The date-range has been extended from 28 days to 100 days while the `limit` field on the preceding `ListEventFilterCommon` is still hard-coded to 200. If more than 200 events exist in a 100-day window, newer events will be silently omitted, resulting in incomplete synchronisation data. Increase the limit or calculate it dynamically to match the wider range.</comment>
<file context>
@@ -484,7 +484,7 @@ async fn list_db_events(
},
specific: ListEventFilterSpecific::DateRange {
start: Utc::now(),
- end: Utc::now() + chrono::Duration::days(28),
+ end: Utc::now() + chrono::Duration::days(100),
},
}))
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
plugins/apple-calendar/src/sync.rs (3)
481-488: 200-event limit will miss data for a 100-day window, causing duplicate upserts and missed updatesWith a larger time range, fetching only 200 events risks not seeing existing DB events during reconciliation, which can lead to duplicate inserts or missed updates. Remove the limit (or significantly raise it) and reuse the same window constant as the system fetch.
Apply this diff:
common: ListEventFilterCommon { user_id: user_id.into(), - limit: Some(200), + // Fetch the full 100-day window to avoid missing events and creating duplicates + limit: None, }, specific: ListEventFilterSpecific::DateRange { start: Utc::now(), - end: Utc::now() + chrono::Duration::days(100), + end: Utc::now() + chrono::Duration::days(SYNC_WINDOW_DAYS), },
255-262: Avoid O(N^2) scans; use the handled_system_event_ids HashSet you already maintainThis path scans state.to_update for every system event. With 100 days, this scales poorly. Use handled_system_event_ids for O(1) membership checks (you already populate it when updating).
Apply this diff:
- // Skip if this event was already handled as an update - let already_handled = state - .to_update - .iter() - .any(|e| e.tracking_id == composite_tracking_id); - if already_handled { - continue; - } + // Skip if this event was already handled as an update + if handled_system_event_ids.contains(&composite_tracking_id) { + continue; + }
428-437: PII-heavy and high-volume logging; downgrade and redact for 100-day scaleLogging titles, participants, and IDs at info level for every event will be noisy and may leak PII. Use debug level and avoid dumping participant details.
Apply this diff:
- for event in &events { - tracing::info!( - "System event: '{}' | participants: {:?} | is_recurring: {} | start_date: {} | tracking_id: {}", - event.name, - event.participants, - event.is_recurring, - event.start_date, - event.id, - ); - } + for event in &events { + tracing::debug!( + "System event fetched | participants={} | is_recurring={} | start_date={}", + event.participants.len(), + event.is_recurring, + event.start_date, + ); + }
🧹 Nitpick comments (1)
plugins/apple-calendar/src/sync.rs (1)
506-513: N+1 DB lookups for sessions will not scale with a 100-day windowFetching sessions one-by-one per event will amplify latency and DB load. Prefer a batched API (e.g., list sessions by event IDs) or a join in list_events_with_sessions to eliminate the N+1 pattern.
I can sketch an API change for hypr_db_user to fetch sessions by a set of event IDs if that’s feasible on your side.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
plugins/apple-calendar/src/sync.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit Configuration File
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
plugins/apple-calendar/src/sync.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci (windows, windows-latest)
| let filter = EventFilter { | ||
| calendar_tracking_id: calendar_tracking_id.clone(), | ||
| from: now, | ||
| to: now + chrono::Duration::days(28), | ||
| to: now + chrono::Duration::days(100), | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consolidate the 100-day window into a single constant to keep system and DB queries in lockstep
Hard-coding 100 in multiple places is brittle. Define one constant and reuse it here and in the DB filter to prevent drift.
Apply this diff within the current block:
- to: now + chrono::Duration::days(100),
+ to: now + chrono::Duration::days(SYNC_WINDOW_DAYS),Add this near the top of the file (after imports) to support the change:
const SYNC_WINDOW_DAYS: i64 = 100;Run this to catch any other stale 28-day or newly hard-coded 100-day windows that should use the constant:
🏁 Script executed:
#!/bin/bash
# Find any lingering 28-day or hard-coded 100-day duration usages
rg -n -S $'Duration::days\\(\\s*28\\s*\\)|Duration::days\\(\\s*100\\s*\\)'Length of output: 651
Consolidate hard-coded 100-day window into a single constant (fix required)
rg found multiple hard-coded uses of Duration::days(100). Replace them with a single constant to avoid drift between system and DB queries.
Files/locations to update:
- plugins/apple-calendar/src/sync.rs — lines ~404 and ~487
- crates/calendar-apple/src/lib.rs — lines ~315–316
- crates/db-user/src/init.rs — lines ~738–739
Suggested changes (apply across the listed files):
- Prefer a single, shared constant if your workspace has a common/shared crate. Example (in that crate):
pub const SYNC_WINDOW_DAYS: i64 = 100;Then import and use it in other crates: chrono::Duration::days(SYNC_WINDOW_DAYS).
- If a shared crate isn't available, add a per-crate/file constant near the imports:
const SYNC_WINDOW_DAYS: i64 = 100;- Replace literals. Example diffs:
plugins/apple-calendar/src/sync.rs
- to: now + chrono::Duration::days(100),
+ to: now + chrono::Duration::days(SYNC_WINDOW_DAYS),plugins/apple-calendar/src/sync.rs (other occurrence)
- end: Utc::now() + chrono::Duration::days(100),
+ end: Utc::now() + chrono::Duration::days(SYNC_WINDOW_DAYS),crates/calendar-apple/src/lib.rs
- from: chrono::Utc::now() - chrono::Duration::days(100),
- to: chrono::Utc::now() + chrono::Duration::days(100),
+ from: chrono::Utc::now() - chrono::Duration::days(SYNC_WINDOW_DAYS),
+ to: chrono::Utc::now() + chrono::Duration::days(SYNC_WINDOW_DAYS),crates/db-user/src/init.rs
- created_at: now - chrono::Duration::days(100),
- visited_at: now - chrono::Duration::days(100),
+ created_at: now - chrono::Duration::days(SYNC_WINDOW_DAYS),
+ visited_at: now - chrono::Duration::days(SYNC_WINDOW_DAYS),After changes, re-run the search to confirm no remaining literals:
rg -n -S $'Duration::days\(\s100\s\)'
🤖 Prompt for AI Agents
In plugins/apple-calendar/src/sync.rs around lines 401–405 and ~487,
crates/calendar-apple/src/lib.rs around lines 315–316, and
crates/db-user/src/init.rs around lines 738–739, replace the hard-coded
chrono::Duration::days(100) occurrences with a single named constant; preferably
add a shared constant pub const SYNC_WINDOW_DAYS: i64 = 100 in a common
workspace crate and import it where needed, using
chrono::Duration::days(SYNC_WINDOW_DAYS), or if no shared crate is available
declare const SYNC_WINDOW_DAYS: i64 = 100 near the file imports in each crate
and use that constant similarly; update the imports/usages accordingly and
re-run rg -n -S $'Duration::days\\(\\s*100\\s*\\)' to confirm no remaining
literals.
1 participant
Summary by cubic
Expanded calendar event syncing and listing to include events up to 100 days into the future instead of 28 days.