Deployment tooling cleanup

[rchalamala]

Implements all Tier 1 priorities by cleaning up deployment, adding comprehensive tests, and improving mobile responsiveness.

---

  

---

[cursor]

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	caltech-dev	fd1ec4b	Commit Preview URL Branch Preview URL	Mar 11 2026, 06:17 AM

[devin-ai-integration]

Devin Review found 4 potential issues.

View 4 additional findings in Devin Review.

[devin-ai-integration]

🚩 package.json scripts still use npm run internally despite Bun-first policy

The check and build scripts in package.json:34-35 still use npm run for sub-script invocation ("check": "npm run lint && npm run test:run && npm run build"). When invoked via bun run check, this spawns npm as a subprocess. While functionally correct (npm can run scripts without installing), it's inconsistent with the Bun-first policy documented in AGENTS.md and README.md. These lines are unchanged in this PR (pre-existing), but worth noting since this PR establishes the Bun-first tooling more firmly.

(Refers to lines 34-35)

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🔴 useWorkspaceState does not reinitialize when termPath changes via client-side navigation

The useState initializers at src/hooks/useWorkspaceState.ts:93-98 only run on first mount. When the user navigates between terms via React Router client-side navigation (e.g., clicking the <Link to={DEFAULT_TERM_PATH}> on the error page at src/App.tsx:240-245), the TermPage component re-renders rather than remounts because the same <Route path="/:term"> matches. This means loadWorkspaces(termPath) and loadWorkspaceIdx(termPath) are never called for the new term.

Worse, the persistence useEffect at line 101-104 fires immediately with the new termPath but the old workspaces value, overwriting the new term's localStorage data with the previous term's workspace state. This corrupts the user's saved workspaces for the destination term.

Reproduction path

1. Visit /badterm — TermPage mounts, workspace state initialized for /badterm (empty defaults)
2. Error page appears with a Link to /sp2026
3. Click the link — React Router client-side nav, TermPage re-renders with term="sp2026"
4. useWorkspaceState("/sp2026", ...) keeps the old empty defaults in memory
5. saveWorkspaces("/sp2026", oldEmptyDefaults) overwrites any previously saved sp2026 data
6. User sees empty workspaces instead of their saved ones

Prompt for agents

In src/hooks/useWorkspaceState.ts, add a useEffect that watches termPath and reinitializes the workspace state when it changes. Something like:

useEffect(() => {
  setWorkspaces(loadWorkspaces(termPath));
  setWorkspaceIdxRaw(loadWorkspaceIdx(termPath));
}, [termPath]);

Place this after the useState declarations (around line 98) and before the persistence useEffect. Make sure the persistence useEffect doesn't fire synchronously with stale data during the term change — you may need to gate the save behind a check that the state has been initialized for the current term, or use a ref to track whether a term switch is in progress.

Alternatively, add a key prop to the TermPage element in the route definition in src/App.tsx to force remounting on term change. This is simpler but slightly less efficient. In App() around line 282, change:
  <Route path="/:term" element={<TermPage />} />
to something that forces a remount on term change. One approach is to create a wrapper component that reads useParams and passes key={term} to TermPage. But the useEffect approach in the hook is more targeted.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🔴 exportICS crashes with TypeError for terms missing from term_start_dates.json

exportICS at src/lib/ics.ts:53-57 accesses TERM_START_DATES[term] without checking if the key exists, then immediately calls .split("-") on the result. The term_start_dates.json file only contains entries for fa2025 through sp2026, but the app supports terms back to FA2022. If a user on an older term route (e.g., /fa2023) clicks "Export .ics" in src/Workspace.tsx:617, this throws TypeError: Cannot read properties of undefined (reading 'split'), crashing the UI with no error handling.

