Optimize schedule generation and improve optional course handling#446
Optimize schedule generation and improve optional course handling#446YasushikoX wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
@YasushikoX is attempting to deploy a commit to the Sandbox NU Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR refactors and optimizes the scheduler’s core schedule-generation logic by introducing a more efficient combination generator, improving optional-course handling, and expanding the unit test suite to cover the production implementations.
Changes:
- Introduces
generateCombinationsOptimized+addOptionalCourseshelpers (withMAX_RESULTS) and wires them intogenerateSchedules. - Optimizes meeting-time mask creation in
binaryMeetingTime.tsusing a range-mask formula. - Replaces prior “shadow implementation” tests with end-to-end tests against the real scheduling functions; adds optional-course and benchmark coverage.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| package-lock.json | Adds an npm lockfile (new) alongside existing pnpm setup. |
| apps/searchneu/lib/scheduler/generateSchedules.ts | Switches schedule generation to new optimized helpers; precomputes optional masks; applies MAX_RESULTS when adding optionals. |
| apps/searchneu/lib/scheduler/generateSchedules.test.ts | Rewrites tests to exercise production scheduling helpers via a DB-free simulation. |
| apps/searchneu/lib/scheduler/generateCombinations.ts | New optimized locked-combination generator (prefix masks) and optimized optional-course recursion with caps. |
| apps/searchneu/lib/scheduler/binaryMeetingTimeTests/binaryMeetingTime.test.ts | Adds coverage for mask encoding, incrementIndex, masksConflict, and combination edge cases. |
| apps/searchneu/lib/scheduler/binaryMeetingTimeTests/addOptionalCourses.test.ts | New comprehensive test suite for optional-course expansion logic. |
| apps/searchneu/lib/scheduler/binaryMeetingTimeTests/benchmark.bench.ts | Adds manual benchmark suite for before/after comparisons. |
| apps/searchneu/lib/scheduler/binaryMeetingTime.ts | Replaces per-slot bit setting with a contiguous range-mask shift. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
benchmark.bench.ts exists purely to compare the old vs new algorithm side by side. It bundles a copy of the original code for that purpose. This file can be removed entirely since it has no value in production or ongoing CI. |
Replace per-slot looping with a BigInt range-mask in meetingTimesToBinaryMask for faster mask construction and remove the now-unused helper. Introduce generateCombinations.ts: an optimized, iterative combination generator with prefix/skip logic and a MAX_RESULTS cap to avoid runaway generation. Update generateSchedules to use the new generator, precompute optional section masks, and rewrite addOptionalCourses to thread a combined bigint mask and respect MAX_RESULTS for early exit. Add extensive unit/benchmark tests to validate performance and correctness across many scenarios.
Introduce addOptionalCourses to generate valid combinations including optional courses and export it from generateCombinations.ts. Reduce MAX_RESULTS from 1_000 to 100 and add guards/limits in generateCombinationsOptimized (single-course slicing and empty-course early-return) to avoid runaway generation. Add extensive unit tests: new addOptionalCourses tests, expanded binaryMeetingTime tests (meetingTimesToBinaryMask, masksConflict, incrementIndex) and updated generateSchedules tests to exercise the locked+optional pipeline. These changes improve correctness for optional course handling and strengthen test coverage.
0f4927d to
8f88c96
Compare
Rename benchmark.test.ts to benchmark.bench.ts and move the large benchmarking suite and helper builders (scaling, adversarial, realistic, edge cases) into the new .bench file. Trim binaryMeetingTime.test.ts to keep unit tests (remove the long benchmark helpers and brute-force generator/original comparisons), and update imports accordingly. Also remove the re-export of incrementIndex from generateSchedules.ts to avoid exposing that internal helper.
8f88c96 to
8cc1b5d
Compare
|
Hey, really appreciate you taking the time to dig into the codebase and put this together! External contributions mean a lot to us and this is genuinely impressive work. We do have a more pressing change to the generation algorithm that we need to get merged first before we can bring this in, to avoid conflicts. Once that's in we'll do a proper review of this PR and work with you to get it merged. Don't want this to get lost; we'll make sure it doesn't. Thanks again, this is exactly the kind of contribution that makes open source great! 🙏 |
|
Hello, no worries, if you could comment whenever those changes would be merged in I will be happy to bring my branch up to date and revise any changes in case there would be conflicts. Happy to help! |
Optimize schedule generation algorithm
This PR optimizes the core algorithm, adds a result cap, and rewrites the test suite.
Algorithm changes
Range-mask formula in
binaryMeetingTime.ts- Replaced per-slot bit loop with(1n << numSlots) - 1nshifted into place. One expression instead of a tight loop.Prefix mask array in
generateCombinations.ts- Instead of rechecking the whole schedule from scratch each iteration, aprefixMasksarray maintains the running OR of all accepted masks. Conflict detection goes from O(n) to O(1) per position, and the prefix state is reused when only the last index advances.Precomputed optional masks in
generateSchedules.ts- Optional section masks are computed once before the main loop instead of once per locked schedule.addOptionalCoursesthreads a single combined bigint mask through recursion instead of an array, and uses push/pop instead of array spreading.MAX_RESULTS cap (100) - Prevents runaway generation when many non-conflicting sections exist. This was the primary cause of the hour-long runtime.
Bug fixes
Empty course guard - Added early return when any course has zero sections. Previously this would silently produce wrong results.
Single-course maxResults - The single-course fast path now respects the
maxResultscap via.slice(). Previously it was ignored.Test improvements
Rewrote
generateSchedules.test.ts- The old tests tested local shadow copies of the logic, not the actual production functions. The new file imports and tests the real implementations. Covers locked-only, optional-only, mixed,numCoursesedge cases,MAX_RESULTScapping, and conflict invariants.New
addOptionalCourses.test.ts- Full coverage for the new function: skip/include choices, conflict filtering, multi-meeting sections,numCoursescombinations,maxResultscapping, and push/pop array isolation safety.New unit tests in
binaryMeetingTime.test.ts- New tests coveringincrementIndexcarry/overflow,meetingTimesToBinaryMaskedge cases (zero-duration, multi-day, multi-block, back-to-back),masksConflict, andgenerateCombinationsOptimized(empty course guard, mask correctness, single-course cap).Benchmark suite - Before/after comparison across 13 scenarios including adversarial, edge case, and uncapped exhaustion tests. File is named
benchmark.bench.ts(not.test.ts) so it won't run during normal CI test runs. Run manually withnpx tsx --test apps/searchneu/lib/scheduler/binaryMeetingTimeTests/benchmark.bench.ts.Benchmark results (69 unit tests + 21 benchmarks, all passing)