Improve scheduling algorithm #3126
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces support for locked events and task splitting within the scheduling application. It modifies the scheduling algorithm, UI components, and data types to handle locked events, allow marking tasks as splittable, and visually distinguish locked events. The changes include new UI controls, updated scheduling logic, and expanded test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SchedulerPage
participant LockedEventForm
participant SchedulingAlgorithm
User->>SchedulerPage: Open "Add Locked Event" modal
SchedulerPage->>LockedEventForm: Render form
User->>LockedEventForm: Submit locked event details
LockedEventForm->>SchedulerPage: Add locked event to state
User->>SchedulerPage: Click "Schedule"
SchedulerPage->>SchedulingAlgorithm: Call scheduleTasks(tasks, activeHours, lockedEvents)
SchedulingAlgorithm->>SchedulerPage: Return schedule including locked events
SchedulerPage->>User: Display schedule with locked events visually distinguished
sequenceDiagram
participant User
participant TaskModal
participant SchedulerPage
participant SchedulingAlgorithm
User->>TaskModal: Open "Add Task" modal
TaskModal->>User: Show "Allow split" toggle
User->>TaskModal: Set allowSplit and submit
TaskModal->>SchedulerPage: Add task with allowSplit property
User->>SchedulerPage: Click "Schedule"
SchedulerPage->>SchedulingAlgorithm: Call scheduleTasks(tasks with allowSplit)
SchedulingAlgorithm->>SchedulerPage: Return scheduled events (split if allowed)
SchedulerPage->>User: Display scheduled tasks (split as needed)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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. 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
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @datnguyen-en, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the scheduling functionality by introducing the ability to define and manage 'locked events' that the scheduler must respect, and by providing granular control over whether individual tasks can be split into multiple sessions. These changes improve the flexibility and realism of the scheduling algorithm, supported by corresponding updates to the user interface for task management and event display.
Highlights
- Core Scheduling Algorithm Enhancement: The
scheduleTasksalgorithm now supports 'locked events' which are pre-existing, unmovable time blocks that the scheduler must work around. It also introduces a newallowSplitproperty for tasks, enabling tasks to be scheduled either as a single continuous block or split into multiple sessions based on user preference. - User Interface for Locked Events: A new section and modal have been added to the scheduler page allowing users to define and manage 'locked events' directly within the UI. These events are visually represented in the schedule display with a lock icon and specific styling.
- Task Splittability Control: The task creation/editing modal and the task list display now include a toggle to control whether a task can be split into multiple sessions. This
allowSplitproperty is integrated into the task data model. - Improved Scheduling Logic: The algorithm has been refined to handle minimum and maximum duration constraints more effectively for both splittable and non-splittable tasks, including better handling of deadlines and logging of scheduling outcomes (e.g., tasks split, tasks unscheduled).
- New Test Cases: Comprehensive unit tests have been added for the
schedulefunction to validate the newallowSplitfunctionality, ensuring non-splittable tasks are scheduled correctly and splittable tasks are split when necessary.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces 'locked events' and 'task splitting control' (allowSplit). The UI has been updated, and the core scheduling algorithm has been refactored to accommodate these new constraints. Overall, the changes are well-implemented and enhance the scheduler's capabilities.
| </div> | ||
| <div> | ||
| <Label>Category</Label> | ||
| <Select value={category} onValueChange={(v) => setCategory(v as any)}> |
There was a problem hiding this comment.
The type assertion v as any for setting the category should be avoided for better type safety. Validate the value before casting.
| event.locked | ||
| ? 'border-dynamic-blue/30 bg-dynamic-blue/10' | ||
| : event.isPastDeadline | ||
| ? 'border-destructive/30 bg-destructive/5' | ||
| : 'hover:bg-accent/5' |
There was a problem hiding this comment.
The conditional logic for className is becoming complex. Consider extracting this logic into a helper function or using a utility like clsx to improve readability.
| <SplitIcon className="h-4 w-4 text-primary" /> | ||
| <Switch | ||
| id={`split-toggle-${task.id}`} | ||
| checked={task.allowSplit ?? true} |
There was a problem hiding this comment.
The default value for task.allowSplit is true using the nullish coalescing operator (??). This is a good default, making tasks splittable unless explicitly set otherwise.
| id: `locked-${Date.now()}`, | ||
| locked: true, | ||
| taskId: 'locked', |
There was a problem hiding this comment.
Using taskId: 'locked' for locked events might lead to confusion if taskId is generally expected to be an ID of an actual task. Consider using null or undefined.
| <button | ||
| className="ml-auto rounded bg-dynamic-blue px-3 py-1 text-xs text-white transition hover:bg-dynamic-blue/80" | ||
| onClick={() => setIsLockedEventModalOpen(true)} | ||
| > | ||
| Add Locked Event | ||
| </button> |
There was a problem hiding this comment.
Consider replacing the button element with the imported Button component from @tuturuuu/ui/button for consistency.
<Button
variant="outline"
className="ml-auto bg-dynamic-blue px-3 py-1 text-xs text-white transition hover:bg-dynamic-blue/80"
onClick={() => setIsLockedEventModalOpen(true)}
>
Add Locked Event
</Button>| // Start with locked events in the schedule | ||
| const scheduledEvents: Event[] = lockedEvents.map((e) => ({ | ||
| ...e, | ||
| locked: true, | ||
| })); |
There was a problem hiding this comment.
Initializing scheduledEvents with lockedEvents ensures these events are respected by the algorithm from the start. Marking them with locked: true is also clear.
| // Prepare a working pool of tasks with remaining duration | ||
| const taskPool = tasks.map((task) => ({ | ||
| ...task, | ||
| duration: hoursToQuarterHours(task.duration), | ||
| minDuration: ensureMinimumDuration(task.minDuration), | ||
| maxDuration: hoursToQuarterHours(task.maxDuration), | ||
| remaining: hoursToQuarterHours(task.duration), | ||
| nextPart: 1, | ||
| scheduledParts: 0, | ||
| })); |
There was a problem hiding this comment.
The creation of taskPool with pre-calculated quarter-hour durations and tracking properties is a good refactoring, centralizing task state management.
| // Non-splittable: try to schedule as a single block | ||
| if (task.allowSplit === false) { | ||
| if (task.scheduledParts > 0) continue; // Already tried | ||
| let scheduled = false; | ||
| let tryTime = availableTimes[task.category]; | ||
| let blockAttempts = 0; | ||
| while (!scheduled && blockAttempts < 50) { | ||
| blockAttempts++; | ||
| const availableSlots = getAvailableSlots( | ||
| tryTime, | ||
| categoryHours, | ||
| scheduledEvents | ||
| ); | ||
| const slot = availableSlots.find((slot) => { | ||
| const slotDuration = slot.end.diff(slot.start, 'hour', true); | ||
| return slotDuration >= task.duration; | ||
| }); | ||
| if (slot) { | ||
| const partStart = roundToQuarterHour(slot.start, false); | ||
| const partEnd = partStart.add(task.duration, 'hour'); | ||
| const newEvent: Event = { | ||
| id: `event-${task.id}`, | ||
| name: task.name, | ||
| range: { start: partStart, end: partEnd }, | ||
| isPastDeadline: false, | ||
| taskId: task.id, | ||
| }; | ||
| if (task.deadline && partEnd.isAfter(task.deadline)) { | ||
| newEvent.isPastDeadline = true; | ||
| logs.push({ | ||
| type: 'warning', | ||
| message: `Task "${task.name}" is scheduled past its deadline of ${task.deadline.format('YYYY-MM-DD HH:mm')}.`, | ||
| }); | ||
| } | ||
| scheduledEvents.push(newEvent); | ||
| availableTimes[task.category] = roundToQuarterHour(partEnd, true); | ||
| scheduled = true; | ||
| task.remaining = 0; | ||
| task.scheduledParts = 1; | ||
| anyScheduled = true; | ||
| } else { | ||
| tryTime = tryTime.add(1, 'day').startOf('day'); | ||
| tryTime = getNextAvailableTime(categoryHours, tryTime); | ||
| logs.push({ | ||
| type: 'warning', | ||
| message: `Moving task "${task.name}" to ${tryTime.format('YYYY-MM-DD')} due to lack of available time slots for non-splittable task.`, | ||
| }); | ||
| } | ||
| } | ||
| if (!scheduled) { | ||
| logs.push({ | ||
| type: 'error', | ||
| message: `Task "${task.name}" could not be scheduled as a single block after 50 attempts.`, | ||
| }); | ||
| task.remaining = 0; | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
The logic for handling non-splittable tasks (task.allowSplit === false) is well-defined, ensuring that non-splittable tasks are either scheduled whole or not at all.
| logs.push({ | ||
| type: 'error', | ||
| message: `Cannot fit minimum duration for task "${task.name}" part ${task.nextPart}.`, | ||
| }); | ||
| task.remaining = 0; | ||
| break; | ||
| } | ||
| partDuration = hoursToQuarterHours(adjustedDuration); | ||
| } | ||
| const finalPartEnd = partStart.add(partDuration, 'hour'); | ||
| // If this part would end after the deadline, skip scheduling | ||
| if (task.deadline && finalPartEnd.isAfter(task.deadline)) { | ||
| logs.push({ | ||
| type: 'warning', | ||
| message: `Cannot schedule remaining ${remainingDuration}h of task "${task.name}" due to minimum duration constraint (${adjustedTask.minDuration}h).`, | ||
| message: `Cannot schedule part of task "${task.name}" because it would end after the deadline (${task.deadline.format('YYYY-MM-DD HH:mm')}). Skipping remaining duration.`, | ||
| }); | ||
| task.remaining = 0; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Ensure the part start time is rounded to 15-minute increment | ||
| const partStart = roundToQuarterHour(slot.start, false); | ||
| const partEnd = partStart.add(partDuration, 'hour'); | ||
|
|
||
| // Verify the end time doesn't exceed the slot | ||
| if (partEnd.isAfter(slot.end)) { | ||
| const adjustedDuration = slot.end.diff(partStart, 'hour', true); | ||
|
|
||
| if (adjustedDuration < adjustedTask.minDuration) { | ||
| const totalParts = Math.ceil(task.duration / task.maxDuration); | ||
| const newEvent: Event = { | ||
| id: `event-${task.id}-part-${task.nextPart}`, | ||
| name: | ||
| totalParts > 1 | ||
| ? `${task.name} (Part ${task.nextPart}/${totalParts})` | ||
| : task.name, | ||
| range: { start: partStart, end: finalPartEnd }, | ||
| isPastDeadline: false, | ||
| taskId: task.id, | ||
| partNumber: totalParts > 1 ? task.nextPart : undefined, | ||
| totalParts: totalParts > 1 ? totalParts : undefined, | ||
| }; | ||
| if (task.deadline && finalPartEnd.isAfter(task.deadline)) { | ||
| newEvent.isPastDeadline = true; | ||
| logs.push({ | ||
| type: 'error', | ||
| message: `Cannot fit minimum duration for task "${task.name}" part ${partNumber}.`, | ||
| type: 'warning', | ||
| message: `Part ${task.nextPart} of task "${task.name}" is scheduled past its deadline of ${task.deadline.format('YYYY-MM-DD HH:mm')}.`, | ||
| }); | ||
| break; | ||
| } | ||
|
|
||
| partDuration = hoursToQuarterHours(adjustedDuration); | ||
| scheduledEvents.push(newEvent); | ||
| tryTime = roundToQuarterHour(finalPartEnd, true); | ||
| availableTimes[task.category] = tryTime; | ||
| task.remaining -= partDuration; | ||
| task.scheduledParts++; | ||
| task.nextPart++; | ||
| anyScheduled = true; | ||
| scheduledPart = true; | ||
| } | ||
|
|
||
| const finalPartEnd = partStart.add(partDuration, 'hour'); | ||
| const totalParts = Math.ceil( | ||
| adjustedTask.duration / adjustedTask.maxDuration | ||
| ); | ||
|
|
||
| const newEvent: Event = { | ||
| id: `event-${task.id}-part-${partNumber}`, | ||
| name: | ||
| totalParts > 1 | ||
| ? `${task.name} (Part ${partNumber}/${totalParts})` | ||
| : task.name, | ||
| range: { start: partStart, end: finalPartEnd }, | ||
| isPastDeadline: false, | ||
| taskId: task.id, | ||
| partNumber: totalParts > 1 ? partNumber : undefined, | ||
| totalParts: totalParts > 1 ? totalParts : undefined, | ||
| }; | ||
|
|
||
| // Check if this part is past the deadline | ||
| if (task.deadline && finalPartEnd.isAfter(task.deadline)) { | ||
| newEvent.isPastDeadline = true; | ||
| logs.push({ | ||
| type: 'warning', | ||
| message: `Part ${partNumber} of task "${task.name}" is scheduled past its deadline of ${task.deadline.format('YYYY-MM-DD HH:mm')}.`, | ||
| }); | ||
| } | ||
|
|
||
| taskParts.push(newEvent); | ||
| scheduledEvents.push(newEvent); | ||
|
|
||
| remainingDuration -= partDuration; | ||
| availableTimes[task.category] = roundToQuarterHour(finalPartEnd, true); | ||
| partNumber++; | ||
| } |
There was a problem hiding this comment.
The logic for splittable tasks is comprehensive, correctly considering minDuration, maxDuration, and task deadlines. Filtering availableSlots by deadline is a crucial improvement.
| const totalParts = Math.ceil(task.duration / task.maxDuration); | ||
| const newEvent: Event = { | ||
| id: `event-${task.id}-part-${task.nextPart}`, | ||
| name: | ||
| totalParts > 1 | ||
| ? `${task.name} (Part ${task.nextPart}/${totalParts})` | ||
| : task.name, | ||
| range: { start: partStart, end: finalPartEnd }, | ||
| isPastDeadline: false, | ||
| taskId: task.id, | ||
| partNumber: totalParts > 1 ? task.nextPart : undefined, | ||
| totalParts: totalParts > 1 ? totalParts : undefined, |
There was a problem hiding this comment.
Calculating totalParts using Math.ceil(task.duration / task.maxDuration) is correct for determining how many parts a task might be split into, enhancing clarity for the user.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/ai/src/scheduling/templates.ts (1)
89-126: Consider adding allowSplit property consistently across all scenarios.Several template scenarios don't define the
allowSplitproperty. While this is valid since the property is optional, it creates inconsistency and could lead to unclear behavior expectations.Consider adding explicit
allowSplitvalues to all tasks for consistency:{ id: 'mixed-1', name: 'Team Meeting', duration: 1, minDuration: 1, maxDuration: 1.5, category: 'meeting', events: [], + allowSplit: false, },Also applies to: 141-171, 185-223, 237-256, 271-311, 327-366, 380-417, 432-478, 494-531, 547-583, 599-636, 650-688, 704-741
apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskList.tsx (1)
250-272: LGTM: Well-implemented allowSplit toggle with good UX.The toggle switch implementation is excellent with proper tooltip, accessibility considerations, and clean integration into the task header. The callback handling correctly updates the task state.
Consider whether defaulting to
truewithtask.allowSplit ?? trueis appropriate, as some template scenarios explicitly set this tofalse. You might want to use a more conservative default or make it explicit:- checked={task.allowSplit ?? true} + checked={task.allowSplit ?? false}apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskModal.tsx (1)
23-23: Remove unused import.The
Separatorcomponent is imported but never used in the component.-import { Separator } from '@tuturuuu/ui/separator';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/calendar/src/app/[locale]/(root)/scheduler/components/ScheduleDisplay.tsx(2 hunks)apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskList.tsx(3 hunks)apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskModal.tsx(6 hunks)apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx(7 hunks)packages/ai/src/scheduling/algorithm.test.ts(1 hunks)packages/ai/src/scheduling/algorithm.ts(2 hunks)packages/ai/src/scheduling/default.ts(1 hunks)packages/ai/src/scheduling/templates.ts(6 hunks)packages/ai/src/scheduling/types.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/ai/src/scheduling/algorithm.ts (2)
packages/ai/src/scheduling/types.ts (4)
ActiveHours(31-35)Event(8-17)ScheduleResult(42-45)Log(37-40)packages/ai/src/scheduling/default.ts (1)
defaultActiveHours(4-23)
packages/ai/src/scheduling/algorithm.test.ts (2)
packages/ai/src/scheduling/algorithm.ts (1)
scheduleTasks(47-303)packages/ai/src/scheduling/default.ts (1)
defaultActiveHours(4-23)
apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskModal.tsx (3)
packages/ui/src/components/ui/dialog.tsx (2)
DialogContent(134-134)DialogFooter(136-136)packages/ui/src/components/ui/textarea.tsx (1)
Textarea(17-17)packages/ui/src/components/ui/select.tsx (5)
Select(170-170)SelectTrigger(178-178)SelectValue(179-179)SelectContent(171-171)SelectItem(173-173)
🪛 GitHub Actions: Vercel Calendar Preview Deployment
apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx
[error] 490-490: ESLint: Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskModal.tsx
[error] 23-23: ESLint: 'Separator' is defined but never used. (@typescript-eslint/no-unused-vars)
🪛 GitHub Check: Run tests and collect coverage
packages/ai/src/scheduling/algorithm.test.ts
[failure] 217-217: src/scheduling/algorithm.test.ts > Scheduling Algorithm > schedule function > should split a task if allowSplit is true or undefined and needed
Error: Cannot find module './algorithm'
Require stack:
- /home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts
❯ src/scheduling/algorithm.test.ts:217:33
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'MODULE_NOT_FOUND', requireStack: [ '/home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts' ] }
[failure] 198-198: src/scheduling/algorithm.test.ts > Scheduling Algorithm > schedule function > should not schedule a non-splittable task if no single block is available
Error: Cannot find module './algorithm'
Require stack:
- /home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts
❯ src/scheduling/algorithm.test.ts:198:33
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'MODULE_NOT_FOUND', requireStack: [ '/home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts' ] }
[failure] 178-178: src/scheduling/algorithm.test.ts > Scheduling Algorithm > schedule function > should schedule a non-splittable task as a single block if possible
Error: Cannot find module './algorithm'
Require stack:
- /home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts
❯ src/scheduling/algorithm.test.ts:178:33
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'MODULE_NOT_FOUND', requireStack: [ '/home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts' ] }
🪛 GitHub Actions: Run tests and upload coverage
packages/ai/src/scheduling/algorithm.test.ts
[error] 178-217: Scheduling Algorithm test failures: Cannot find module './algorithm' causing 3 test failures in schedule function tests.
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Prettier Check (23)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
🔇 Additional comments (26)
packages/ai/src/scheduling/default.ts (1)
34-34: LGTM: Clean addition of allowSplit property.The addition of
allowSplit: trueto the default task is well-implemented and aligns with the new task splitting functionality.packages/ai/src/scheduling/templates.ts (2)
17-17: LGTM: Appropriate allowSplit values for Basic Work Day scenario.Setting
allowSplit: falsefor Basic Work Day tasks is logical as these represent focused, non-splittable activities like standups and meetings.Also applies to: 27-27, 37-37
53-53: LGTM: Correct allowSplit configuration for splitting scenario.Setting
allowSplit: truefor Task Splitting Challenge tasks is appropriate as this scenario is specifically designed to test task splitting functionality.Also applies to: 63-63, 73-73
packages/ai/src/scheduling/types.ts (2)
16-16: LGTM: Well-designed optional locked property.The
locked?: booleanproperty is appropriately optional and clearly indicates its purpose for marking events as non-reschedulable.
28-28: LGTM: Clean allowSplit property addition.The
allowSplit?: booleanproperty follows good TypeScript practices by being optional and maintaining backward compatibility.apps/calendar/src/app/[locale]/(root)/scheduler/components/ScheduleDisplay.tsx (3)
16-16: LGTM: Appropriate icon import.The
LockIconimport is correctly added to support the new locked event visualization.
229-234: LGTM: Well-structured conditional styling.The conditional styling logic properly prioritizes locked events over past deadline styling, with appropriate blue theming for locked events.
240-251: LGTM: Excellent user experience with lock icon and tooltip.The lock icon with descriptive tooltip provides clear visual feedback for locked events. The conditional rendering and accessibility considerations are well-implemented.
apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskList.tsx (1)
21-21: LGTM: Correct UI component imports.The
SplitIconandSwitchimports are properly added to support the new allowSplit toggle functionality.Also applies to: 32-32
apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx (5)
59-60: Well-implemented state management for locked events.The addition of locked events state variables follows the existing pattern and integrates well with the component's state management.
104-118: Clean implementation of locked event management functions.Both
addLockedEventanddeleteLockedEventfunctions follow good practices:
- Proper event structure with required fields
- Consistent ID generation pattern
- Immutable state updates
125-126: Correct integration of locked events into scheduling.The updated
handleSchedulefunction properly passes locked events to the scheduling algorithm, maintaining the expected function signature.
212-251: Excellent UI implementation for locked events management.The locked events section provides:
- Clear visual indicators with lock icons
- Intuitive add/remove functionality
- Proper styling that distinguishes locked events
- Good user experience with empty state handling
428-536: Well-structured LockedEventForm component with proper validation.The form implementation includes:
- Comprehensive form validation
- Good error handling and user feedback
- Proper date/time constraints
- Clean separation of concerns
The validation logic correctly prevents past dates and invalid time ranges.
apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskModal.tsx (3)
67-67: Good integration of allowSplit property.The addition of
allowSplitto the form state with a sensible default value oftruealigns well with the scheduling algorithm's expectations.
144-149: Properly typed updateFormData function.The function signature correctly accepts boolean values alongside string and number types, enabling proper type safety for the allowSplit toggle.
153-340: Excellent UI restructure with improved user experience.The two-column layout significantly improves the modal's organization:
- Left column: Basic task information (name, description, category, deadline)
- Right column: Technical constraints (duration settings, split toggle)
- Proper spacing and visual hierarchy
- Clear labeling and help text for duration semantics
The allowSplit toggle is well-integrated with appropriate icons and explanatory text.
packages/ai/src/scheduling/algorithm.ts (9)
49-56: Excellent addition of locked events support.The function signature update and initialization of locked events in the schedule provides a solid foundation for respecting pre-existing commitments during scheduling.
60-68: Well-designed task pool architecture.The normalization of tasks to quarter-hour increments and addition of tracking metadata (
remaining,nextPart,scheduledParts) enables sophisticated multi-part scheduling while maintaining data integrity.
87-89: Good safeguard against infinite loops.The attempt counter with a reasonable maximum (2000) prevents infinite loops while allowing sufficient iterations for complex scheduling scenarios.
107-163: Robust non-splittable task scheduling logic.The implementation correctly:
- Attempts to find single continuous blocks for non-splittable tasks
- Includes multiple retry attempts with different start times
- Provides appropriate error logging when scheduling fails
- Respects deadline constraints
176-182: Smart deadline filtering for splittable tasks.The deadline-aware slot filtering prevents scheduling parts that would extend beyond task deadlines, maintaining scheduling integrity.
202-226: Sophisticated duration constraint handling.The logic for handling minimum duration constraints when remaining time is small is well thought out:
- Attempts to extend duration to meet minimum requirements
- Falls back gracefully when constraints cannot be satisfied
- Provides clear warning messages
251-263: Intelligent part naming and metadata.The dynamic calculation of total parts and conditional part numbering creates clear event names while avoiding unnecessary part numbers for tasks that don't actually get split.
283-284: Important deadlock prevention.The check for
anyScheduledprevents infinite loops when no progress can be made, ensuring the algorithm terminates gracefully even in complex scheduling scenarios.
287-300: Comprehensive logging and reporting.The final logging phase provides valuable feedback about:
- Tasks that were split into multiple parts
- Tasks that couldn't be fully scheduled
- Clear quantification of remaining unscheduled time
This will be very helpful for users to understand scheduling decisions.
| it('should not schedule a non-splittable task if no single block is available', () => { | ||
| const { scheduleTasks } = require('./algorithm'); | ||
| const tasks = [ | ||
| { | ||
| id: 'single-block-fail', | ||
| name: 'Non-splittable Task', | ||
| duration: 10, // longer than any available slot | ||
| minDuration: 1, | ||
| maxDuration: 10, | ||
| category: 'work', | ||
| events: [], | ||
| allowSplit: false, | ||
| }, | ||
| ]; | ||
| const result = scheduleTasks(tasks, defaultActiveHours); | ||
| expect(result.events.length).toBe(0); | ||
| expect(result.logs.some((log: any) => log.type === 'error')).toBe(true); | ||
| }); |
There was a problem hiding this comment.
Apply the same import fix and improve test assertion.
The same module import issue exists here. Additionally, the error log assertion could be more specific.
- it('should not schedule a non-splittable task if no single block is available', () => {
- const { scheduleTasks } = require('./algorithm');
+ it('should not schedule a non-splittable task if no single block is available', async () => {
+ const { scheduleTasks } = await import('./algorithm');
const tasks = [
{
id: 'single-block-fail',
name: 'Non-splittable Task',
duration: 10, // longer than any available slot
minDuration: 1,
maxDuration: 10,
category: 'work',
events: [],
allowSplit: false,
},
];
const result = scheduleTasks(tasks, defaultActiveHours);
expect(result.events.length).toBe(0);
- expect(result.logs.some((log: any) => log.type === 'error')).toBe(true);
+ expect(result.logs.some((log) => log.type === 'error' && log.message.includes('could not be scheduled as a single block'))).toBe(true);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should not schedule a non-splittable task if no single block is available', () => { | |
| const { scheduleTasks } = require('./algorithm'); | |
| const tasks = [ | |
| { | |
| id: 'single-block-fail', | |
| name: 'Non-splittable Task', | |
| duration: 10, // longer than any available slot | |
| minDuration: 1, | |
| maxDuration: 10, | |
| category: 'work', | |
| events: [], | |
| allowSplit: false, | |
| }, | |
| ]; | |
| const result = scheduleTasks(tasks, defaultActiveHours); | |
| expect(result.events.length).toBe(0); | |
| expect(result.logs.some((log: any) => log.type === 'error')).toBe(true); | |
| }); | |
| it('should not schedule a non-splittable task if no single block is available', async () => { | |
| const { scheduleTasks } = await import('./algorithm'); | |
| const tasks = [ | |
| { | |
| id: 'single-block-fail', | |
| name: 'Non-splittable Task', | |
| duration: 10, // longer than any available slot | |
| minDuration: 1, | |
| maxDuration: 10, | |
| category: 'work', | |
| events: [], | |
| allowSplit: false, | |
| }, | |
| ]; | |
| const result = scheduleTasks(tasks, defaultActiveHours); | |
| expect(result.events.length).toBe(0); | |
| expect( | |
| result.logs.some( | |
| (log) => | |
| log.type === 'error' && | |
| log.message.includes('could not be scheduled as a single block') | |
| ) | |
| ).toBe(true); | |
| }); |
🧰 Tools
🪛 GitHub Check: Run tests and collect coverage
[failure] 198-198: src/scheduling/algorithm.test.ts > Scheduling Algorithm > schedule function > should not schedule a non-splittable task if no single block is available
Error: Cannot find module './algorithm'
Require stack:
- /home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts
❯ src/scheduling/algorithm.test.ts:198:33
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'MODULE_NOT_FOUND', requireStack: [ '/home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts' ] }
🤖 Prompt for AI Agents
In packages/ai/src/scheduling/algorithm.test.ts around lines 197 to 214, the
test imports the scheduleTasks function inside the test case, which is
inconsistent with best practices. Move the import statement for scheduleTasks to
the top of the file to ensure it is imported once. Also, improve the error log
assertion by checking for a specific error message or error type in the logs
array instead of just verifying the presence of any error type, making the test
more precise.
| it('should split a task if allowSplit is true or undefined and needed', () => { | ||
| const { scheduleTasks } = require('./algorithm'); | ||
| const tasks = [ | ||
| { | ||
| id: 'splittable', | ||
| name: 'Splittable Task', | ||
| duration: 6, | ||
| minDuration: 1, | ||
| maxDuration: 2, | ||
| category: 'work', | ||
| events: [], | ||
| allowSplit: true, | ||
| }, | ||
| ]; | ||
| const result = scheduleTasks(tasks, defaultActiveHours); | ||
| expect(result.events.length).toBeGreaterThan(1); | ||
| expect(result.events[0].name).toContain('Part 1'); | ||
| }); |
There was a problem hiding this comment.
Fix import and enhance splittable task test validation.
Same import issue plus the test could validate more aspects of task splitting behavior.
- it('should split a task if allowSplit is true or undefined and needed', () => {
- const { scheduleTasks } = require('./algorithm');
+ it('should split a task if allowSplit is true or undefined and needed', async () => {
+ const { scheduleTasks } = await import('./algorithm');
const tasks = [
{
id: 'splittable',
name: 'Splittable Task',
duration: 6,
minDuration: 1,
maxDuration: 2,
category: 'work',
events: [],
allowSplit: true,
},
];
const result = scheduleTasks(tasks, defaultActiveHours);
expect(result.events.length).toBeGreaterThan(1);
expect(result.events[0].name).toContain('Part 1');
+ // Validate that all parts belong to the same task
+ expect(result.events.every(event => event.taskId === 'splittable')).toBe(true);
+ // Validate total scheduled duration doesn't exceed task duration
+ const totalScheduled = result.events.reduce((sum, event) =>
+ sum + event.range.end.diff(event.range.start, 'hour', true), 0);
+ expect(totalScheduled).toBeLessThanOrEqual(6);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should split a task if allowSplit is true or undefined and needed', () => { | |
| const { scheduleTasks } = require('./algorithm'); | |
| const tasks = [ | |
| { | |
| id: 'splittable', | |
| name: 'Splittable Task', | |
| duration: 6, | |
| minDuration: 1, | |
| maxDuration: 2, | |
| category: 'work', | |
| events: [], | |
| allowSplit: true, | |
| }, | |
| ]; | |
| const result = scheduleTasks(tasks, defaultActiveHours); | |
| expect(result.events.length).toBeGreaterThan(1); | |
| expect(result.events[0].name).toContain('Part 1'); | |
| }); | |
| it('should split a task if allowSplit is true or undefined and needed', async () => { | |
| const { scheduleTasks } = await import('./algorithm'); | |
| const tasks = [ | |
| { | |
| id: 'splittable', | |
| name: 'Splittable Task', | |
| duration: 6, | |
| minDuration: 1, | |
| maxDuration: 2, | |
| category: 'work', | |
| events: [], | |
| allowSplit: true, | |
| }, | |
| ]; | |
| const result = scheduleTasks(tasks, defaultActiveHours); | |
| expect(result.events.length).toBeGreaterThan(1); | |
| expect(result.events[0].name).toContain('Part 1'); | |
| // Validate that all parts belong to the same task | |
| expect(result.events.every(event => event.taskId === 'splittable')).toBe(true); | |
| // Validate total scheduled duration doesn't exceed task duration | |
| const totalScheduled = result.events.reduce((sum, event) => | |
| sum + event.range.end.diff(event.range.start, 'hour', true), 0); | |
| expect(totalScheduled).toBeLessThanOrEqual(6); | |
| }); |
🧰 Tools
🪛 GitHub Check: Run tests and collect coverage
[failure] 217-217: src/scheduling/algorithm.test.ts > Scheduling Algorithm > schedule function > should split a task if allowSplit is true or undefined and needed
Error: Cannot find module './algorithm'
Require stack:
- /home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts
❯ src/scheduling/algorithm.test.ts:217:33
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'MODULE_NOT_FOUND', requireStack: [ '/home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts' ] }
🤖 Prompt for AI Agents
In packages/ai/src/scheduling/algorithm.test.ts around lines 216 to 233, fix the
import of scheduleTasks by using an ES module import statement at the top
instead of requiring it inside the test. Enhance the test by adding assertions
to verify that all split parts have durations within minDuration and
maxDuration, and that the total duration equals the original task duration,
ensuring the splitting logic is correctly validated.
| it('should schedule a non-splittable task as a single block if possible', () => { | ||
| const { scheduleTasks } = require('./algorithm'); | ||
| const tasks = [ | ||
| { | ||
| id: 'single-block', | ||
| name: 'Non-splittable Task', | ||
| duration: 2, | ||
| minDuration: 1, | ||
| maxDuration: 2, | ||
| category: 'work', | ||
| events: [], | ||
| allowSplit: false, | ||
| }, | ||
| ]; | ||
| const result = scheduleTasks(tasks, defaultActiveHours); | ||
| expect(result.events.length).toBe(1); | ||
| expect(result.events[0].name).toBe('Non-splittable Task'); | ||
| expect(result.events[0].partNumber).toBeUndefined(); | ||
| }); |
There was a problem hiding this comment.
Fix module import issue and use consistent import syntax.
The test is failing because it cannot find the './algorithm' module. Use ES6 import syntax consistently with the rest of the file instead of require().
- it('should schedule a non-splittable task as a single block if possible', () => {
- const { scheduleTasks } = require('./algorithm');
+ it('should schedule a non-splittable task as a single block if possible', async () => {
+ const { scheduleTasks } = await import('./algorithm');Alternatively, add the import at the top of the file:
+import { scheduleTasks } from './algorithm';
import { defaultActiveHours, defaultTasks } from './default';
import dayjs from 'dayjs';
import { describe, expect, it } from 'vitest';Then remove the require statements from individual tests.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should schedule a non-splittable task as a single block if possible', () => { | |
| const { scheduleTasks } = require('./algorithm'); | |
| const tasks = [ | |
| { | |
| id: 'single-block', | |
| name: 'Non-splittable Task', | |
| duration: 2, | |
| minDuration: 1, | |
| maxDuration: 2, | |
| category: 'work', | |
| events: [], | |
| allowSplit: false, | |
| }, | |
| ]; | |
| const result = scheduleTasks(tasks, defaultActiveHours); | |
| expect(result.events.length).toBe(1); | |
| expect(result.events[0].name).toBe('Non-splittable Task'); | |
| expect(result.events[0].partNumber).toBeUndefined(); | |
| }); | |
| it('should schedule a non-splittable task as a single block if possible', async () => { | |
| const { scheduleTasks } = await import('./algorithm'); | |
| const tasks = [ | |
| { | |
| id: 'single-block', | |
| name: 'Non-splittable Task', | |
| duration: 2, | |
| minDuration: 1, | |
| maxDuration: 2, | |
| category: 'work', | |
| events: [], | |
| allowSplit: false, | |
| }, | |
| ]; | |
| const result = scheduleTasks(tasks, defaultActiveHours); | |
| expect(result.events.length).toBe(1); | |
| expect(result.events[0].name).toBe('Non-splittable Task'); | |
| expect(result.events[0].partNumber).toBeUndefined(); | |
| }); |
🧰 Tools
🪛 GitHub Check: Run tests and collect coverage
[failure] 178-178: src/scheduling/algorithm.test.ts > Scheduling Algorithm > schedule function > should schedule a non-splittable task as a single block if possible
Error: Cannot find module './algorithm'
Require stack:
- /home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts
❯ src/scheduling/algorithm.test.ts:178:33
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'MODULE_NOT_FOUND', requireStack: [ '/home/runner/work/platform/platform/packages/ai/src/scheduling/algorithm.test.ts' ] }
🤖 Prompt for AI Agents
In packages/ai/src/scheduling/algorithm.test.ts around lines 177 to 195, replace
the require('./algorithm') call inside the test with a consistent ES6 import
statement at the top of the file. Import scheduleTasks using `import {
scheduleTasks } from './algorithm';` and remove the require statement from the
test to fix the module import issue and ensure consistent import syntax
throughout the file.
| </div> | ||
| <div> | ||
| <Label>Category</Label> | ||
| <Select value={category} onValueChange={(v) => setCategory(v as any)}> |
There was a problem hiding this comment.
Replace explicit 'any' type with proper typing.
ESLint is correctly flagging the use of any. Use the proper union type instead.
- <Select value={category} onValueChange={(v) => setCategory(v as any)}>
+ <Select value={category} onValueChange={(v) => setCategory(v as 'work' | 'personal' | 'meeting')}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Select value={category} onValueChange={(v) => setCategory(v as any)}> | |
| <Select value={category} onValueChange={(v) => setCategory(v as 'work' | 'personal' | 'meeting')}> |
🧰 Tools
🪛 GitHub Actions: Vercel Calendar Preview Deployment
[error] 490-490: ESLint: Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
In apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx at line 490, the use
of 'any' in the type assertion for setCategory is flagged by ESLint. Replace
'any' with the correct union type that matches the expected values for category,
ensuring type safety and compliance with linting rules.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3126 +/- ##
==========================================
+ Coverage 0.80% 0.88% +0.07%
==========================================
Files 2542 2542
Lines 323788 324107 +319
Branches 3043 3095 +52
==========================================
+ Hits 2621 2872 +251
- Misses 318998 319067 +69
+ Partials 2169 2168 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests