fix: Fix Digest scheduling logic + update tests#584
Conversation
|
@edulelis is attempting to deploy a commit to the Inbox Zero Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedThe pull request is closed. WalkthroughThe changes update scheduling logic to exclude the current date if the slot time matches exactly and extend the weekly search window to 14 days. The test suite was enhanced to be timezone-independent with UTC dates and expanded with comprehensive monthly and long-interval scheduling tests. The AI digest API route was refactored to add middleware for error handling and signature verification, and a redundant simple route wrapper was removed. The digest enqueue function was adjusted to omit custom cron secret headers. Changes
Estimated code review effort4 (60–120 minutes) Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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 (1)
apps/web/utils/schedule.test.ts (1)
314-333: Critical: Test logic is inconsistent with the scheduling changes.The test description says "should find next occurrence on same day even if time has passed" but the logic change in
schedule.ts(line 166) now uses>instead of>=, which means it will NOT schedule on the same day if the time has passed - it will find the next occurrence.The test expectation (line 330) expects date 15 (same day) but the pipeline failure shows it gets 22 (next week), which is actually the correct behavior with the new logic.
The test should be updated to match the new logic:
- it("should find next occurrence on same day even if time has passed", () => { + it("should find next occurrence when current time has passed", () => { const fromDate = new Date("2024-01-15T12:00:00Z"); // Monday 12 PM const timeOfDay = createCanonicalTimeOfDay(10, 0); const result = calculateNextScheduleDate( { intervalDays: null, daysOfWeek: DAYS.MONDAY, timeOfDay, occurrences: null, }, fromDate, ); - // Current time is 12 PM UTC, but 10 AM scheduled time has already passed today, so schedule for same Monday at 10:00 AM + // Current time is 12 PM UTC, and 10 AM scheduled time has already passed today, so schedule for next Monday at 10:00 AM expect(result).not.toBeNull(); - expect(result!.getDate()).toBe(15); // Same Monday + expect(result!.getDate()).toBe(22); // Next Monday expect(result!.getHours()).toBe(10); expect(result!.getMinutes()).toBe(0); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/utils/schedule.test.ts(15 hunks)apps/web/utils/schedule.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
apps/web/**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- apps/web/CLAUDE.md
!{.cursor/rules/*.mdc}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/cursor-rules.mdc
**/*.ts
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/form-handling.mdc
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/logging.mdc
- .cursor/rules/prisma.mdc
- .cursor/rules/ultracite.mdc
apps/web/utils/**
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/project-structure.mdc
apps/web/utils/**/*.ts
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/project-structure.mdc
**/*.{js,jsx,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/ultracite.mdc
!pages/_document.{js,jsx,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/ultracite.mdc
**/*.test.{ts,js}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/security.mdc
**/*.test.{ts,js,tsx,jsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/testing.mdc
**/*.{test,spec}.{js,jsx,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/ultracite.mdc
🧠 Learnings (2)
apps/web/utils/schedule.ts (5)
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-07-18T15:06:40.243Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Format data consistently across similar LLM-related functions.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-07-18T15:06:40.243Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Make Zod schemas as specific as possible to guide the LLM output.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-07-18T15:06:40.243Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Extract common patterns into utility functions in LLM-related code.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-07-18T15:06:40.243Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Implement fallbacks for AI failures in LLM-related functions.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-07-18T15:06:40.243Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Include relevant context in log messages for LLM-related code.
apps/web/utils/schedule.test.ts (13)
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.554Z
Learning: Applies to apps/web/tests/**/*.test.ts : Test both AI and non-AI paths in LLM-related tests
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.554Z
Learning: Applies to apps/web/tests/**/*.test.ts : Include standard test cases: happy path, error handling, edge cases (empty input, null values), different user configurations, and various input formats in LLM-related tests
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.554Z
Learning: Applies to apps/web/tests/**/*.test.ts : Use descriptive console.debug statements for generated content in LLM-related tests
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.554Z
Learning: Applies to apps/web/tests/**/*.test.ts : Set appropriate timeouts for LLM calls in tests (e.g., 15,000ms for long-running operations)
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-20T09:03:06.287Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Make sure the assertion function, like expect, is placed inside an it() function call.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.554Z
Learning: Applies to apps/web/tests/**/*.test.ts : Always create helper functions for common test data in LLM-related tests
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-07-18T15:06:40.243Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Document complex AI logic with clear comments in LLM-related code.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-20T09:03:06.287Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Don't nest describe() blocks too deeply in test files.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-07-18T15:06:40.243Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Format data consistently across similar LLM-related functions.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-20T09:03:06.287Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Don't use disabled tests.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-06-23T12:27:30.570Z
Learning: When mocking Prisma in Vitest, import the Prisma mock from '@/utils/mocks/prisma', mock '@/utils/prisma', and clear all mocks in a beforeEach hook to ensure test isolation.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-20T09:03:06.287Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Don't use export or module.exports in test files.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-20T09:03:06.287Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Don't use export or module.exports in test files.
🪛 GitHub Check: test
apps/web/utils/schedule.test.ts
[failure] 330-330: utils/schedule.test.ts > calculateNextScheduleDate > weekly pattern with specific days > should find next occurrence on same day even if time has passed
AssertionError: expected 22 to be 15 // Object.is equality
- Expected
- Received
- 15
- 22
❯ utils/schedule.test.ts:330:33
[failure] 581-581: utils/schedule.test.ts > calculateNextScheduleDate > real-world scenarios > should handle monthly schedule on the 15th
AssertionError: expected 1 to be +0 // Object.is equality
- Expected
- Received
- 0
- 1
❯ utils/schedule.test.ts:581:34
🪛 GitHub Actions: Run Tests
apps/web/utils/schedule.test.ts
[error] 330-330: Test failure in calculateNextScheduleDate > weekly pattern with specific days > should find next occurrence on same day even if time has passed: AssertionError - expected 22 to be 15 (Object.is equality).
[error] 581-581: Test failure in calculateNextScheduleDate > real-world scenarios > should handle monthly schedule on the 15th: AssertionError - expected 1 to be 0 (Object.is equality).
⏰ 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)
- GitHub Check: Static Code Analysis Js
- GitHub Check: Jit Security
🔇 Additional comments (5)
apps/web/utils/schedule.ts (2)
166-166: LGTM! Logic change correctly excludes exact time matches.The change from
>=to>prevents scheduling at the exact same time asfromDate, which is the correct behavior for finding the "next" occurrence.
182-183: LGTM! Extended search window improves scheduling flexibility.Extending the search window from 7 to 14 days allows the function to find weekly occurrences across a two-week period, which is more robust for edge cases.
apps/web/utils/schedule.test.ts (3)
1-1: Import reordering looks good.The reordering of imports maintains alphabetical order and readability.
694-757: LGTM! Comprehensive test coverage for long intervals.The new test cases provide excellent coverage for edge cases including:
- Very long intervals (365 and 1000 days)
- Multiple occurrences within long intervals
- Leap year handling
- Year boundary crossing
These tests validate the robustness of the scheduling algorithm for extreme scenarios.
207-212: Comment improvements enhance test clarity.The updates to test comments, replacing "local time" with "AM" and improving time descriptions, make the test intentions much clearer and more precise.
Also applies to: 284-289, 307-312, 328-333, 349-354, 369-374, 389-394, 410-415, 432-438, 453-460, 494-499, 517-522, 538-543, 558-563
2 participants
Summary by CodeRabbit
Bug Fixes
Tests
API Enhancements
Chores