Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| occurrences.push({ | ||
| ...hijri, | ||
| gregorian: new Date(cursor.getTime()), | ||
| reminder: addDays(cursor, -3), | ||
| }); |
There was a problem hiding this comment.
Populate Hijri fields expected by timeline consumers
The occurrences pushed in getUpcomingHolyDays expose the Hijri day, month, and year under generic keys day, month, and year, but downstream code and tests read hijriDay, hijriMonth, and hijriYear. This leaves the timeline rendering undefined values and causes the unit tests that assert result.map(item => item.hijriDay) to fail. The objects returned here should either rename the fields or provide aliases so the expected properties exist.
Useful? React with 👍 / 👎.
| if (!targetMonth) { | ||
| if (hijri.day === HOLY_DAYS[0]) { | ||
| targetMonth = { ...hijri }; | ||
| } |
There was a problem hiding this comment.
Skip remaining holy nights when reference date is mid-Ramadan
The search for upcoming nights only initializes targetMonth once the loop encounters a day equal to HOLY_DAYS[0] (17 Ramadan). If the reference date is already after the 17th but before the 21st of the current Ramadan, targetMonth remains null until the loop reaches the next year’s Ramadan, so the function returns the following year’s 17/19/21 and never lists the still-upcoming 21 of the current cycle. Consider detecting that the reference date is already inside Ramadan and collecting the remaining nights without waiting for another 17.
Useful? React with 👍 / 👎.
Summary
Testing
https://chatgpt.com/codex/tasks/task_b_690262c3093083289fedfb6b0c6c632d