packages/lib/date-ranges.ts (1)

128-176: Heavy Day.js use inside nested loops

The newly-added merge loop performs multiple dayjs.min/max calls and valueOf() conversions per existing range. Per team guidelines, prefer native timestamps or Day.js .utc() only when needed in performance-critical loops. Caching numeric epoch values before the loop will cut object allocations significantly.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops

Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/lib/date-ranges.ts (2)

128-169: O(N²) complexity will degrade performance with many intervals

This nested loop iterates through all existing results for each new range, resulting in O(N²) complexity. Combined with Day.js object creation on every iteration (lines 138-139), this becomes a performance bottleneck as the number of intervals grows.

Consider:

1. Using a sorted data structure (e.g., interval tree or sorted array with binary search) to find overlaps in O(log N)
2. Pre-computing numeric timestamps with .valueOf() before the loop to avoid repeated Day.js instantiation
3. Using native Date comparisons in the critical path

-    let foundOverlapOrAdjacent = false;
-    for (const existingKey in results) {
-      const existingRange = results[existingKey];
-
-      const isOverlapping =
-        (startResult.valueOf() <= existingRange.end.valueOf() &&
-          endResult.valueOf() >= existingRange.start.valueOf()) ||
-        Math.abs(startResult.valueOf() - existingRange.end.valueOf()) <= 15 * 60 * 1000 ||
-        Math.abs(endResult.valueOf() - existingRange.start.valueOf()) <= 15 * 60 * 1000;
+    // Pre-compute timestamps outside the loop
+    const startValue = startResult.valueOf();
+    const endValue = endResult.valueOf();
+    
+    let foundOverlapOrAdjacent = false;
+    // TODO: Replace with sorted structure for O(log N) lookup
+    for (const existingKey in results) {
+      const existingRange = results[existingKey];
+      const existingStartValue = existingRange.start.valueOf();
+      const existingEndValue = existingRange.end.valueOf();
+
+      const isOverlapping =
+        (startValue <= existingEndValue && endValue >= existingStartValue) ||
+        Math.abs(startValue - existingEndValue) <= 15 * 60 * 1000 ||
+        Math.abs(endValue - existingStartValue) <= 15 * 60 * 1000;

---

157-161: Duplicate keys accumulating in endTimeToKeyMap

Line 160 pushes newKey to the array without checking for duplicates, causing the same key to be added multiple times during repeated merges.

Apply the same Set-based deduplication used elsewhere in the file:

-          if (!endTimeToKeyMap.has(newKey)) {
-            endTimeToKeyMap.set(newKey, []);
-          }
-          endTimeToKeyMap.get(newKey)!.push(newKey);
+          const keySet = new Set(endTimeToKeyMap.get(newKey) || []);
+          keySet.add(newKey);
+          endTimeToKeyMap.set(newKey, Array.from(keySet));

packages/lib/date-ranges.ts (1)

135-136: Consider making the 15-minute adjacency threshold configurable

The hardcoded 15-minute threshold for considering ranges as adjacent might need to be adjustable based on business requirements.

Consider extracting this as a constant or configuration parameter:

+// Threshold in milliseconds for considering ranges as adjacent
+const ADJACENCY_THRESHOLD_MS = 15 * 60 * 1000;
+
 const isOverlapping =
   (startResult.valueOf() <= existingRange.end.valueOf() &&
     endResult.valueOf() >= existingRange.start.valueOf()) ||
-  Math.abs(startResult.valueOf() - existingRange.end.valueOf()) <= 15 * 60 * 1000 ||
-  Math.abs(endResult.valueOf() - existingRange.start.valueOf()) <= 15 * 60 * 1000;
+  Math.abs(startResult.valueOf() - existingRange.end.valueOf()) <= ADJACENCY_THRESHOLD_MS ||
+  Math.abs(endResult.valueOf() - existingRange.start.valueOf()) <= ADJACENCY_THRESHOLD_MS;

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops

packages/lib/date-ranges.ts (2)

128-178: O(N²) complexity remains unaddressed despite past review feedback

The nested loop over results object (lines 134-173) creates O(N²) time complexity as flagged in previous reviews. With multiple Day.js operations inside the loop (dayjs.min, dayjs.max), this will degrade performance significantly as the number of working hour intervals grows.

While timestamps are now pre-computed (lines 129-130), the fundamental algorithmic issue persists.

Consider refactoring to use a sorted data structure:

-    // TODO: Replace with sorted structure for O(log N) lookup
-    for (const existingKey in results) {
+    // Use sorted array or Map for O(log N) overlap detection
+    const sortedRanges = Object.entries(results)
+      .map(([key, range]) => ({ key: Number(key), ...range, startValue: range.start.valueOf(), endValue: range.end.valueOf() }))
+      .sort((a, b) => a.startValue - b.startValue);
+    
+    // Binary search for overlaps instead of linear scan

---

144-145: Excessive Day.js usage in performance-critical loop

The loop repeatedly calls dayjs.min() and dayjs.max() for each overlap check, creating unnecessary Day.js object instantiation in a hot path. This violates the coding guideline about excessive Day.js use in performance-critical code.

Use pre-computed numeric timestamps for comparisons:

-        const mergedStart = dayjs.min(existingRange.start, startResult);
-        const mergedEnd = dayjs.max(existingRange.end, endResult);
+        const mergedStartValue = Math.min(existingStartValue, startValue);
+        const mergedEndValue = Math.max(existingEndValue, endValue);
+        
+        const mergedStart = mergedStartValue === existingStartValue ? existingRange.start : startResult;
+        const mergedEnd = mergedEndValue === existingEndValue ? existingRange.end : endResult;

packages/lib/date-ranges.ts (1)

139-142: Adjacency threshold hardcoded without configuration

The 15-minute adjacency threshold (15 * 60 * 1000) is hardcoded, making it difficult to adjust for different scheduling requirements or business rules.

Extract the threshold to a configurable parameter:

+const ADJACENCY_THRESHOLD_MS = 15 * 60 * 1000; // 15 minutes

   const isOverlapping =
     (startValue <= existingEndValue && endValue >= existingStartValue) ||
-    Math.abs(startValue - existingEndValue) <= 15 * 60 * 1000 ||
-    Math.abs(endValue - existingStartValue) <= 15 * 60 * 1000;
+    Math.abs(startValue - existingEndValue) <= ADJACENCY_THRESHOLD_MS ||
+    Math.abs(endValue - existingStartValue) <= ADJACENCY_THRESHOLD_MS;

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops

packages/lib/date-ranges.ts (1)

175-180: Similar duplicate key concern for new interval insertion

The same duplicate key issue exists when adding completely new intervals.

Apply the same fix as suggested above:

  if (!endTimeToKeyMap.has(endTimeKey)) {
    endTimeToKeyMap.set(endTimeKey, []);
  }
- endTimeToKeyMap.get(endTimeKey)!.push(newKey);
+ const keys = endTimeToKeyMap.get(endTimeKey)!;
+ if (!keys.includes(newKey)) {
+   keys.push(newKey);
+ }

packages/lib/date-ranges.ts (2)

32-183: Performance concern: O(N²) complexity in interval merging

The nested loop checking all existing results for each new interval creates O(N²) complexity. Combined with repeated Day.js object instantiation in the hot path, this will degrade performance as schedules grow.

Consider these optimizations:

1. Use a sorted data structure (e.g., sorted array with binary search) for O(log N) overlap detection
2. Cache .valueOf() results outside loops to avoid repeated Day.js instantiation
3. Consider native Date comparisons in critical sections

// At the beginning of processWorkingHours, create sorted structure
+ const sortedIntervals = new Map(); // sorted by start time
+ 
  for (let date = dateFrom.startOf("day"); utcDateTo.isAfter(date); date = date.add(1, "day")) {
    // ... existing code ...
    
+   const startValue = startResult.valueOf();
+   const endValue = endResult.valueOf();
    
-   for (const key of keysWithSameEndTime) {
-     const existingRange = results[key];
-     if (
-       startResult.valueOf() <= existingRange.end.valueOf() &&
-       endResult.valueOf() >= existingRange.start.valueOf()
-     ) {
+   // Use binary search or interval tree for O(log N) overlap detection
+   const overlappingKey = findOverlappingInterval(sortedIntervals, startValue, endValue);
+   if (overlappingKey !== null) {
      // ... merge logic ...
    }

---

149-163: Potential for duplicate keys in endTimeToKeyMap

When newKey equals endTimeKey, the same key could be added multiple times to the array if this code path is hit repeatedly.

Use a Set to prevent duplicates:

- if (!endTimeToKeyMap.has(endTimeKey)) {
-   endTimeToKeyMap.set(endTimeKey, []);
- }
- endTimeToKeyMap.get(endTimeKey)!.push(newKey);
+ const keySet = endTimeToKeyMap.get(endTimeKey) ?? new Set<number>();
+ keySet.add(newKey);
+ endTimeToKeyMap.set(endTimeKey, Array.from(keySet));

Or check before adding:

  if (!endTimeToKeyMap.has(endTimeKey)) {
    endTimeToKeyMap.set(endTimeKey, []);
  }
- endTimeToKeyMap.get(endTimeKey)!.push(newKey);
+ const keys = endTimeToKeyMap.get(endTimeKey)!;
+ if (!keys.includes(newKey)) {
+   keys.push(newKey);
+ }

packages/lib/date-ranges.ts (1)

86-95: Lazy initialization of endTimeToKeyMap could be optimized

The lazy initialization rebuilds the entire map on first overlap check. For better performance, consider maintaining this map throughout the function lifecycle.

Initialize the map once at the beginning:

  const utcDateTo = dateTo.utc();
- let endTimeToKeyMap: Map<number, number[]> | undefined;
+ const endTimeToKeyMap = new Map<number, number[]>();
+ 
+ // Build initial map from existing results
+ for (const [key, range] of Object.entries(results)) {
+   const endTime = range.end.valueOf();
+   if (!endTimeToKeyMap.has(endTime)) {
+     endTimeToKeyMap.set(endTime, []);
+   }
+   endTimeToKeyMap.get(endTime)!.push(Number(key));
+ }

  for (let date = dateFrom.startOf("day"); utcDateTo.isAfter(date); date = date.add(1, "day")) {
    // ... existing code ...
    
-   if (!endTimeToKeyMap) {
-     endTimeToKeyMap = new Map<number, number[]>();
-     for (const [key, range] of Object.entries(results)) {
-       const endTime = range.end.valueOf();
-       if (!endTimeToKeyMap.has(endTime)) {
-         endTimeToKeyMap.set(endTime, []);
-       }
-       endTimeToKeyMap.get(endTime)!.push(Number(key));
-     }
-   }

Configuration used: CodeRabbit UI
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.

Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops

packages/lib/date-ranges.ts (1)

337-341: Missing key insertion after creating map entry

When endTimeToKeyMap exists and a new end time key is created, the code only creates an empty array but doesn't add the newKey to it. This breaks the invariant that the map should track all keys with that end time.

Apply this fix:

     if (endTimeToKeyMap) {
       if (!endTimeToKeyMap.has(endTimeKey)) {
         endTimeToKeyMap.set(endTimeKey, []);
       }
+      const keySet = new Set(endTimeToKeyMap.get(endTimeKey) || []);
+      keySet.add(newKey);
+      endTimeToKeyMap.set(endTimeKey, Array.from(keySet));
     }

packages/lib/date-ranges.ts (1)

322-324: Good fix for duplicate key issue

Using a Set to deduplicate keys before converting back to an array properly addresses the duplicate accumulation issue.

Configuration used: CodeRabbit UI
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 sources in your CodeRabbit configuration.

Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops

packages/lib/date-ranges.ts (1)

229-233: Missing: new result never added to endTimeToKeyMap in the fresh-insert path

You create the bucket but don’t add newKey, so subsequent same-end-time lookups won’t find it.

   if (endTimeToKeyMap) {
     if (!endTimeToKeyMap.has(endTimeKey)) {
       endTimeToKeyMap.set(endTimeKey, []);
     }
+    const keySet = new Set(endTimeToKeyMap.get(endTimeKey) || []);
+    keySet.add(newKey);
+    endTimeToKeyMap.set(endTimeKey, Array.from(keySet));
   }

packages/lib/date-ranges.ts (1)

137-158: O(N²) scan across results on every insert; consider a sorted structure or bucketed search

This still linearly scans all results for each candidate. With large schedules it’ll degrade. Consider a sorted array + binary search or an interval tree to find only potentially overlapping/adjacent ranges.

If you want a low-effort step-up: maintain two indexes in parallel:

• byEnd: Map<endValue, number[]> (you already have)
• byStart: a sorted array of [startValue, key] to binary-search first candidate where start <= endValue + threshold, then scan forward until start > endValue + threshold.

packages/lib/date-ranges.ts (2)

106-116: Simplify same-end-time merge to a single assignment

The if/else is redundant; min() already covers both branches and avoids duplicate object literals.

-        if (startResult.valueOf() < existingRange.start.valueOf()) {
-          results[key] = {
-            start: dayjs.min(existingRange.start, startResult),
-            end: endResult,
-          };
-        } else {
-          results[key] = {
-            start: existingRange.start,
-            end: endResult,
-          };
-        }
+        results[key] = {
+          start: dayjs.min(existingRange.start, startResult),
+          end: endResult,
+        };

---

130-130: Consider hoisting ADJACENCY_THRESHOLD_MS to module scope

Minor: defined inside the loop, it’s re-created per candidate. Hoisting reduces churn and clarifies intent.

packages/lib/date-ranges.test.ts (1)

153-153: Verify why last slot end shrank to 18:29:59.999Z on 2023-11-30 (America/New_York)

This is a notable change from 22:00:00.000Z to 18:29:59.999Z. If this comes from adjacency/DST gating, confirm it’s expected product behavior and not masking a regression in processWorkingHours’ offset math.

If intended, please add a brief inline comment in the test to document the rationale. I can also add a focused unit test for the overlapping-hours case from the PR description (e.g., 09:00–15:30 and 15:15–17:00 on the same day) to prevent regressions. Want me to draft it?

Configuration used: CodeRabbit UI
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 sources in your CodeRabbit configuration.

Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops

packages/lib/date-ranges.ts (4)

126-135: Adjacency policy: confirm 15 minutes is intended behavior

Hard-coding 15 minutes will merge gaps up to 15 min (not only overlaps). Verify with product/spec; consider tying this to slot length or making it configurable.

---

137-145: Avoid entries() allocation in hot loop

Minor perf: iterate keys directly to skip Object.entries() array creation.

-    for (const [key, range] of Object.entries(results)) {
-      const rangeKey = Number(key);
-      const rangeStartValue = range.start.valueOf();
-      const rangeEndValue = range.end.valueOf();
+    for (const k in results) {
+      const rangeKey = Number(k);
+      const range = results[rangeKey];
+      const rangeStartValue = range.start.valueOf();
+      const rangeEndValue = range.end.valueOf();

---

137-158: Potential O(N²): consider ordered structure for overlaps

Scanning all ranges per insertion is O(N²). If this runs on large schedules, consider a sorted array + binary search (or interval tree) to locate the first candidate with end >= startValue - threshold, then sweep forward only while start <= endValue + threshold.

---

196-225: Redundant same-start merge branch

The global merge above already handles overlaps/adjacency. This branch adds complexity and duplicate code paths.

-    if (results[startResult.valueOf()]) {
-      const oldKey = startResult.valueOf();
-      const newKey = endResult.valueOf();
-      results[newKey] = {
-        start: results[oldKey].start,
-        end: dayjs.max(results[oldKey].end, endResult),
-      };
-      if (endTimeToKeyMap) {
-        const oldEndTime = results[oldKey].end.valueOf();
-        const oldKeys = endTimeToKeyMap.get(oldEndTime) || [];
-        const filteredKeys = oldKeys.filter((k) => k !== oldKey);
-        if (filteredKeys.length === 0) {
-          endTimeToKeyMap.delete(oldEndTime);
-        } else {
-          endTimeToKeyMap.set(oldEndTime, filteredKeys);
-        }
-        if (!endTimeToKeyMap.has(endTimeKey)) {
-          endTimeToKeyMap.set(endTimeKey, []);
-        }
-        const keySet = new Set(endTimeToKeyMap.get(endTimeKey) || []);
-        keySet.add(newKey);
-        endTimeToKeyMap.set(endTimeKey, Array.from(keySet));
-      }
-      delete results[oldKey];
-      continue;
-    }
+    // Removed redundant same-start merge; covered by the global merge path

Configuration used: CodeRabbit UI

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 sources in your CodeRabbit configuration.

Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops