Restore main to fix branch state - #180
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return course.courseData.sections | ||
| .filter(section => section.number - 1 === course.sectionId) // Filter by selected section |
There was a problem hiding this comment.
🔴 Calendar export picks the wrong class section, or silently drops classes
The calendar export matches a class's chosen meeting time by assuming section labels are numbered consecutively (section.number - 1 === course.sectionId at src/Workspace.tsx:88) instead of using the actually chosen section, so exported files can contain another section's times or omit the class entirely.
Impact: Downloaded schedules can show wrong meeting times or be missing courses.
Section numbers are not contiguous in the catalog data
course.sectionId is an index into courseData.sections, while section.number is the catalog's section label. In src/data/IndexedTotalFA2026-27.json, 51 of 442 courses have gaps in their section numbers (e.g. ACM 190 has sections [1,2,4,5,6,7,8]). For such a course, selecting index 2 (label 4) matches nothing (4-1 !== 2) → course dropped; selecting index 3 (label 5) also fails, while some other index can match a different section's label → wrong times exported. The code being replaced looked the section up directly by index (course.courseData.sections[course.sectionId]).
Prompt for agents
In exportICS in src/Workspace.tsx, the selected section is found by filtering sections with `section.number - 1 === course.sectionId`. sectionId is an array index, while section.number is the catalog label, and labels are frequently non-contiguous in the data files. The export should instead take `course.courseData.sections[course.sectionId]` directly (skipping courses where sectionId is null or the section is missing).
Was this helpful? React with 👍 or 👎 to provide feedback.
| function exportICS(term: string, courses: CourseStorage[]): string { | ||
| const termStartDate = (TERM_START_DATES as { [key: string]: string })[term]; | ||
| if (!termStartDate) { | ||
| throw new Error(`No term start date is available for ${term}`); | ||
| } | ||
| const [startYear, startMonth, startDay] = termStartDate | ||
| .split("-") | ||
| .map(Number); | ||
| const termStartDate = new Date(( TERM_START_DATES as {[key: string] : string} )[term]); |
There was a problem hiding this comment.
🟡 Downloading a schedule for an older term does nothing and throws an error
The schedule download builds dates from a term start date that does not exist for older terms (new Date(TERM_START_DATES[term]) at src/Workspace.tsx:61), and the button now runs without any error handling, so pressing it on those terms fails with no file and no message.
Impact: Users viewing past terms get no download and no explanation when exporting their schedule.
Invalid Date propagates into toISOString()
src/data/term_start_dates.json only contains fa2025 through fa2027, but the app routes cover fa2023…sp2024 too. For those terms TERM_START_DATES[term] is undefined, new Date(undefined) is an Invalid Date, and event.startTime.toISOString() (src/Workspace.tsx:119-120) throws RangeError: Invalid time value. The click handler (src/Workspace.tsx:646-659) has no try/catch, so the error escapes into React's event handling; the removed code threw a descriptive Error and the handler alerted the user.
Prompt for agents
exportICS in src/Workspace.tsx assumes TERM_START_DATES contains an entry for every term, but src/data/term_start_dates.json only covers fa2025 onwards while the router exposes fa2023–sp2024. Missing entries produce an Invalid Date and a RangeError from toISOString, and the 'Export .ics' click handler has no error handling. Consider validating the term start date up front and surfacing a user-visible message (or disabling the button) when it is missing.
Was this helpful? React with 👍 or 👎 to provide feedback.
| state.setCourses( | ||
| // Change based on term | ||
| (DEFAULT_COURSES[term.substring(0, 2)] ?? []).flatMap((name) => { | ||
| const course = getCourse(name, indexedCourses); | ||
| if (!course) { | ||
| console.warn( | ||
| `Default course "${name}" not found in ${term} catalog; skipping`, | ||
| ); | ||
| return []; | ||
| } | ||
| return [{ ...course, enabled: true, locked: true }]; | ||
| }), | ||
| DEFAULT_COURSES[term.substring(0, 2)].map((name) => ({ | ||
| ...getCourse(name, indexedCourses)!, | ||
| enabled: true, | ||
| locked: true, | ||
| })), |
There was a problem hiding this comment.
🔴 Loading the default schedule can blank out the whole app on spring terms
The default-schedule action assumes every preset course exists in the catalog (...getCourse(name, indexedCourses)! at src/Workspace.tsx:637-638), so on terms where one is missing it stores a broken entry and the page crashes.
Impact: Clicking "Default Schedule" on affected terms crashes the page and the user loses their view.
getCourse returns null and the non-null assertion spreads nothing
getCourse returns null when no matching course is found (src/Workspace.tsx:57). Spreading null yields {enabled: true, locked: true} with no courseData. The preset "CS 3 x" is absent from IndexedTotalSP2022-23.json, IndexedTotalSP2023-24.json and IndexedTotalSP2024-25.json, so on /sp2023, /sp2024, /sp2025 the resulting entry reaches setCourses → generateCourseSections → request.courseData.sections and throws a TypeError during render. The same happens if the button is pressed before course data is populated, since indexedCourses starts as {} (src/App.tsx:279). The replaced code skipped missing courses with a warning and guarded on an empty catalog.
Prompt for agents
The 'Default Schedule' button in src/Workspace.tsx maps DEFAULT_COURSES entries through getCourse with a non-null assertion. getCourse returns null when the course is not present in the term's catalog (e.g. 'CS 3 x' is missing from the sp2023/sp2024/sp2025 data files) and also when the catalog has not loaded yet, producing course entries without courseData that crash downstream scheduling/render code. Filter out missing courses (and guard against an empty catalog) before calling setCourses.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const localWorkspaces = localStorage.getItem("workspaces" + realPath); | ||
| const [workspaces, setWorkspaces] = useState<Workspace[]>( | ||
| localWorkspaces | ||
| ? JSON.parse(localWorkspaces) | ||
| : [ | ||
| emptyWorkspace(), | ||
| emptyWorkspace(), | ||
| emptyWorkspace(), | ||
| emptyWorkspace(), | ||
| emptyWorkspace(), | ||
| ], | ||
| ); | ||
| const localWorkspaceIdx = localStorage.getItem("workspaceIdx" + realPath); | ||
| const [workspaceIdx, setWorkspaceIdx] = useState<number>( | ||
| localWorkspaceIdx ? JSON.parse(localWorkspaceIdx) : 0, | ||
| ); | ||
|
|
||
| const courses = workspaces[workspaceIdx].courses; | ||
| const availableTimes: Date[][] = [[], [], [], [], []]; | ||
|
|
||
| for (let i = 0; i < availableTimes.length; ++i) { | ||
| for (let j = 0; j < 2; ++j) { | ||
| availableTimes[i][j] = new Date( | ||
| workspaces[workspaceIdx].availableTimes[i][j], | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // Save state to local storage | ||
| useEffect(() => { | ||
| localStorage.setItem("workspaces" + realPath, JSON.stringify(workspaces)); | ||
| localStorage.setItem( | ||
| "workspaceIdx" + realPath, | ||
| JSON.stringify(workspaceIdx), | ||
| ); | ||
| }, [workspaces, workspaceIdx, realPath]); |
There was a problem hiding this comment.
🔴 Saved workspaces for one term get overwritten when navigating back to it
Saved course lists are only read from browser storage when the page first loads (localStorage.getItem("workspaces" + realPath) at src/App.tsx:291-305) while the save step keeps writing under the newly visited term, so returning to a term via the back button replaces its saved data with the other term's.
Impact: A user's previously saved schedules for a term can be silently wiped out.
State initialisation vs. reactive realPath
realPath changes reactively via the popstate listener (src/App.tsx:258-277), but workspaces/workspaceIdx are initialised from localStorage only in useState initialisers, which run once. After a popstate navigation between terms, the in-memory workspaces still belong to the previous term, while the persistence effect at src/App.tsx:320-326 immediately writes them to "workspaces" + realPath for the new term, clobbering whatever was stored there. The removed implementation explicitly re-read storage on path change (see src/useAppState.ts:111-124).
Prompt for agents
In src/App.tsx, workspaces and workspaceIdx are initialised from localStorage only on mount, but realPath is reactive (popstate listener). When the term changes without a full reload, the stale state is persisted under the new term's storage key by the save effect, destroying the stored data for that term. Re-read (or reset to defaults) workspaces and workspaceIdx whenever realPath changes, before the persistence effect runs.
Was this helpful? React with 👍 or 👎 to provide feedback.
| parsedEvents.forEach(event => { | ||
| const dtStart = event.startTime.toISOString().replace(/-|:|\.\d+/g, ""); // Convert to UTC in .ics format | ||
| const dtEnd = event.endTime.toISOString().replace(/-|:|\.\d+/g, ""); // Convert to UTC in .ics format | ||
|
|
||
| // Add each event to the ICS content | ||
| icsContent += `BEGIN:VEVENT | ||
| SUMMARY:${event.name} | ||
| LOCATION:${event.location} | ||
| DTSTART:${dtStart} | ||
| DTEND:${dtEnd} | ||
| RRULE:FREQ=WEEKLY;COUNT=10 | ||
| UID:${Date.now() + Math.random()}@caltech.dev | ||
| END:VEVENT | ||
| `; | ||
| }); |
There was a problem hiding this comment.
🟡 Exported class times drift by an hour after daylight-saving changes
Class meeting times are written as fixed absolute instants (toISOString() at src/Workspace.tsx:119-120) with a weekly repeat, so every meeting after a daylight-saving switch appears an hour off in the user's calendar.
Impact: Imported schedules show wrong class times for part of the term.
UTC DTSTART + FREQ=WEEKLY ignores local DST
getFirstOccurrence builds the first meeting in local wall-clock time, then toISOString() converts it to a UTC instant written as DTSTART:...Z with RRULE:FREQ=WEEKLY;COUNT=10. Recurrences of a UTC-anchored DTSTART all occur at the same absolute instant, so once local time crosses a DST boundary (e.g. a fall term starting late September running 10 weeks past the November transition) the displayed local time shifts by one hour. The replaced implementation emitted floating local times anchored to a VTIMEZONE for America/Los_Angeles, which keeps wall-clock times stable across DST.
Prompt for agents
exportICS in src/Workspace.tsx emits DTSTART/DTEND as UTC instants (toISOString) together with RRULE:FREQ=WEEKLY;COUNT=10. Weekly recurrences from a UTC anchor keep the same absolute instant, so occurrences after a DST transition are displayed one hour off. Emit local/floating times with an explicit TZID (VTIMEZONE for the campus timezone) instead, as the previous implementation did.
Was this helpful? React with 👍 or 👎 to provide feedback.
| @@ -552,25 +503,20 @@ export default function Workspace({ term }: { term: string }) { | |||
| const shortened = shortenCourses(state.courses) | |||
| .map((c) => [c.courseId, c.enabled, c.locked, c.sectionId]) | |||
| .flat(); | |||
There was a problem hiding this comment.
🔍 Workspace share codes generated by the current deployment stop importing
Export now produces window.btoa(JSON.stringify(...)) and import only accepts window.atob(code), dropping the compressed-code path plus the legacy fallback (decodeShareCode). Any code a user copied from the currently deployed (compressed) version will fail to parse and only produce an "Error importing workspace." alert after this revert. If share codes are expected to be durable, a fallback decoder should be kept.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export function parseTimes(times: string): Maybe<TimeInterval>[][] { | ||
| const ret: Maybe<TimeInterval>[][] = [[], [], [], [], []]; | ||
| const day_to_i = ["M", "T", "W", "R", "F"]; // TODO: Include Sat/Sun Courses, OM Courses | ||
|
|
There was a problem hiding this comment.
📝 Info: Debug logging reintroduced in the time parser
console.log(times_clean) runs for every section time string parsed. parseTimes is called from sectionsIntersect/verify inside the arrangement search, which is invoked combinatorially, so this will spam the console (and slow down) arrangement generation for workspaces with several unlocked courses.
Was this helpful? React with 👍 or 👎 to provide feedback.
| import DATA_FA2023 from "./data/IndexedTotalFA2022-23.json"; | ||
| import DATA_WI2023 from "./data/IndexedTotalWI2022-23.json"; | ||
| import DATA_SP2023 from "./data/IndexedTotalSP2022-23.json"; | ||
| import DATA_FA2024 from "./data/IndexedTotalFA2023-24.json"; | ||
| import DATA_WI2024 from "./data/IndexedTotalWI2023-24.json"; | ||
| import DATA_SP2024 from "./data/IndexedTotalSP2023-24.json"; | ||
| import DATA_FA2025 from "./data/IndexedTotalFA2024-25.json"; | ||
| import DATA_WI2025 from "./data/IndexedTotalWI2024-25.json"; | ||
| import DATA_SP2025 from "./data/IndexedTotalSP2024-25.json"; | ||
| import DATA_FA2026 from "./data/IndexedTotalFA2025-26.json"; | ||
| import DATA_WI2026 from "./data/IndexedTotalWI2025-26.json"; | ||
| import DATA_SP2026 from "./data/IndexedTotalSP2025-26.json"; | ||
| import DATA_FA2027 from "./data/IndexedTotalFA2026-27.json"; |
There was a problem hiding this comment.
📝 Info: All 13 term catalogs are statically imported again
src/App.tsx imports every IndexedTotal*.json at module scope, so all term catalogs are bundled into the initial chunk regardless of which term is viewed (the removed src/courseData.ts lazily import()ed them and cached the result). Combined with the removal of manualChunks in vite.config.ts, this significantly increases first-load payload.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const data = courseDataSources[realPath]; | ||
| const [indexedCourses, setIndexedCourses] = useState({}); | ||
|
|
||
| // load course data from a json url | ||
| useEffect(() => { | ||
| try { | ||
| setIndexedCourses(data); | ||
| } catch { | ||
| alert("Error loading course data"); | ||
| } | ||
| }, [data]); |
There was a problem hiding this comment.
📝 Info: Unknown term paths yield an undefined course index instead of an empty one
const data = courseDataSources[realPath] is undefined for any URL other than the 13 known terms; the try/catch around setIndexedCourses(data) never triggers because nothing throws, so the alert is dead code and AllCourses ends up undefined. Object.values(indexedCourses) in WorkspaceSearch then throws, blanking the page. The removed loader returned {} for unknown terms.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| # Wrangler local dev cache | ||
| .wrangler/ | ||
|
|
||
| ### OSX ### |
There was a problem hiding this comment.
📝 Info: Wrangler local dev cache no longer ignored by git
The .wrangler/ ignore entry is removed while npm run dev still runs wrangler dev, which creates a .wrangler/ state directory in the repo root. Contributors running the dev server will now see that generated directory as untracked noise and may accidentally commit it.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
also, rahul, can i have some modal credits please |
1 participant
all you have to do is undo the devin commits