Suggested change

	export function exportICS(term: string, courses: CourseStorage[]): string {
	const [termStartYear, termStartMonth, termStartDay] = (
	TERM_START_DATES as { [key: string]: string }
	)[term]
	.split("-")
	.map(Number);
	export function exportICS(term: string, courses: CourseStorage[]): string {
	const termStartStr = (TERM_START_DATES as { [key: string]: string })[term];
	if (!termStartStr) {
	throw new Error(`No start date configured for term: ${term}`);
	}
	const [termStartYear, termStartMonth, termStartDay] = termStartStr
	.split("-")
	.map(Number);

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 Deploy workflow lacks build verification before deploy

The deploy workflow at .github/workflows/deploy.yml:50-58 runs bun run build then immediately deploys via wrangler deploy. Unlike the CI workflow, there's no lint or test step before deployment. While CI runs on all pushes (including to main), the deploy workflow uses concurrency: cancel-in-progress: true which could theoretically race with CI — a deploy could start and complete before CI finishes, deploying code that hasn't passed checks. In practice, both workflows start simultaneously from the same push event, so the deploy usually finishes first.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[Copilot]

Pull request overview

This PR modernizes the app’s deployment + runtime structure by moving to Cloudflare-based SPA routing, extracting/persisting workspace state into dedicated modules, and improving UI accessibility/mobile responsiveness.

Changes:

• Introduces React Router-based routing with term-specific pages and new error/not-found pages.
• Refactors workspace localStorage persistence and state reconciliation into src/lib/persistence.ts and src/hooks/useWorkspaceState.ts.
• Extracts .ics export into src/lib/ics.ts with accompanying Vitest coverage; adds multiple UI accessibility and mobile CSS improvements.

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/lib/scheduling.ts	Adds helper to clear unlocked section selections.
src/lib/persistence.ts	New localStorage persistence helpers for workspaces and active index.
src/lib/ics.ts	New .ics exporter used by Workspace UI.
src/lib/ics.test.ts	Adds unit tests for .ics export behaviors.
src/index.tsx	Wraps app in a router.
src/hooks/useWorkspaceState.ts	New centralized workspace state hook + persistence.
src/Workspace.tsx	Uses extracted .ics export and adds accessibility/tab semantics improvements.
src/Workspace.test.tsx	Adds component tests for Workspace UI interactions.
src/Planner.test.tsx	Adds component tests for Planner rendering behavior.
src/Modal.tsx / src/Modal.test.tsx	Improves modal focus/keyboard accessibility and updates tests.
src/App.tsx / src/App.test.tsx	Replaces custom routing with React Router and adds routing/error-state tests.
src/css/*.css	Mobile responsiveness tweaks + skip-link styling + modal width improvements.
public/404.html / index.html	Removes GitHub Pages SPA redirect hack.
package.json	Adds React Router dependency.
README.md / AGENTS.md / .cursor/...	Updates docs to reflect Bun/CI/CD/Cloudflare deployment.
.github/workflows/deploy.yml	Adds Cloudflare deploy workflow.
.gitignore	Ignores package-lock.json and normalizes Icon entry.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

termPath is normalized to lowercase for data loading/storage, but the UI continues to use the raw term param (e.g., WorkspacePanel term={term ?? ""} and footer/loader display). For routes like /SP2026, this passes an uppercase term into Workspace, which then does DEFAULT_COURSES[term.substring(0, 2)] and will throw because the keys are lowercase. Use a single normalized term string (lowercased, without the leading /) consistently for display and for the WorkspacePanel prop.

[Copilot]

BrowserRouter is imported from react-router, but in React Router’s DOM bindings it is typically provided by react-router-dom. With only react-router in package.json, this import is likely to fail at build/runtime. Consider adding react-router-dom and importing BrowserRouter from there (and similarly update other DOM components like Link/MemoryRouter if needed).

Suggested change

	import { BrowserRouter } from "react-router";
	import { BrowserRouter } from "react-router-dom";

[Copilot]

MemoryRouter is imported from react-router. If the app switches to the DOM package (needed for BrowserRouter/Link), the test should import MemoryRouter from react-router-dom as well to match the runtime router implementation and avoid version/export mismatches.

Suggested change

	import { MemoryRouter } from "react-router";
	import { MemoryRouter } from "react-router-dom";

[Copilot]

ICS text fields should escape special characters per RFC 5545 (notably backslashes, commas, semicolons, and newlines). SUMMARY:${event.name} and LOCATION:${event.location} are currently injected raw, so course numbers/locations containing commas or newlines can produce an invalid .ics file in some calendar clients. Add an escaping helper and apply it to text properties (at least SUMMARY/LOCATION, and any future DESCRIPTION fields).

[Copilot]

exportICS selects the active section via section.number - 1 === course.sectionId, but throughout the app sectionId is treated as the index into courseData.sections (e.g., SectionDropdown stores findIndex(...), and getSelectedSection indexes by sectionId). This will export the wrong section whenever section numbers aren’t a simple 1..N mapping or sections are reordered. Filter by array index (or reuse the same “selected section” lookup logic) instead of section.number - 1.

Suggested change

	.filter(section => section.number - 1 === course.sectionId) // Filter by selected section
	.filter((section, index) => index === course.sectionId) // Filter by selected section using index

[Copilot]

useWorkspaceState initializes workspaces/workspaceIdx from termPath only once. If the user navigates between terms in-app (e.g., /sp2026 -> /fa2026), the hook will keep the previous term’s state but start saving it under the new termPath key due to the persistence effect, corrupting localStorage. Add an effect that resets state when termPath changes (and avoid persisting until after re-hydration), or otherwise force a remount keyed by termPath.

Suggested change

	// Persist to localStorage on every change
	useEffect(() => {
	saveWorkspaces(termPath, workspaces);
	saveWorkspaceIdx(termPath, workspaceIdx);
	}, [workspaces, workspaceIdx, termPath]);
	// Track which term the in-memory state has been hydrated for
	const [hydratedTermPath, setHydratedTermPath] = useState(termPath);
	// Re-hydrate state from persistence whenever the term changes
	useEffect(() => {
	const nextWorkspaces = loadWorkspaces(termPath);
	const nextWorkspaceIdx = loadWorkspaceIdx(termPath);
	setWorkspaces(nextWorkspaces);
	setWorkspaceIdxRaw(nextWorkspaceIdx);
	setHydratedTermPath(termPath);
	}, [termPath]);
	// Persist to localStorage on every change, but only after re-hydration
	useEffect(() => {
	if (hydratedTermPath !== termPath) return;
	saveWorkspaces(termPath, workspaces);
	saveWorkspaceIdx(termPath, workspaceIdx);
	}, [workspaces, workspaceIdx, termPath, hydratedTermPath]);

[Copilot]

activeWorkspace is read as workspaces[workspaceIdx] without validating that workspaceIdx is within bounds. Since loadWorkspaceIdx() accepts any parsed number (including corrupted/tampered values), this can become undefined and crash on activeWorkspace.courses. Consider clamping workspaceIdx to [0, workspaces.length - 1] (or falling back to 0 when invalid) before indexing.

[Copilot]

loadWorkspaces() returns whatever JSON is stored (any length/shape) and loadWorkspaceIdx() returns any parsed number. If localStorage is corrupted, you can end up with fewer than 5 workspaces or an out-of-range index and the UI will crash. Consider validating the parsed values (ensure array length is NUM_WORKSPACES, pad/truncate as needed, and clamp workspaceIdx to a valid integer).

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 6 additional findings in Devin Review.

[devin-ai-integration]

🚩 exportICS will crash for terms without start date entries

In src/lib/ics.ts:52-57, exportICS accesses TERM_START_DATES[term] and calls .split("-") on it. The term_start_dates.json only contains entries for fa2025 through sp2026, but the app supports terms back to FA2022. If a user viewing an older term clicks "Export .ics", this will throw a TypeError (cannot read .split of undefined). This is a pre-existing issue (carried over unchanged from the old Workspace.tsx implementation), not introduced by this PR, but worth noting since the code was moved to a new module.

---

Was this helpful? React with 👍 or 👎 to provide feedback.