-
-
Notifications
You must be signed in to change notification settings - Fork 19
optimize algorithm for scheduling #3124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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. |
WalkthroughThis update introduces support for locked events and task splitting in the scheduling system. It modifies scheduling logic, type definitions, and UI components to handle locked events (which cannot be moved) and tasks that can be split into multiple sessions. The user interface is updated to manage, display, and interact with these new features. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SchedulerPage
participant LockedEventForm
participant ScheduleTasks
participant ScheduleDisplay
User->>SchedulerPage: Opens "Add Locked Event" modal
SchedulerPage->>LockedEventForm: Renders form
User->>LockedEventForm: Submits locked event details
LockedEventForm->>SchedulerPage: Calls addLockedEvent()
SchedulerPage->>ScheduleTasks: Calls scheduleTasks(tasks, activeHours, lockedEvents)
ScheduleTasks-->>SchedulerPage: Returns scheduled events
SchedulerPage->>ScheduleDisplay: Passes events (including locked)
ScheduleDisplay-->>User: Displays events (locked visually indicated)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/calendar/src/app/[locale]/(root)/scheduler/components/ScheduleDisplay.tsxOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskList.tsxOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskModal.tsxOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 introduces significant enhancements to the scheduling application by providing users with more granular control over how tasks are scheduled. The core scheduling algorithm has been upgraded to respect pre-defined 'locked events' and to handle tasks that cannot be split into multiple sessions. These new capabilities are fully integrated into the user interface, allowing for easy management of both task splitting preferences and fixed calendar commitments.
Highlights
- Enhanced Scheduling Algorithm: The core scheduling algorithm (
packages/ai/src/scheduling/algorithm.ts
) has been significantly updated to support two new features: non-splittable tasks (ensuring they are scheduled as a single, contiguous block) and locked events (pre-defined time blocks that the algorithm must respect). - Task Splitting Control: A new
allowSplit
property has been added to theTask
interface, enabling users to specify whether a task can be broken down into multiple sessions. This control is exposed through a new toggle in theTaskModal
and directly on each task item in theTaskList
component. - Locked Event Management UI: A new section for 'Locked Events' has been introduced on the main scheduler page (
apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx
), allowing users to view, add, and remove fixed time commitments via a dedicated modal. - Improved Event Display: The
ScheduleDisplay
component now visually distinguisheslocked
events with a unique border/background color and displays aLockIcon
with a helpful tooltip for quick identification. - Algorithm Test Coverage: New unit tests have been added for the
schedule
function inpackages/ai/src/scheduling/algorithm.test.ts
to validate the behavior of both non-splittable and splittable tasks, ensuring the new logic functions as expected.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces enhancements for locked events and controllable task splitting. Key areas for attention include the readability of conditional styling, type safety in form handling, and ensuring consistent deadline logic within the scheduling algorithm.
</div> | ||
<div> | ||
<Label>Category</Label> | ||
<Select value={category} onValueChange={(v) => setCategory(v as any)}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/ai/src/scheduling/templates.ts (1)
89-127
: Consider adding explicitallowSplit
values for consistency.Many template scenarios don't specify the
allowSplit
property, which means they'll rely on undefined behavior. For better predictability and documentation, consider explicitly settingallowSplit
values for all tasks in all scenarios.For example, you could add appropriate
allowSplit
values based on the nature of each task:{ id: 'mixed-1', name: 'Team Meeting', duration: 1, minDuration: 1, maxDuration: 1.5, category: 'meeting', events: [], + allowSplit: false, },
Also applies to: 141-172, 185-224, 237-257, 271-311, 327-367, 380-418, 432-479, 493-531, 545-583, 597-636, 650-688, 702-742
apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskModal.tsx (1)
23-23
: Remove unused import.The
Separator
component is imported but not 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
🪛 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: Test
packages/ai/src/scheduling/algorithm.test.ts
[error] 1-1: Vitest test suite failed with 3 tests failing in src/scheduling/algorithm.test.ts.
🪛 GitHub Actions: Run tests and upload coverage
packages/ai/src/scheduling/algorithm.test.ts
[error] 178-217: 3 test failures due to 'Cannot find module './algorithm'' error when requiring './algorithm' module in schedule function tests.
🪛 GitHub Actions: Vercel Calendar Preview Deployment
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)
apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx
[error] 490-490: ESLint: Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Prettier Check (23)
🔇 Additional comments (30)
packages/ai/src/scheduling/default.ts (1)
34-34
: LGTM! Default task configuration updated correctly.The addition of
allowSplit: true
to the default task aligns well with the new task splitting feature and provides a sensible default behavior.packages/ai/src/scheduling/types.ts (2)
16-16
: Well-designed type addition for locked events.The optional
locked
property provides a clean way to mark events that cannot be moved during scheduling, maintaining backward compatibility.
28-28
: Appropriate type addition for task splitting control.The optional
allowSplit
property gives fine-grained control over which tasks can be divided into multiple sessions while preserving existing functionality.packages/ai/src/scheduling/templates.ts (2)
17-17
: Good design choice for Basic Work Day scenario.Setting
allowSplit: false
for focused work tasks like meetings and code reviews makes sense as these activities typically benefit from uninterrupted time blocks.Also applies to: 27-27, 37-37
53-53
: Appropriate configuration for Task Splitting Challenge scenario.The
allowSplit: true
setting aligns perfectly with the scenario's description of "large tasks that need to be split due to time constraints."Also applies to: 63-63, 73-73
apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskList.tsx (2)
21-21
: Good component imports for the new feature.Adding
SplitIcon
andSwitch
imports provides the necessary UI components for the task splitting toggle functionality.Also applies to: 32-32
250-272
: Well-designed task splitting toggle UI.The implementation includes several good UX practices:
- Tooltip provides clear explanation of the feature
- Sensible default (
true
) whenallowSplit
is undefined- Proper integration with the existing task update callback
- Clean styling that fits with the component's design system
The positioning next to the task name keeps the control accessible while maintaining visual hierarchy.
apps/calendar/src/app/[locale]/(root)/scheduler/components/ScheduleDisplay.tsx (3)
16-16
: Appropriate import for locked event visualization.Adding
LockIcon
provides the visual indicator needed for locked events in the schedule display.
229-234
: Excellent conditional styling for different event states.The styling logic effectively handles multiple event states:
- Locked events get distinct blue styling for immediate recognition
- Overdue events maintain their destructive styling
- Regular events keep the standard hover behavior
The conditional structure is clean and maintainable.
240-251
: Great UX for locked event indication.The lock icon with tooltip provides clear visual feedback and user education about what locked events represent. The positioning and styling integrate well with the existing event header design.
packages/ai/src/scheduling/algorithm.test.ts (2)
177-233
: LGTM on test cases for allowSplit functionality!The test cases comprehensively cover the new allowSplit behavior:
- Non-splittable tasks scheduled as single blocks
- Non-splittable tasks failing when no suitable block exists
- Splittable tasks being divided into multiple parts with proper naming
The test logic and assertions are well-structured and will provide good coverage once the module import issue is resolved.
178-178
: ```shell
#!/bin/bashSearch for scheduleTasks exports in the algorithm module
rg "export (async )?function scheduleTasks" -n packages/ai/src/scheduling/algorithm.ts
rg "export const scheduleTasks" -n packages/ai/src/scheduling/algorithm.ts
rg "export default scheduleTasks" -n packages/ai/src/scheduling/algorithm.ts</details> <details> <summary>apps/calendar/src/app/[locale]/(root)/scheduler/components/TaskModal.tsx (4)</summary> `24-24`: **LGTM on new UI component imports!** The Switch and SplitIcon imports are properly used in the allowSplit toggle implementation. Also applies to: 28-28 --- `67-67`: **Excellent integration of allowSplit property!** The allowSplit property is properly: - Initialized with a sensible default (true) - Included in task creation - Reset on form close This provides a consistent user experience for the new feature. Also applies to: 121-121, 138-138 --- `144-144`: **Good type safety improvement for form updates.** Updating the `updateFormData` function signature to handle boolean values maintains type safety while supporting the new allowSplit toggle. --- `153-153`: **Excellent UI layout reorganization!** The two-column layout effectively organizes related fields: - Left: Basic info and description - Right: Duration controls and settings The allowSplit toggle is well-positioned with clear labeling and helpful explanatory text. The dialog width adjustment accommodates the new layout nicely. Also applies to: 164-340 </details> <details> <summary>apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx (6)</summary> `21-21`: **LGTM on new imports and UI components!** The Button, LockIcon, and Select component imports are properly used throughout the locked events implementation. Also applies to: 33-33, 39-45 --- `59-60`: **Good state management for locked events.** The state variables for locked events and modal visibility follow React patterns correctly and support the new functionality well. --- `104-118`: **Excellent locked event management functions!** The `addLockedEvent` and `deleteLockedEvent` functions properly: - Generate unique IDs with timestamps - Set the locked property to true - Use a consistent taskId for locked events - Follow immutable state update patterns --- `125-126`: **Proper integration with scheduling algorithm.** Passing locked events to the `scheduleTasks` function ensures that the algorithm can respect pre-existing locked time slots during scheduling. --- `212-251`: **Excellent UI for locked events management!** The locked events section provides: - Clear visual distinction with lock icons - Readable time formatting - Easy removal functionality - Proper empty state handling The styling with blue theme colors effectively communicates the locked status. --- `428-536`: **Well-implemented LockedEventForm component!** The form component provides: - Proper validation for required fields and time logic - Good user experience with clear error messages - Consistent styling with the rest of the application - Proper form submission handling The component follows React form patterns correctly and integrates well with the parent modal. </details> <details> <summary>packages/ai/src/scheduling/algorithm.ts (8)</summary> `49-50`: **Excellent addition of locked events support!** Adding the `lockedEvents` parameter with a sensible default enables the algorithm to respect pre-existing time blocks while maintaining backward compatibility. --- `52-56`: **Proper integration of locked events into schedule.** Locked events are correctly added to the initial schedule with the `locked: true` property, ensuring they won't be modified during the scheduling process. --- `59-68`: **Smart task pool preparation with duration normalization.** The task pool setup correctly: - Normalizes durations to quarter-hour increments for consistency - Ensures minimum 15-minute durations - Tracks remaining time and part numbers for splitting logic - Maintains all necessary state for the enhanced scheduling --- `78-85`: **Good deadline-aware task prioritization.** Sorting tasks by deadline (earliest first) ensures that time-sensitive tasks get priority in the round-robin scheduling approach, improving deadline adherence. --- `87-92`: **Proper infinite loop protection.** The `maxAttempts` limit and progress tracking (`anyScheduled`) prevent infinite loops while allowing sufficient iterations for complex scheduling scenarios. --- `106-163`: **Excellent non-splittable task scheduling logic!** The implementation correctly: - Respects the `allowSplit: false` constraint - Attempts to find single continuous blocks - Retries across multiple days when needed - Provides informative logging for failures - Handles deadline violations appropriately This ensures tasks that cannot be split are properly accommodated or clearly flagged when impossible. --- `165-279`: **Robust splittable task scheduling with comprehensive constraint handling!** The splittable task logic expertly handles: - Deadline filtering of available slots - Min/max duration constraints per part - Remaining duration less than minimum duration edge cases - Proper part numbering and naming - Deadline violation detection and warnings The implementation balances complexity with thoroughness, ensuring tasks are split intelligently while respecting all constraints. --- `287-300`: **Valuable logging for split tasks and unscheduled work.** The post-scheduling logging provides essential feedback about: - Which tasks were split and into how many parts - Any remaining unscheduled duration with reasons This helps users understand the scheduling decisions and take corrective action if needed. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
</div> | ||
<div> | ||
<Label>Category</Label> | ||
<Select value={category} onValueChange={(v) => setCategory(v as any)}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix TypeScript any type usage.
Using explicit any
type reduces type safety. Define a proper type for the category value.
-<Select value={category} onValueChange={(v) => setCategory(v as any)}>
+<Select value={category} onValueChange={(v: 'work' | 'personal' | 'meeting') => setCategory(v)}>
📝 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: 'work' | 'personal' | 'meeting') => setCategory(v)}> |
🧰 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
onValueChange handler casts the value to 'any', which reduces type safety.
Replace the 'any' cast by defining and using the correct type for the category
value, ensuring the type matches the expected type of the Select component's
value and state.
Summary by CodeRabbit