Restructure project as pnpm monorepo with full MVP#1
Merged
nathanialhenniges merged 32 commits intomainfrom Feb 20, 2026
Merged
Restructure project as pnpm monorepo with full MVP#1nathanialhenniges merged 32 commits intomainfrom
nathanialhenniges merged 32 commits intomainfrom
Conversation
Move all planning markdown files into notes/ folder with TL;DR sections for quick scanning. Add iCal test fixtures for import feature development. Update CLAUDE.md to reference notes/ and fix ConPaws+ naming.
Set up workspace configuration with apps/mobile, apps/web, and packages/supabase. Update root package.json, pnpm-workspace.yaml, and add shared tsconfig base. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove README.md and .vscode config that belonged to the flat project structure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These files have been relocated to apps/mobile/ as part of the monorepo migration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These have been relocated to apps/mobile/src/ in the monorepo structure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These have been relocated to apps/mobile/src/components/ in the monorepo. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These have been relocated to apps/mobile/src/ in the monorepo structure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Onboarding, tabs, and root layouts relocated to apps/mobile/src/app/. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Assets relocated to apps/mobile/src/assets/. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Set up apps/mobile/ with Expo config, EAS build config, ESLint, Metro, NativeWind, TypeScript, Drizzle, and Vitest configuration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add shared utilities, constants, icon definitions, color scheme hook, date helpers, i18n setup with English translations, and TypeScript types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add Drizzle ORM schema for conventions and events tables, database client setup, and repository modules for data access. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Core convention import functionality: iCal (.ics) file parsing, Sched URL validation, and convention data export/import utilities. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Badge, Button, Card, Input, Separator, Switch, Text, and Typography components built with NativeWind styling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Convention cards, event list items, day headers, empty states, onboarding slides, settings groups/items, and status badges. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Onboarding context, convention/event hooks with TanStack Query, data transfer hooks, and iCal import hook. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Root layout, onboarding flow, tab navigation (home, profile, settings), convention CRUD screens, event CRUD screens, import flow, and about page. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
App icons, favicon, and splash screen images for the mobile app. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tests for iCal parser, Sched URL validation, date utilities, data export, and general utility functions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Scaffold apps/web/ (Next.js placeholder) and packages/supabase/ (config and migrations placeholder) for future development phases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Lint, type-check, and test pipeline for the monorepo. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update CLAUDE.md with monorepo architecture, add web deployment docs, and refresh planning and marketing notes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughRestructures into a pnpm monorepo with Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant ImportScreen as Import Screen
participant Hook as useICalImport Hook
participant Parser as ICS Parser
participant Importer as Import Workflow
participant DB as Drizzle DB
participant Cache as React Query Cache
User->>ImportScreen: Choose .ics file or paste Sched URL
ImportScreen->>Hook: pickAndParseFile() or fetchAndParseUrl()
Hook->>Parser: parseICalContent(icsData)
Parser-->>Hook: ParsedCalendar
Hook-->>ImportScreen: show preview
User->>ImportScreen: Select events & tap Import
ImportScreen->>Importer: importEvents(calendar, indices)
Importer->>DB: ensure convention (create if needed)
Importer->>DB: bulkCreateEvents(filtered, dedupe by sourceUid)
DB-->>Importer: success
Importer->>Cache: invalidate ["conventions","events"]
Importer-->>ImportScreen: complete
ImportScreen->>User: navigate to convention view
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![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
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (19)
apps/mobile/src/app/convention/[id]/event/[eventId]/edit.tsx-48-69 (1)
48-69:⚠️ Potential issue | 🟠 MajorMissing error handling on
mutateAsyncand no end-time validation.Two issues:
- If
mutateAsyncrejects, the error is unhandled — the user gets no feedback. Wrap in try/catch or use.mutate()withonError.- Unlike
NewConventionScreen(which validatesendDate >= startDate), this screen doesn't validate thatendTimeis afterstartTime.Proposed fix
const handleSave = async () => { if (!title.trim()) { Alert.alert("Error", "Please enter an event title."); return; } + if (endTime <= startTime) { + Alert.alert("Error", "End time must be after start time."); + return; + } + + try { await updateEvent.mutateAsync({ id: eventId, conventionId: id, data: { title: title.trim(), description: description.trim() || null, location: location.trim() || null, room: room.trim() || null, category: category.trim() || null, startTime: startTime.toISOString(), endTime: endTime.toISOString(), }, }); - router.back(); + } catch { + Alert.alert("Error", "Failed to save changes. Please try again."); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/convention/`[id]/event/[eventId]/edit.tsx around lines 48 - 69, The handleSave function currently calls updateEvent.mutateAsync without error handling and does not validate that endTime is after startTime; modify handleSave to first validate that endTime > startTime (compare startTime and endTime Date objects and show Alert.alert("Error", "End time must be after start time.") and return if invalid), then wrap the updateEvent.mutateAsync call in a try/catch (or use updateEvent.mutate with onError) so failed requests are caught and the user is informed via Alert.alert with the error message; only call router.back() after a successful mutation inside the try block (or in the onSuccess handler).apps/mobile/src/app/convention/new.tsx-40-47 (1)
40-47:⚠️ Potential issue | 🟠 MajorUnhandled
mutateAsyncrejection.Same issue as the edit screen — if the mutation fails, the user sees no error feedback and
router.back()is never reached. Wrap in try/catch or switch to.mutate()with anonErrorcallback.Proposed fix
+ try { await createConvention.mutateAsync({ name: name.trim(), startDate: startDate.toISOString(), endDate: endDate.toISOString(), }); - router.back(); + } catch { + Alert.alert("Error", "Failed to create convention. Please try again."); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/convention/new.tsx` around lines 40 - 47, The createConvention.mutateAsync call in the new convention handler is unhandled and can leave the user with no feedback and prevent router.back() from running; wrap the call to createConvention.mutateAsync({...}) in a try/catch (or switch to createConvention.mutate with an onError callback) so errors are caught, display user-visible feedback on failure (e.g., set an error state or trigger a toast) and only call router.back() after a successful mutation; reference createConvention.mutateAsync and router.back to locate and update the handler.apps/mobile/src/lib/__tests__/data-export.test.ts-3-14 (1)
3-14: 🛠️ Refactor suggestion | 🟠 MajorTests validate a local copy, not the production
validateExportData.The inline validator duplicates
apps/mobile/src/lib/data-import.ts:12-24but is decoupled from it. If the production validator changes (e.g., adds a required field), these tests will still pass — giving false confidence. Consider extracting the pure validation logic into a standalone module (noexpo-sqlitedependency) that bothdata-import.tsand this test can import, so you're actually testing the real code path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/lib/__tests__/data-export.test.ts` around lines 3 - 14, Tests currently define a local duplicate of the validator (validateExportData) instead of using the production validator, so extract the pure validation logic into a small standalone module (e.g., export a validateExportData function from a new module) that has no expo-sqlite dependency, replace the inline validator in the test with an import of that exported validateExportData, and update apps/mobile/src/lib/data-import.ts to import and use the same validateExportData so both runtime and tests share the identical validation logic.apps/mobile/src/lib/__tests__/date-utils.test.ts-71-97 (1)
71-97:⚠️ Potential issue | 🟠 MajorTimezone-dependent tests will fail in non-UTC environments.
groupEventsByDaycallsformat(parseISO(...), "yyyy-MM-dd")which uses the local timezone. The test fixtures use UTC timestamps, so the day grouping changes depending on where the test runs. For example, in UTC+10,"2026-07-02T10:00:00Z"becomes July 2 at 20:00, but"2026-07-03T14:00:00Z"becomes July 4 — producing 3 groups instead of the expected 2.The same issue affects the
formatTimetest (Line 28).Fix by setting
TZ=UTCin the Vitest config or CI environment, or by using timezone-independent test fixtures (e.g., dates at noon UTC so they stay on the same calendar day in all realistic timezones).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/lib/__tests__/date-utils.test.ts` around lines 71 - 97, The tests fail in non-UTC environments because groupEventsByDay uses format(parseISO(...), "yyyy-MM-dd") (local timezone) and formatTime is likewise timezone-dependent; fix by either setting the test runtime to UTC (set TZ=UTC in the Vitest config or CI environment) or make the test fixtures timezone-independent (e.g., change event timestamps used in the tests to noon UTC or otherwise ensure dates won’t shift across timezones) and update the expectations accordingly for groupEventsByDay and formatTime.apps/mobile/src/locales/en.json-29-35 (1)
29-35: 🛠️ Refactor suggestion | 🟠 MajorAuth strings defined here but hardcoded in
auth.tsx.The onboarding auth screen (
apps/mobile/src/app/(onboarding)/auth.tsx) renders hardcoded English strings ("Create an Account", "Skip for now", etc.) instead of consuming these locale keys. The same applies tofeatures.tsx,about.tsx, andevent-item.tsx(e.g., "18+ Only" on Line 89 ofevent-item.tsxvsevent.ageRestrictedhere). This undermines the purpose of having an i18n file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/locales/en.json` around lines 29 - 35, Replace hardcoded English strings in the onboarding and UI components with the corresponding i18n keys from en.json: in apps/mobile/src/app/(onboarding)/auth.tsx swap "Create an Account", "Skip for now", "Continue with Google", "Continue with Apple" for the locale keys auth.title, auth.description, auth.google, auth.apple, auth.skip; do the same in features.tsx and about.tsx using the appropriate keys from the locales file; in event-item.tsx replace "18+ Only" with event.ageRestricted. Use the project's localization hook/utility (e.g., useTranslations or t) already used elsewhere in the codebase: import it at the top of each file, call it to resolve the keys (e.g., const t = useTranslations(); t('auth.title')) and remove the hardcoded literals so UI text comes from the en.json keys.apps/mobile/src/contexts/onboarding-context.tsx-34-39 (1)
34-39:⚠️ Potential issue | 🟠 MajorMissing error handling for
AsyncStorage.getItem— app can get stuck in loading state.If
AsyncStorage.getItemrejects, the.thencallback never fires, soisLoadingremainstrueindefinitely and the app could hang on a loading/splash screen. Add a.catchto handle failures gracefully.Proposed fix
useEffect(() => { AsyncStorage.getItem(STORAGE_KEY).then((value) => { setIsOnboardingComplete(value === "true"); setIsLoading(false); - }); + }).catch(() => { + setIsOnboardingComplete(false); + setIsLoading(false); + }); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/contexts/onboarding-context.tsx` around lines 34 - 39, The useEffect that calls AsyncStorage.getItem(STORAGE_KEY) lacks error handling and can leave isLoading true if the promise rejects; update the effect to add a .catch (or .finally) on AsyncStorage.getItem so any rejection sets setIsOnboardingComplete(false) or a safe default and always calls setIsLoading(false); specifically modify the AsyncStorage.getItem(...).then(...) chain in the useEffect so rejections are handled (use AsyncStorage.getItem, .then -> setIsOnboardingComplete(value === "true"), and in .catch -> setIsOnboardingComplete(false) and log the error) or move setIsLoading(false) into a .finally to guarantee the loading flag is cleared.apps/mobile/src/components/settings-item.tsx-24-26 (1)
24-26:⚠️ Potential issue | 🟠 MajorRemove or replace
last:border-b-0—NativeWind v4 does not support thelast:modifier on React Native.The
last:pseudo-class modifier was removed in NativeWind v4 and is not currently supported. The last item in eachSettingsGroupwill retain its bottom border. Use a manual workaround: when rendering items, conditionally apply a differentclassName(without the bottom border) when rendering the final item (e.g., checkindex === items.length - 1).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/components/settings-item.tsx` around lines 24 - 26, The Pressable uses the unsupported NativeWind v4 modifier `last:border-b-0`; update the rendering logic so the bottom-border class is applied conditionally instead of using `last:`: in the component that maps items (e.g., SettingsGroup) pass an `isLast` boolean (or derive via index === items.length - 1) into SettingsItem, and in SettingsItem toggle the `border-b` class on the Pressable's className (remove `last:border-b-0` and only include `border-b border-border` when isLast is false) so the final item renders without the bottom border.apps/mobile/src/app/convention/import.tsx-19-37 (1)
19-37:⚠️ Potential issue | 🟠 MajorPassing large parsed calendar data via route params risks exceeding URL size limits.
JSON.stringify(parsed)for a convention with hundreds of events can produce a very large string. Expo Router params are serialized into the navigation state, and excessively large params can cause performance issues or crashes on some platforms.Consider storing the parsed calendar in a shared ref, context, or the existing
useICalImporthook state (which already holdsparsedCalendar) and reading it from there in the preview screen, rather than serializing it into route params.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/convention/import.tsx` around lines 19 - 37, The handlers handleFilePick and handleUrlFetch currently serialize the entire parsed object into router params (JSON.stringify(parsed)); instead, save the parsed calendar into shared state (e.g., useICalImport's parsedCalendar or a ref/context) after calling pickAndParseFile or fetchAndParseUrl and then call router.push to "/convention/import-preview" with a lightweight identifier (or the original URL) only; update the preview screen to read the parsed calendar from useICalImport.parsedCalendar (or the shared ref/context) rather than from route params.apps/mobile/src/app/convention/[id]/edit.tsx-37-53 (1)
37-53:⚠️ Potential issue | 🟠 MajorSame issues as NewEventScreen: unhandled mutation error and no date validation.
mutateAsynccan throw without user feedback, andendDatecould precedestartDateif the user adjusts the start date picker after selecting the end date.Proposed fix
const handleSave = async () => { if (!name.trim()) { Alert.alert("Error", "Please enter a convention name."); return; } + if (endDate < startDate) { + Alert.alert("Error", "End date must be on or after start date."); + return; + } + + try { await updateConvention.mutateAsync({ id, data: { name: name.trim(), startDate: startDate.toISOString(), endDate: endDate.toISOString(), }, }); router.back(); + } catch { + Alert.alert("Error", "Failed to save changes. Please try again."); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/convention/`[id]/edit.tsx around lines 37 - 53, handleSave currently calls updateConvention.mutateAsync without catching errors and doesn't validate dates; change handleSave to first validate that endDate is not before startDate (compare startDate and endDate and Alert the user if invalid), then wrap the await updateConvention.mutateAsync({...}) call in a try/catch so any thrown error is caught and surfaced to the user (use Alert.alert with a clear error message), and optionally return early on error; keep references to name, startDate, endDate, updateConvention.mutateAsync and router.back() so you validate inputs before calling mutateAsync and only call router.back() after a successful mutation.apps/mobile/src/app/convention/[id]/event/new.tsx-34-53 (1)
34-53:⚠️ Potential issue | 🟠 MajorUnhandled mutation error and missing start/end time validation.
Two issues in
handleSave:
mutateAsynccan throw, but there's no try/catch — the user won't see any feedback on failure and you'll get an unhandled promise rejection.- When the user changes
startTimeto a value after the currentendTime, the form happily submits an inverted range. TheminimumDateon the end-picker only constrains future end-date selections, not already-selected values.Proposed fix
const handleSave = async () => { if (!title.trim()) { Alert.alert("Error", "Please enter an event title."); return; } + if (endTime <= startTime) { + Alert.alert("Error", "End time must be after start time."); + return; + } + + try { await createEvent.mutateAsync({ conventionId: id, title: title.trim(), description: description.trim() || null, location: location.trim() || null, room: room.trim() || null, category: category.trim() || null, startTime: startTime.toISOString(), endTime: endTime.toISOString(), type: "custom", }); router.back(); + } catch { + Alert.alert("Error", "Failed to create event. Please try again."); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/convention/`[id]/event/new.tsx around lines 34 - 53, In handleSave, add validation to ensure startTime <= endTime (e.g., compare startTime.getTime() and endTime.getTime()) and short-circuit with Alert.alert if the range is invalid, and wrap the createEvent.mutateAsync(...) call in a try/catch so any thrown mutation error is caught; on success call router.back(), on failure show an error Alert with the caught error message (or a friendly fallback) to avoid unhandled promise rejections and give user feedback (refer to handleSave and createEvent.mutateAsync).apps/mobile/src/app/_layout.tsx-21-39 (1)
21-39:⚠️ Potential issue | 🟠 MajorRace condition: navigation fires before database is initialized.
The first
useEffect(Lines 21-31) can callrouter.replace("/(tabs)")whileinitDatabase()(Lines 33-39) hasn't completed yet. When the tabs screen mounts, its queries will execute against a database whose tables may not exist, causing runtime errors. The splash screen hides only afterinitDatabaseresolves, but component mounting and data fetching happen immediately on navigation.Consider gating navigation on DB readiness as well:
Proposed fix — track DB init state
function RootNavigator() { const { isOnboardingComplete, isLoading } = useOnboarding(); const segments = useSegments(); const router = useRouter(); + const [isDbReady, setIsDbReady] = useState(false); useEffect(() => { - if (isLoading) return; + if (isLoading || !isDbReady) return; const inOnboarding = segments[0] === "(onboarding)"; if (!isOnboardingComplete && !inOnboarding) { router.replace("/(onboarding)/welcome"); } else if (isOnboardingComplete && inOnboarding) { router.replace("/(tabs)"); } - }, [isOnboardingComplete, isLoading, segments]); + }, [isOnboardingComplete, isLoading, segments, router, isDbReady]); useEffect(() => { if (!isLoading) { initDatabase().then(() => { + setIsDbReady(true); SplashScreen.hideAsync(); }); } }, [isLoading]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/_layout.tsx` around lines 21 - 39, The navigation useEffect can run before initDatabase finishes; modify the logic to gate routing on DB readiness by introducing a local state (e.g., dbReady) that is set true when initDatabase() resolves (the existing initDatabase().then(() => { SplashScreen.hideAsync(); setDbReady(true); })). Update the routing effect (the useEffect referencing isOnboardingComplete, isLoading, segments) to early-return unless !isLoading && dbReady, and then perform the same router.replace("/(onboarding)/welcome") or router.replace("/(tabs)"); ensure initDatabase is only called once (keep the existing initDatabase call in the second useEffect) and reference initDatabase, SplashScreen.hideAsync, router.replace, isOnboardingComplete, isLoading, segments, and the new dbReady state to locate where to change the code.apps/mobile/src/app/convention/[id]/event/[eventId]/index.tsx-40-55 (1)
40-55:⚠️ Potential issue | 🟠 MajorUnhandled rejection in
handleDelete'sonPresscallback.
deleteEvent.mutateAsync(...)at Line 47 can reject, but theonPressasync callback has notry/catch. This will surface as an unhandled promise rejection. Consider wrapping intry/catchwith an error alert, or switching todeleteEvent.mutate(...)with anonSuccesscallback that callsrouter.back().Proposed fix using mutate + onSuccess
const handleDelete = () => { Alert.alert("Delete Event", "Are you sure?", [ { text: "Cancel", style: "cancel" }, { text: "Delete", style: "destructive", - onPress: async () => { - await deleteEvent.mutateAsync({ + onPress: () => { + deleteEvent.mutate({ id: eventId, conventionId: id, + }, { + onSuccess: () => router.back(), + onError: () => Alert.alert("Error", "Could not delete event."), }); - router.back(); }, }, ]); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/convention/`[id]/event/[eventId]/index.tsx around lines 40 - 55, The onPress async callback in handleDelete calls deleteEvent.mutateAsync(...) without error handling which can cause unhandled promise rejections; fix by either wrapping the await deleteEvent.mutateAsync({...}) call in a try/catch and show an Alert.alert on error before/after calling router.back(), or replace mutateAsync with deleteEvent.mutate({...}) and move router.back() into the mutate's onSuccess callback to ensure failures are handled; update the handleDelete function accordingly (references: handleDelete, deleteEvent.mutateAsync / deleteEvent.mutate, router.back).apps/mobile/src/lib/useColorScheme.tsx-38-45 (1)
38-45:⚠️ Potential issue | 🟠 MajorMissing error handler on
AsyncStorage.getItem— app may never render.If
AsyncStorage.getItemrejects (e.g., storage corruption), there's no.catch(), sosetIsLoaded(true)is never called and the provider returnsnullindefinitely, leaving the app with a blank screen.Proposed fix
useEffect(() => { AsyncStorage.getItem(STORAGE_KEY).then((value) => { if (value === "light" || value === "dark" || value === "system") { setPreferenceState(value); } setIsLoaded(true); - }); + }).catch(() => { + setIsLoaded(true); + }); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/lib/useColorScheme.tsx` around lines 38 - 45, The useEffect that calls AsyncStorage.getItem(STORAGE_KEY) lacks error handling so a reject prevents setIsLoaded(true) from running; wrap the call with a .catch(...) (or use an async IIFE with try/catch/finally) to handle errors and ensure setIsLoaded(true) always runs (in finally), and keep existing setPreferenceState(value) logic when value is valid; update the effect containing AsyncStorage.getItem, setPreferenceState, and setIsLoaded so rejections are logged/handled and the provider won't hang.apps/mobile/src/hooks/use-data-transfer.ts-62-71 (1)
62-71:⚠️ Potential issue | 🟠 MajorUnhandled errors in the async
onPresscallback.The
onPresshandler at Line 64 is anasyncfunction, but it runs afterAlert.alertreturns — meaning exceptions fromimportData(data)orqueryClient.invalidateQueries()escape the outertry/catchblock and become unhandled promise rejections.Proposed fix: wrap the onPress body in its own try/catch
{ text: "Import", onPress: async () => { + try { const summary = await importData(data); queryClient.invalidateQueries({ queryKey: ["conventions"] }); Alert.alert( "Import Complete", `Added ${summary.conventionsAdded} conventions and ${summary.eventsAdded} events. Skipped ${summary.conventionsSkipped} duplicate conventions and ${summary.eventsSkipped} duplicate events.`, ); + } catch { + Alert.alert("Import Failed", "Could not import data. Please try again."); + } }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/hooks/use-data-transfer.ts` around lines 62 - 71, The async onPress handler in use-data-transfer.ts can throw unhandled promise rejections from importData(data) or queryClient.invalidateQueries(); wrap the entire onPress body in a try/catch so all awaits (including awaiting queryClient.invalidateQueries(...)) are inside the try, and in the catch log the error and show a user-facing Alert.alert (e.g., "Import Failed" with error.message) so failures are handled gracefully; locate the anonymous onPress async function, the importData call, and queryClient.invalidateQueries to apply this change.apps/mobile/src/app/convention/[id].tsx-25-41 (1)
25-41:⚠️ Potential issue | 🟠 MajorUnhandled rejection in the delete flow — a failed
mutateAsyncwill silently swallow the error.If
deleteConvention.mutateAsync(id)rejects (e.g., DB error), the error propagates as an unhandled promise rejection inside the AlertonPresscallback. The user sees nothing and the convention remains. Add a try-catch with user feedback.🛡️ Proposed fix
{ text: "Delete", style: "destructive", onPress: async () => { - await deleteConvention.mutateAsync(id); - router.back(); + try { + await deleteConvention.mutateAsync(id); + router.back(); + } catch { + Alert.alert("Error", "Failed to delete convention. Please try again."); + } }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/convention/`[id].tsx around lines 25 - 41, The delete flow in handleDelete uses deleteConvention.mutateAsync(id) inside Alert.alert onPress without a try/catch, causing unhandled promise rejections and no user feedback on failure; wrap the async call in a try/catch within the onPress handler (referencing handleDelete and deleteConvention.mutateAsync) and on success call router.back(), but on failure show a user-visible error (e.g., another Alert.alert or toast) and optionally log the error to aid debugging.apps/mobile/src/db/repositories/events.ts-42-60 (1)
42-60:⚠️ Potential issue | 🟠 Major
bulkCreateEventsissues individual inserts without a transaction — slow and non-atomic for large imports.A convention import can have hundreds of events. Inserting them one-by-one without a transaction means: (1) each insert is its own SQLite transaction (significant overhead), and (2) a failure partway through leaves a partial import. Wrap in
db.transaction().🛡️ Proposed fix
export async function bulkCreateEvents( events: Omit<NewConventionEvent, "id" | "createdAt" | "updatedAt">[], ): Promise<ConventionEvent[]> { const now = new Date().toISOString(); const created: ConventionEvent[] = []; - for (const data of events) { - const event: NewConventionEvent = { - id: uuid(), - ...data, - createdAt: now, - updatedAt: now, - }; - await db.insert(conventionEvents).values(event); - created.push(event as ConventionEvent); - } + await db.transaction(async (tx) => { + for (const data of events) { + const event: NewConventionEvent = { + id: uuid(), + ...data, + createdAt: now, + updatedAt: now, + }; + await tx.insert(conventionEvents).values(event); + created.push(event as ConventionEvent); + } + }); return created; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/db/repositories/events.ts` around lines 42 - 60, The function bulkCreateEvents currently performs individual db.insert calls per event causing many separate SQLite transactions and non-atomic imports; wrap the work in a single db.transaction and perform the inserts inside that transaction (use the transaction object, e.g. tx.insert(conventionEvents).values(eventsArray) or iterate with tx.insert for each event) so all inserts run in one atomic transaction and roll back on error; keep the existing id/createdAt/updatedAt generation (uuid(), now) but build the full events array first and then insert it inside db.transaction to improve performance and ensure atomicity.apps/mobile/src/lib/data-import.ts-27-65 (1)
27-65:⚠️ Potential issue | 🟠 MajorWrap the entire import in a transaction to prevent partial imports.
If an insert fails midway (e.g., FK constraint, disk error), you'll have orphaned conventions without their events, or a subset of records imported with no way for the user to retry cleanly. Drizzle's SQLite driver supports
db.transaction().🛡️ Proposed fix
export async function importData(data: ExportData): Promise<ImportSummary> { const summary: ImportSummary = { conventionsAdded: 0, conventionsSkipped: 0, eventsAdded: 0, eventsSkipped: 0, }; - for (const convention of data.conventions) { - const existing = await db - .select() - .from(conventions) - .where(eq(conventions.id, convention.id)); - - if (existing.length > 0) { - summary.conventionsSkipped++; - continue; + await db.transaction(async (tx) => { + for (const convention of data.conventions) { + const existing = await tx + .select() + .from(conventions) + .where(eq(conventions.id, convention.id)); + + if (existing.length > 0) { + summary.conventionsSkipped++; + continue; + } + + await tx.insert(conventions).values(convention); + summary.conventionsAdded++; } - await db.insert(conventions).values(convention); - summary.conventionsAdded++; - } - - for (const event of data.events) { - const existing = await db - .select() - .from(conventionEvents) - .where(eq(conventionEvents.id, event.id)); - - if (existing.length > 0) { - summary.eventsSkipped++; - continue; + for (const event of data.events) { + const existing = await tx + .select() + .from(conventionEvents) + .where(eq(conventionEvents.id, event.id)); + + if (existing.length > 0) { + summary.eventsSkipped++; + continue; + } + + await tx.insert(conventionEvents).values(event); + summary.eventsAdded++; } - - await db.insert(conventionEvents).values(event); - summary.eventsAdded++; - } + }); return summary; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/lib/data-import.ts` around lines 27 - 65, The importData function currently performs multiple selects/inserts directly on db which can lead to partial imports; wrap the entire import logic in a single transaction using db.transaction(...) so the work is atomic. Inside the transaction callback use the transaction handle (e.g., tx) in place of db for all operations on conventions and conventionEvents (selects and inserts) so that any failure rolls back; preserve building and returning the ImportSummary at the end of the transaction. Ensure you keep references to the same types ExportData and ImportSummary and the function name importData unchanged.apps/mobile/src/app/convention/import-preview.tsx-26-26 (1)
26-26:⚠️ Potential issue | 🟠 Major
JSON.parse(data)can crash if the navigation param is missing or malformed.
datacomes fromuseLocalSearchParamsand will beundefinedat runtime if the param isn't supplied (TypeScript generics don't enforce runtime presence). A malformed string also throws. Wrap in try-catch and handle the failure gracefully (e.g., show an error or navigate back).🛡️ Proposed fix
- const calendar: ParsedCalendar = useMemo(() => JSON.parse(data), [data]); + const calendar: ParsedCalendar | null = useMemo(() => { + try { + return JSON.parse(data); + } catch { + return null; + } + }, [data]); + + if (!calendar) { + router.back(); + return null; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/convention/import-preview.tsx` at line 26, The current useMemo call that sets calendar via JSON.parse(data) can throw when data is undefined or malformed; update the hook around ParsedCalendar calendar to first check typeof data === 'string', wrap JSON.parse in try-catch inside useMemo (or move parsing into a safe parser function), and on parse failure set a safe fallback (null or an empty ParsedCalendar) and/or set an error state so the component can show an error UI or navigate back; ensure references to calendar and any downstream code handle the null/err state.apps/mobile/src/db/repositories/events.ts-2-2 (1)
2-2:⚠️ Potential issue | 🟠 MajorAdd crypto polyfill for
uuidv11 compatibility in Expo.The
uuidv11.1.0 package requiresglobalThis.crypto.getRandomValues, which is not available by default in Expo / React Native. Without a polyfill, the code will fail at runtime whenuuid()is called increateEvent()andbulkCreateEvents().Install
expo-cryptoand add this polyfill at the very top ofapps/mobile/src/app/_layout.tsxbefore other imports:import * as ExpoCrypto from 'expo-crypto'; if (!globalThis.crypto) globalThis.crypto = {} as any; globalThis.crypto.getRandomValues = ExpoCrypto.getRandomValues;Then add
expo-cryptoto dependencies:npx expo install expo-crypto🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/db/repositories/events.ts` at line 2, The app crashes at runtime because uuid v11 needs globalThis.crypto.getRandomValues; add the polyfill by installing expo-crypto (npx expo install expo-crypto) and then at the very top of apps/mobile/src/app/_layout.tsx before any other imports, import ExpoCrypto and set globalThis.crypto (if missing) and assign globalThis.crypto.getRandomValues = ExpoCrypto.getRandomValues; this will ensure calls to uuid() in functions like createEvent and bulkCreateEvents in apps/mobile/src/db/repositories/events.ts work correctly in Expo/React Native.
Remove unused imports/variables across components, hooks, and tests. Escape apostrophe in JSX, add missing dependency to useEffect, fix FileSystem.cacheDirectory to documentDirectory, and suppress false-positive i18next lint warning. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove unused CATEGORY_COLORS import from event detail screen. Suppress false-positive import/namespace lint error for expo-file-system cacheDirectory which is a valid runtime export. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix Badge children type (number to string template) in import-preview - Use expo-file-system/legacy import for cacheDirectory compatibility - Cast ical.js getFirstPropertyValue returns to string - Handle ColorSchemeName "unspecified" value in useColorScheme Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Upgrade expo to 55.0.0-preview.11 (React Native 0.83.1, React 19.2) - Upgrade all Expo packages to SDK 55 compatible versions - Upgrade NativeWind v4 to v5 preview (Tailwind v4 compatible) - Replace react-native-css-interop with react-native-css - Add react-native-worklets (required by reanimated v4.2) - Remove newArchEnabled config (new arch is default in SDK 55) - Remove implicit @babel/core from devDependencies - Update global.css imports for NativeWind v5 - Simplify metro.config.js withNativewind call - Upgrade TypeScript to 5.9, eslint-config-expo to 55 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (7)
apps/mobile/src/app/settings/about.tsx (2)
1-1: Consider using the project's customTextcomponent instead of the raw React NativeText.The project provides a
Textwrapper at@/components/ui/textthat applies defaulttext-base text-foregroundstyles. Importing directly fromreact-nativebypasses this, forcing you to repeat those classes manually on every<Text>(e.g., Lines 14, 26). Switching to the custom component would reduce duplication and stay consistent with the rest of the app's design system.♻️ Suggested change
-import { View, Text } from "react-native"; +import { View } from "react-native"; +import { Text } from "@/components/ui/text";After switching, you can drop the redundant
text-foreground/text-baseclasses from theTextelements that rely on the defaults.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/settings/about.tsx` at line 1, Replace the raw React Native Text import with the project's custom Text component from "@/components/ui/text" and update the import line in about.tsx to import View from "react-native" and Text from "@/components/ui/text"; then remove redundant class names like "text-base" and "text-foreground" from the About screen's <Text> usages (the instances currently showing those classes) so they rely on the default styling provided by the custom Text wrapper.
11-11: Hardcoded icon color won't adapt to theme changes.
#ffffffworks for a light-on-dark badge today, but if the primary color or theme ever changes, this will be out of sync. Consider deriving the color from the theme (e.g., viauseColorSchemeor a design-token constant) for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/settings/about.tsx` at line 11, The PawPrint icon in the About component uses a hardcoded color "#ffffff" which won't adapt to theme or primary color changes; update the component to derive the color from the app theme (for example use useColorScheme() or your design-token/theme hook like useTheme/useColors) and pass that value into the PawPrint color prop (or compute a dynamic variable such as badgeColor) so the icon follows dark/light modes and centralized color tokens instead of a literal string.apps/mobile/src/global.css (1)
31-53: Dark mode overrides look correct; consider contrast for--color-destructive.The
.darkclass override pattern is appropriate for NativeWind's dark mode handling. One minor observation:--color-destructivechanges from#ef4444(light) to#dc2626(dark), which is a darker red on a dark background. Typically, dark mode uses lighter/brighter variants for visibility. Verify this provides sufficient contrast against the dark--color-background:#0a0a0a``.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/global.css` around lines 31 - 53, The dark-mode destructive color (--color-destructive) in the .dark CSS block is darker (`#dc2626`) and may lack contrast against --color-background (`#0a0a0a`); update the .dark rule to use a lighter/brighter red (or the same light-mode value) that ensures sufficient contrast for visibility and accessibility, e.g., choose a higher luminance hex for --color-destructive or run a contrast check between --color-destructive and --color-background and adjust until it meets AA/AAA; modify the .dark { --color-destructive: ... } declaration accordingly while keeping the rest of the variables intact.apps/mobile/src/components/event-item.tsx (1)
40-40: Hardcoded icon colors repeated across components.The color values
#a8a29e(dark) and#78716c(light) for muted icons appear here and again in the event detail screen. Consider extracting these into a shared theme constant to keep them in sync.Also applies to: 47-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/components/event-item.tsx` at line 40, Extract the repeated hardcoded muted icon colors into a shared theme constant (e.g., export const MUTED_ICON_COLOR = { dark: "#a8a29e", light: "#78716c" } or add mutedIconColor to your existing theme object), then replace the inline color props on Clock in event-item.tsx (and the corresponding usage in the event detail screen) to use the theme constant based on isDark (e.g., color={theme.MUTED_ICON_COLOR[isDark ? 'dark' : 'light']}); update imports to pull the constant from the shared theme module and ensure both components reference the same identifier so the colors stay in sync.apps/mobile/src/app/convention/import-preview.tsx (1)
105-157: Large event lists rendered without virtualization.Calendar imports can contain hundreds of events. Using
ScrollViewwith.map()renders all items at once. Consider usingFlatListfor virtualized rendering to avoid jank on large imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/convention/import-preview.tsx` around lines 105 - 157, The current mapping over calendar.events renders all items eagerly (using calendar.events.map(...) and Pressable), which will jank for large imports; replace this with a virtualized FlatList: use FlatList with data={calendar.events}, keyExtractor={(item, index) => item.uid || String(index)}, and move the row JSX into a renderItem handler that calls toggleEvent(itemIndex) and reads selectedIndices, CATEGORY_COLORS, event.title, event.category, event.isAgeRestricted, and event.contentWarning so the UI and styles (the checkbox View, category dot, Texts, and numberOfLines) are preserved; ensure to pass extraData={selectedIndices} so selections update correctly.apps/mobile/src/lib/__tests__/date-utils.test.ts (1)
25-29:formatTimetest is also timezone-dependent.
formatTime("2026-07-02T14:30:00Z")yields different clock times depending on the runner's local timezone. The regex/\d{1,2}:\d{2}\s[AP]M/will match any valid time, so the test won't fail, but it also won't catch regressions in the formatting logic. This is acceptable as a smoke test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/lib/__tests__/date-utils.test.ts` around lines 25 - 29, The test for formatTime is timezone-dependent; make it deterministic by computing an expected value in a fixed timezone (or mocking the environment) and asserting equality instead of a loose regex. Update the test in date-utils.test.ts to call new Date("2026-07-02T14:30:00Z").toLocaleTimeString('en-US', { hour: 'numeric', minute: '2-digit', hour12: true, timeZone: 'UTC' }) (or mock Date/timezone) and compare that exact string to formatTime("2026-07-02T14:30:00Z") to ensure formatTime produces the precise expected output.apps/mobile/src/lib/ical-parser.ts (1)
53-63:decodeHtmlEntitieshandles only a small subset of HTML entities.Numeric character references (e.g.,
<,<) and many named entities are not covered. If ICS sources produce varied HTML encoding, consider using a small library likeheorhtml-entitiesfor more robust decoding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/lib/ical-parser.ts` around lines 53 - 63, The current decodeHtmlEntities function only handles a few hard-coded entities and misses numeric and many named entities; replace its implementation in decodeHtmlEntities with a robust decoder from a small library (e.g., install and import he or html-entities) and call the library's decode/unescape function to fully decode named entities and numeric character references (hex and decimal) before normalizing whitespace; ensure the function still normalizes whitespace with .replace(/\s+/g, " ").trim() after decoding.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/mobile/package.json`:
- Line 27: package.json currently pins "expo" and "expo-router" to preview
releases ("expo": "55.0.0-preview.11", "expo-router": "55.0.0-preview.8");
update these entries to stable pins consistent with the other expo packages
(e.g., change to "~55.0.x" when Expo SDK 55 is officially released) or
temporarily revert to the latest stable SDK (match the other expo-family
versions) to avoid preview-instability; update the "expo" and "expo-router"
dependency values in package.json accordingly and run npm/yarn install to verify
compatibility.
In `@apps/mobile/src/app/convention/`[id]/event/[eventId]/index.tsx:
- Around line 212-221: The "View on Sched" label incorrectly assumes
event.sourceUrl is always from Sched; update the Pressable/Text that uses
event.sourceUrl (the Pressable with onPress={() =>
Linking.openURL(event.sourceUrl!)} and its child Text) to display a generic
label like "View Source" or derive the label conditionally from the URL (e.g.,
check event.sourceUrl hostname and show "View on Sched" only if the domain
matches sched.*), keeping the existing onPress and ExternalLink icon behavior.
- Around line 51-64: The reminder Alert in handleReminderPress currently builds
buttons only from REMINDER_OPTIONS and lacks an explicit Cancel button; update
the Alert.alert call to append a Cancel button (e.g., { text: "Cancel", style:
"cancel", onPress: () => {} }) to the array of button options so users can
dismiss without choosing a reminder, keeping the existing options that call
setReminder.mutate({ id: eventId, conventionId: id, reminderMinutes: opt.value
}).
- Around line 40-45: The Alert onPress callback calls deleteEvent.mutateAsync
and then router.back but doesn't handle rejections; wrap the await
deleteEvent.mutateAsync({ id: eventId, conventionId: id }) call in a try/catch
inside the onPress async function (i.e., the same onPress handler) and on error
handle/report it (e.g., process/log or show a user-facing message) instead of
letting the promise reject unhandled; keep router.back() in the try block after
successful deletion so it only runs on success.
In `@apps/mobile/src/app/convention/import-preview.tsx`:
- Around line 58-64: handleImport currently awaits importEvents without error
handling; wrap the await call in a try/catch so thrown errors are caught, avoid
calling router.dismissAll()/router.push on failure, and present user feedback
(e.g., invoke the app's toast/error UI or set an error state) with a friendly
message and the error details; ensure you still log the error for debugging (use
console.error or the existing logger) and keep the existing success flow
(router.dismissAll and router.push(`/convention/${conventionId}`)) inside the
try block.
- Line 23: The component currently does JSON.parse(data) inside useMemo
(creating the calendar: ParsedCalendar) which will throw if data is undefined or
malformed; update the calendar creation (the useMemo block that defines
calendar: ParsedCalendar) to guard and safely parse: check that data is a
non-empty string before parsing and/or wrap JSON.parse in try/catch, returning a
sensible fallback (e.g., null, {} typed as ParsedCalendar, or an empty
ParsedCalendar) when parsing fails, and ensure the rest of the component handles
that fallback.
In `@apps/mobile/src/hooks/use-data-transfer.ts`:
- Around line 63-69: The onPress async callback can throw unhandled promise
rejections (from importData or queryClient.invalidateQueries); wrap the entire
body of the onPress handler in its own try/catch, call importData(data) and
queryClient.invalidateQueries inside the try, show the success Alert.alert with
the summary on success, and in the catch call Alert.alert("Import Failed",
error?.message ?? String(error)) and optionally log the error so failures are
surfaced to the user; update the onPress anonymous function where importData,
queryClient.invalidateQueries, and Alert.alert are used.
- Line 19: Guard against FileSystem.cacheDirectory being null before composing
fileUri in use-data-transfer.ts: check FileSystem.cacheDirectory (used to build
fileUri) and if it's null/undefined, either throw a clear error or substitute a
safe fallback (e.g., FileSystem.documentDirectory or a validated app-specific
directory) and then build fileUri using filename; ensure any downstream
writes/reads use this validated path and surface a descriptive error if no
suitable directory exists.
- Line 65: The importData flow only invalidates ["conventions"] but also inserts
events; update the importData completion logic in use-data-transfer.ts (where
queryClient.invalidateQueries is called) to also invalidate event-related caches
by calling queryClient.invalidateQueries({ queryKey: ["events"] }) and
queryClient.invalidateQueries({ queryKey: ["event"] }) (in addition to the
existing conventions invalidation) so event lists and individual event entries
are refetched and not stale.
In `@apps/mobile/src/lib/__tests__/date-utils.test.ts`:
- Around line 70-96: The test for groupEventsByDay is timezone-sensitive because
groupEventsByDay uses format(parseISO(...),"yyyy-MM-dd") which depends on the
local timezone; update the test to use timezone-robust timestamps (e.g., change
startTime/endTime to midday UTC like "2026-07-02T12:00:00Z" and
"2026-07-03T12:00:00Z") or alternatively mock the timezone/Date handling in this
spec so that groupEventsByDay consistently groups by the intended UTC dates;
modify the events array in date-utils.test.ts (the inputs used by
groupEventsByDay) or add a timezone mock setup/teardown around this test to
ensure stable grouping across environments.
In `@apps/mobile/src/lib/date-utils.ts`:
- Around line 45-56: getRelativeDate uses differenceInDays which measures
24-hour periods and can misclassify calendar-day boundaries; update
getRelativeDate to compute calendar-based differences by normalizing both parsed
date and now with startOfDay (or use differenceInCalendarDays) instead of
differenceInDays so comparisons like Today/Tomorrow/Yesterday align with
calendar days; modify the call that defines days (currently
differenceInDays(date, now)) to either
differenceInCalendarDays(parseISO(dateString), new Date()) or
differenceInDays(startOfDay(date), startOfDay(now)) and keep the existing
conditional logic and final format(date, "MMM d, yyyy").
In `@apps/mobile/src/lib/ical-parser.ts`:
- Around line 149-150: The fallback that sets startDate and endDate to new
Date().toISOString() when earliest/latest are null causes misleading "now"
values; update the logic in the code that computes startDate/endDate
(referencing earliest, latest, startDate, endDate) to instead surface a clear
sentinel or error: either throw an explicit error when no valid events are
found, or return null/undefined (or a named sentinel) for startDate/endDate so
consumers can detect "no events" rather than treating the convention as
happening today; ensure the change is applied where startDate/endDate are
constructed so callers can handle the empty-calendar case properly.
- Around line 90-92: parseICalContent currently calls ICAL.parse(icsContent)
without protection, so malformed ICS will throw and crash; wrap the ICAL.parse
and subsequent new ICAL.Component(jcal) calls in a try/catch inside
parseICalContent, catch parsing errors, and rethrow or return a controlled error
result with a clear message (e.g., "Invalid ICS content" plus the original
error.message) so callers can handle user-supplied/remote failures gracefully;
reference the parseICalContent function and the ICAL.parse / ICAL.Component
calls when making the change.
---
Duplicate comments:
In `@apps/mobile/src/global.css`:
- Around line 1-4: The global.css uses Tailwind v4-style imports (`@import`
"tailwindcss/theme.css" layer(theme); etc.) which require NativeWind v5; verify
and if missing update apps/mobile package.json to depend on NativeWind v5 (e.g.,
"nativewind": "^5.0.0-bridge" or the preview tag used in your repo) and ensure
tailwindcss is the v4-compatible release, then run package manager install and
regenerate the lockfile; alternatively, if you intentionally stayed on
NativeWind v4, change the imports in global.css to the v4/nativewind-compatible
syntax; check the package.json entries and the imports in global.css (the four
`@import` lines) and commit the dependency and lockfile changes.
---
Nitpick comments:
In `@apps/mobile/src/app/convention/import-preview.tsx`:
- Around line 105-157: The current mapping over calendar.events renders all
items eagerly (using calendar.events.map(...) and Pressable), which will jank
for large imports; replace this with a virtualized FlatList: use FlatList with
data={calendar.events}, keyExtractor={(item, index) => item.uid ||
String(index)}, and move the row JSX into a renderItem handler that calls
toggleEvent(itemIndex) and reads selectedIndices, CATEGORY_COLORS, event.title,
event.category, event.isAgeRestricted, and event.contentWarning so the UI and
styles (the checkbox View, category dot, Texts, and numberOfLines) are
preserved; ensure to pass extraData={selectedIndices} so selections update
correctly.
In `@apps/mobile/src/app/settings/about.tsx`:
- Line 1: Replace the raw React Native Text import with the project's custom
Text component from "@/components/ui/text" and update the import line in
about.tsx to import View from "react-native" and Text from
"@/components/ui/text"; then remove redundant class names like "text-base" and
"text-foreground" from the About screen's <Text> usages (the instances currently
showing those classes) so they rely on the default styling provided by the
custom Text wrapper.
- Line 11: The PawPrint icon in the About component uses a hardcoded color
"#ffffff" which won't adapt to theme or primary color changes; update the
component to derive the color from the app theme (for example use
useColorScheme() or your design-token/theme hook like useTheme/useColors) and
pass that value into the PawPrint color prop (or compute a dynamic variable such
as badgeColor) so the icon follows dark/light modes and centralized color tokens
instead of a literal string.
In `@apps/mobile/src/components/event-item.tsx`:
- Line 40: Extract the repeated hardcoded muted icon colors into a shared theme
constant (e.g., export const MUTED_ICON_COLOR = { dark: "#a8a29e", light:
"#78716c" } or add mutedIconColor to your existing theme object), then replace
the inline color props on Clock in event-item.tsx (and the corresponding usage
in the event detail screen) to use the theme constant based on isDark (e.g.,
color={theme.MUTED_ICON_COLOR[isDark ? 'dark' : 'light']}); update imports to
pull the constant from the shared theme module and ensure both components
reference the same identifier so the colors stay in sync.
In `@apps/mobile/src/global.css`:
- Around line 31-53: The dark-mode destructive color (--color-destructive) in
the .dark CSS block is darker (`#dc2626`) and may lack contrast against
--color-background (`#0a0a0a`); update the .dark rule to use a lighter/brighter
red (or the same light-mode value) that ensures sufficient contrast for
visibility and accessibility, e.g., choose a higher luminance hex for
--color-destructive or run a contrast check between --color-destructive and
--color-background and adjust until it meets AA/AAA; modify the .dark {
--color-destructive: ... } declaration accordingly while keeping the rest of the
variables intact.
In `@apps/mobile/src/lib/__tests__/date-utils.test.ts`:
- Around line 25-29: The test for formatTime is timezone-dependent; make it
deterministic by computing an expected value in a fixed timezone (or mocking the
environment) and asserting equality instead of a loose regex. Update the test in
date-utils.test.ts to call new
Date("2026-07-02T14:30:00Z").toLocaleTimeString('en-US', { hour: 'numeric',
minute: '2-digit', hour12: true, timeZone: 'UTC' }) (or mock Date/timezone) and
compare that exact string to formatTime("2026-07-02T14:30:00Z") to ensure
formatTime produces the precise expected output.
In `@apps/mobile/src/lib/ical-parser.ts`:
- Around line 53-63: The current decodeHtmlEntities function only handles a few
hard-coded entities and misses numeric and many named entities; replace its
implementation in decodeHtmlEntities with a robust decoder from a small library
(e.g., install and import he or html-entities) and call the library's
decode/unescape function to fully decode named entities and numeric character
references (hex and decimal) before normalizing whitespace; ensure the function
still normalizes whitespace with .replace(/\s+/g, " ").trim() after decoding.
| "clsx": "^2.1.1", | ||
| "date-fns": "^4.1.0", | ||
| "drizzle-orm": "^0.39.3", | ||
| "expo": "55.0.0-preview.11", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd apps/mobile && cat -n package.json | sed -n '20,50p'Repository: MrDemonWolf/conpaws
Length of output: 1350
🌐 Web query:
Expo SDK 55 release status official
💡 Result:
As of February 17, 2026, Expo has officially announced SDK 55 only as a Beta (announcement dated January 22, 2026). The post says the beta period begins Jan 22 and is expected to last ~2 weeks, with full release notes coming at the stable release. [1]
On Expo’s official changelog feed, the most recent SDK entries show “Expo SDK 55 Beta is now available” (Jan 22, 2026) and the prior stable SDK 54 release (Sep 10, 2025), but no “Expo SDK 55” stable-release announcement appears there yet. [2]
Official status: SDK 55 = Beta (not yet announced as stable on Expo’s official changelog). [1][2]
Sources:
[1] Expo Changelog — “Expo SDK 55 Beta is now available” (Jan 22, 2026) (expo.dev)
[2] Expo “Changelog” feed (shows SDK 55 Beta entry; no SDK 55 stable post listed) (expo.dev)
Citations:
- 1: https://expo.dev/changelog/sdk-55-beta?utm_source=openai
- 2: https://expo.dev/new-changelog?utm_source=openai
🌐 Web query:
expo-router 55.0.0 preview release date 2026
💡 Result:
Expo Router 55.0.0 “preview” builds became publicly available with the Expo SDK 55 beta announcement on January 22, 2026 (that’s when the SDK 55 beta period started). [1]
A concrete dated reference to a specific preview build is the Expo security advisory update published January 26, 2026, which explicitly mentions expo-router@55.0.0-preview.5. [2]
Sources: [1] [2]
Two core dependencies are on preview releases.
expo@55.0.0-preview.11 and expo-router@55.0.0-preview.8 are preview versions, while most other expo-family packages are correctly pinned to stable ~55.0.x versions. As of February 2026, Expo SDK 55 is in beta (announced January 22, 2026). While versions are properly pinned, consider upgrading to stable releases once Expo SDK 55 is officially released. The preview builds carry higher risk of breaking changes and incompatibilities with tooling and other packages.
Also applies to: 36-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/mobile/package.json` at line 27, package.json currently pins "expo" and
"expo-router" to preview releases ("expo": "55.0.0-preview.11", "expo-router":
"55.0.0-preview.8"); update these entries to stable pins consistent with the
other expo packages (e.g., change to "~55.0.x" when Expo SDK 55 is officially
released) or temporarily revert to the latest stable SDK (match the other
expo-family versions) to avoid preview-instability; update the "expo" and
"expo-router" dependency values in package.json accordingly and run npm/yarn
install to verify compatibility.
Comment on lines
+63
to
+69
| onPress: async () => { | ||
| const summary = await importData(data); | ||
| queryClient.invalidateQueries({ queryKey: ["conventions"] }); | ||
| Alert.alert( | ||
| "Import Complete", | ||
| `Added ${summary.conventionsAdded} conventions and ${summary.eventsAdded} events. Skipped ${summary.conventionsSkipped} duplicate conventions and ${summary.eventsSkipped} duplicate events.`, | ||
| ); |
There was a problem hiding this comment.
Unhandled errors in the onPress async callback.
The outer try/catch (line 39–76) has already completed by the time the user taps "Import". If importData(data) or invalidateQueries throws inside this callback, it becomes an unhandled promise rejection — the "Import Failed" alert will never show.
Proposed fix — wrap the callback body
onPress: async () => {
+ try {
const summary = await importData(data);
queryClient.invalidateQueries({ queryKey: ["conventions"] });
Alert.alert(
"Import Complete",
`Added ${summary.conventionsAdded} conventions and ${summary.eventsAdded} events. Skipped ${summary.conventionsSkipped} duplicate conventions and ${summary.eventsSkipped} duplicate events.`,
);
+ } catch {
+ Alert.alert("Import Failed", "Could not import data. Please try again.");
+ }
},📝 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.
Suggested change
| onPress: async () => { | |
| const summary = await importData(data); | |
| queryClient.invalidateQueries({ queryKey: ["conventions"] }); | |
| Alert.alert( | |
| "Import Complete", | |
| `Added ${summary.conventionsAdded} conventions and ${summary.eventsAdded} events. Skipped ${summary.conventionsSkipped} duplicate conventions and ${summary.eventsSkipped} duplicate events.`, | |
| ); | |
| onPress: async () => { | |
| try { | |
| const summary = await importData(data); | |
| queryClient.invalidateQueries({ queryKey: ["conventions"] }); | |
| Alert.alert( | |
| "Import Complete", | |
| `Added ${summary.conventionsAdded} conventions and ${summary.eventsAdded} events. Skipped ${summary.conventionsSkipped} duplicate conventions and ${summary.eventsSkipped} duplicate events.`, | |
| ); | |
| } catch { | |
| Alert.alert("Import Failed", "Could not import data. Please try again."); | |
| } | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/mobile/src/hooks/use-data-transfer.ts` around lines 63 - 69, The onPress
async callback can throw unhandled promise rejections (from importData or
queryClient.invalidateQueries); wrap the entire body of the onPress handler in
its own try/catch, call importData(data) and queryClient.invalidateQueries
inside the try, show the success Alert.alert with the summary on success, and in
the catch call Alert.alert("Import Failed", error?.message ?? String(error)) and
optionally log the error so failures are surfaced to the user; update the
onPress anonymous function where importData, queryClient.invalidateQueries, and
Alert.alert are used.
Comment on lines
+45
to
+56
| export function getRelativeDate(dateString: string): string { | ||
| const date = parseISO(dateString); | ||
| const now = new Date(); | ||
| const days = differenceInDays(date, now); | ||
|
|
||
| if (days === 0) return "Today"; | ||
| if (days === 1) return "Tomorrow"; | ||
| if (days === -1) return "Yesterday"; | ||
| if (days > 0 && days <= 7) return `In ${days} days`; | ||
| if (days < 0 && days >= -7) return `${Math.abs(days)} days ago`; | ||
| return format(date, "MMM d, yyyy"); | ||
| } |
There was a problem hiding this comment.
differenceInDays counts full 24-hour periods, not calendar days.
differenceInDays(date, now) measures elapsed 24-hour blocks. An event at 1:00 AM tomorrow compared against 11:00 PM today yields 0 (→ "Today" instead of "Tomorrow"). If calendar-day semantics are desired, compare using startOfDay:
Suggested approach
+import { startOfDay } from "date-fns";
+
export function getRelativeDate(dateString: string): string {
const date = parseISO(dateString);
const now = new Date();
- const days = differenceInDays(date, now);
+ const days = differenceInDays(startOfDay(date), startOfDay(now));📝 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.
Suggested change
| export function getRelativeDate(dateString: string): string { | |
| const date = parseISO(dateString); | |
| const now = new Date(); | |
| const days = differenceInDays(date, now); | |
| if (days === 0) return "Today"; | |
| if (days === 1) return "Tomorrow"; | |
| if (days === -1) return "Yesterday"; | |
| if (days > 0 && days <= 7) return `In ${days} days`; | |
| if (days < 0 && days >= -7) return `${Math.abs(days)} days ago`; | |
| return format(date, "MMM d, yyyy"); | |
| } | |
| import { parseISO, differenceInDays, format, startOfDay } from "date-fns"; | |
| export function getRelativeDate(dateString: string): string { | |
| const date = parseISO(dateString); | |
| const now = new Date(); | |
| const days = differenceInDays(startOfDay(date), startOfDay(now)); | |
| if (days === 0) return "Today"; | |
| if (days === 1) return "Tomorrow"; | |
| if (days === -1) return "Yesterday"; | |
| if (days > 0 && days <= 7) return `In ${days} days`; | |
| if (days < 0 && days >= -7) return `${Math.abs(days)} days ago`; | |
| return format(date, "MMM d, yyyy"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/mobile/src/lib/date-utils.ts` around lines 45 - 56, getRelativeDate uses
differenceInDays which measures 24-hour periods and can misclassify calendar-day
boundaries; update getRelativeDate to compute calendar-based differences by
normalizing both parsed date and now with startOfDay (or use
differenceInCalendarDays) instead of differenceInDays so comparisons like
Today/Tomorrow/Yesterday align with calendar days; modify the call that defines
days (currently differenceInDays(date, now)) to either
differenceInCalendarDays(parseISO(dateString), new Date()) or
differenceInDays(startOfDay(date), startOfDay(now)) and keep the existing
conditional logic and final format(date, "MMM d, yyyy").
Update architecture section with SDK 55, React Native 0.83, React 19.2, NativeWind v5, react-native-css, react-native-worklets, and new arch defaults. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wrap deleteEvent.mutateAsync in try/catch to handle rejections and only navigate back on success. Add Cancel button to reminder Alert. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Show "View Source" for non-Sched URLs in event detail - Add safe JSON parsing with fallback UI in import preview - Invalidate events/event query caches after data import - Use midday UTC timestamps in date-utils tests for timezone safety - Wrap ICAL.parse in try/catch with descriptive error message - Return null for startDate/endDate when calendar has no events - Guard against null dates in import flow and preview display Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move useState and useMemo above the early return guard so hooks are called in the same order on every render. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use non-null assertions on startDate/endDate since the preceding assertions already verify they are truthy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
apps/mobile/src/lib/__tests__/ical-parser.test.ts (3)
15-130: No negative / edge-case tests for the parser.The suite only exercises well-formed ICS fixtures. Consider adding at least one test for malformed input (verifying the
"Invalid ICS content"error path) and one for an ICS with noVEVENTcomponents (empty events array, null dates).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/lib/__tests__/ical-parser.test.ts` around lines 15 - 130, Add two edge-case tests to the ical-parser.test.ts suite: one that calls parseICalContent with malformed ICS input (e.g., a short string like "INVALID") and asserts it throws the "Invalid ICS content" error, and another that supplies a valid ICS string containing no VEVENT components and asserts parsed.events is an empty array and parsed.startDate/parsed.endDate are null/undefined; reference the parseICalContent function and add descriptive it(...) blocks (e.g., "throws on malformed ICS" and "handles ICS with no VEVENTs") near the existing describe blocks so they run with the existing fixtures.
15-18: Parsing atdescribescope runs eagerly and gives poor diagnostics on failure.
readFileSync+parseICalContentexecute during module evaluation. If either fixture is missing or malformed, every test in the suite fails with a confusing stack trace rather than a clear per-test failure. Wrapping inbeforeAllwith a variable assignment would isolate setup failures.Suggested approach
describe("small-test.ics", () => { - const icsContent = readFileSync(SMALL_TEST_PATH, "utf-8"); - const parsed = parseICalContent(icsContent); + let parsed: ReturnType<typeof parseICalContent>; + + beforeAll(() => { + const icsContent = readFileSync(SMALL_TEST_PATH, "utf-8"); + parsed = parseICalContent(icsContent); + });Same for
indyfurcon2025.ics.Also applies to: 97-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/lib/__tests__/ical-parser.test.ts` around lines 15 - 18, The test eagerly reads and parses fixtures at describe scope (readFileSync and parseICalContent using SMALL_TEST_PATH and the indyfurcon2025 fixture), causing module-eval failures; move the synchronous IO and parsing into beforeAll hooks and store results in outer-scope let variables (e.g., let icsContent, let parsed) so readFileSync(SMALL_TEST_PATH) and parseICalContent(...) run during beforeAll for both "small-test.ics" and the indyfurcon2025 suite, ensuring setup errors are reported per-test rather than at module load.
6-13: Fragile relative paths for test fixtures.The five-level
../../../../../test-data/traversal is brittle and will break if the test file or fixture directory is moved. Consider using a project-root-relative resolution (e.g., via avitestconfigrootor a shared helper) to make this more robust.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/lib/__tests__/ical-parser.test.ts` around lines 6 - 13, SMALL_TEST_PATH and INDY_PATH use brittle five-level relative paths to test fixtures; change them to resolve fixtures from a stable project root instead (e.g., use Vitest's configured root or a shared test helper that exposes a projectRoot or fixturesDir) so the paths no longer depend on this file's location—replace resolve(__dirname, "../../../../../test-data/...") with resolve(projectRootOrFixturesDir, "test-data", "small-test.ics") (or call the shared helper) in the test to make fixture resolution robust.apps/mobile/src/app/convention/import-preview.tsx (1)
1-8: MixedTextimports — using RNTextinstead of customTextcomponent.Line 4 imports
Textfromreact-native, but the project has a themedTextcomponent at@/components/ui/text.tsxthat applies default foreground color styling. The customTextis used elsewhere in the project. Here, every<Text>manually appliestext-foregroundortext-muted-foregroundclasses, which the custom component would handle by default.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/app/convention/import-preview.tsx` around lines 1 - 8, The file currently imports Text from react-native and manually applies text-foreground/text-muted-foreground classes; replace the RN Text import with the project's themed Text component import (import Text from '@/components/ui/text') and update usages of the Text symbol to rely on the custom component's default foreground styling (remove redundant manual text-foreground classes; keep overrides only if a different tone is needed, e.g., text-muted-foreground as a className prop). Ensure you only change the import and class usage for the Text component (leave ScrollView, Pressable, ActivityIndicator imports unchanged).apps/mobile/src/lib/ical-parser.ts (1)
53-63:decodeHtmlEntitiescollapses all whitespace, including intentional newlines.Line 61's
/\s+/greplacement flattens multi-line descriptions (paragraph breaks, lists) into a single line. If event descriptions contain meaningful formatting, this will degrade readability. Consider only collapsing horizontal whitespace (/[^\S\r\n]+/g) or applying the collapse selectively.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/mobile/src/lib/ical-parser.ts` around lines 53 - 63, The decodeHtmlEntities function currently collapses all whitespace via .replace(/\s+/g, " "), which removes intentional newlines and breaks; change that replacement to only collapse horizontal whitespace so newlines are preserved (e.g., use a pattern like /[^\S\r\n]+/g or otherwise restrict to spaces/tabs), keeping the rest of the entity replacements intact; update the replacement in decodeHtmlEntities to use the horizontal-whitespace-only regex to preserve paragraph breaks and lists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/mobile/src/lib/ical-parser.ts`:
- Around line 90-99: In parseICalContent, the construction new
ICAL.Component(jcal) can throw if jcal is malformed; extend the existing
try/catch to include the ICAL.Component creation (i.e., move or add the
Component instantiation inside the try block that calls ICAL.parse) so any
errors from ICAL.parse or new ICAL.Component(jcal) are caught, and rethrow a
descriptive Error preserving the original err.message (use the same error
formatting as the current catch).
- Around line 120-121: The code uses vevent.getFirstPropertyValue("categories")
which only returns the first raw string and loses comma-separated values and
additional CATEGORIES properties; replace this with collecting all category
property values (e.g., vevent.getAllPropertyValues("categories") or
vevent.getAllProperties("categories") then map to values), split each value on
commas, trim and flatten into a single array of category strings (filtering out
empties), assign that array to a new variable (e.g., categories) in place of the
single `category` value, and update any downstream uses of `category` to consume
the array of categories.
In `@CLAUDE.md`:
- Around line 47-64: The markdown fenced directory-tree blocks (the
triple-backtick blocks containing the conpaws/ tree around the snippets shown)
lack a language tag and trigger MD040; update those fenced code blocks by
changing the opening fence from ``` to ```text or ```plaintext for both
occurrences (the conpaws/ directory tree block and the second directory-tree
block later in the file) so markdownlint no longer flags them.
---
Duplicate comments:
In `@apps/mobile/src/app/convention/import-preview.tsx`:
- Around line 82-88: Destructure the error state from useICalImport (alongside
isLoading) in import-preview.tsx and render it so users get feedback when
importEvents fails: update the hook call to pull error and, in the footer/near
the import button (e.g., around the existing handleImport / router logic and
footer area), conditionally render a Text/Alert showing the error message (use
the same styling approach suggested: a small, destructive, centered message) so
when importEvents returns null the error state is visible to the user.
In `@apps/mobile/src/hooks/use-data-transfer.ts`:
- Line 19: Guard use of FileSystem.cacheDirectory before building fileUri in
use-data-transfer.ts: check that FileSystem.cacheDirectory is not null (e.g.,
inside the function where fileUri is constructed or in useDataTransfer) and
return or throw a clear error / fall back to a safe directory if it is null;
only construct fileUri = `${FileSystem.cacheDirectory}${filename}` when
cacheDirectory is a non-null string to avoid producing "null..." paths and
failing writes.
- Around line 63-71: The onPress async callback currently awaits importData and
invalidates queries but is not wrapped in its own try/catch, so rejections from
importData or queryClient.invalidateQueries become unhandled; update the onPress
handler in use-data-transfer.ts to wrap its entire body in a try { const summary
= await importData(data); await queryClient.invalidateQueries({ queryKey:
["conventions"] }); await queryClient.invalidateQueries({ queryKey: ["events"]
}); await queryClient.invalidateQueries({ queryKey: ["event"] });
Alert.alert("Import Complete", /* message using summary */); } catch (err) {
processLogger?.error?.(err) or console.error(err); Alert.alert("Import Failed",
String(err || "Unknown error")); } to ensure errors from importData and
invalidateQueries are caught and the "Import Failed" alert is shown.
---
Nitpick comments:
In `@apps/mobile/src/app/convention/import-preview.tsx`:
- Around line 1-8: The file currently imports Text from react-native and
manually applies text-foreground/text-muted-foreground classes; replace the RN
Text import with the project's themed Text component import (import Text from
'@/components/ui/text') and update usages of the Text symbol to rely on the
custom component's default foreground styling (remove redundant manual
text-foreground classes; keep overrides only if a different tone is needed,
e.g., text-muted-foreground as a className prop). Ensure you only change the
import and class usage for the Text component (leave ScrollView, Pressable,
ActivityIndicator imports unchanged).
In `@apps/mobile/src/lib/__tests__/ical-parser.test.ts`:
- Around line 15-130: Add two edge-case tests to the ical-parser.test.ts suite:
one that calls parseICalContent with malformed ICS input (e.g., a short string
like "INVALID") and asserts it throws the "Invalid ICS content" error, and
another that supplies a valid ICS string containing no VEVENT components and
asserts parsed.events is an empty array and parsed.startDate/parsed.endDate are
null/undefined; reference the parseICalContent function and add descriptive
it(...) blocks (e.g., "throws on malformed ICS" and "handles ICS with no
VEVENTs") near the existing describe blocks so they run with the existing
fixtures.
- Around line 15-18: The test eagerly reads and parses fixtures at describe
scope (readFileSync and parseICalContent using SMALL_TEST_PATH and the
indyfurcon2025 fixture), causing module-eval failures; move the synchronous IO
and parsing into beforeAll hooks and store results in outer-scope let variables
(e.g., let icsContent, let parsed) so readFileSync(SMALL_TEST_PATH) and
parseICalContent(...) run during beforeAll for both "small-test.ics" and the
indyfurcon2025 suite, ensuring setup errors are reported per-test rather than at
module load.
- Around line 6-13: SMALL_TEST_PATH and INDY_PATH use brittle five-level
relative paths to test fixtures; change them to resolve fixtures from a stable
project root instead (e.g., use Vitest's configured root or a shared test helper
that exposes a projectRoot or fixturesDir) so the paths no longer depend on this
file's location—replace resolve(__dirname, "../../../../../test-data/...") with
resolve(projectRootOrFixturesDir, "test-data", "small-test.ics") (or call the
shared helper) in the test to make fixture resolution robust.
In `@apps/mobile/src/lib/ical-parser.ts`:
- Around line 53-63: The decodeHtmlEntities function currently collapses all
whitespace via .replace(/\s+/g, " "), which removes intentional newlines and
breaks; change that replacement to only collapse horizontal whitespace so
newlines are preserved (e.g., use a pattern like /[^\S\r\n]+/g or otherwise
restrict to spaces/tabs), keeping the rest of the entity replacements intact;
update the replacement in decodeHtmlEntities to use the
horizontal-whitespace-only regex to preserve paragraph breaks and lists.
Comment on lines
+90
to
+99
| export function parseICalContent(icsContent: string): ParsedCalendar { | ||
| let jcal; | ||
| try { | ||
| jcal = ICAL.parse(icsContent); | ||
| } catch (err) { | ||
| throw new Error( | ||
| `Invalid ICS content: ${err instanceof Error ? err.message : "parse failed"}`, | ||
| ); | ||
| } | ||
| const comp = new ICAL.Component(jcal); |
There was a problem hiding this comment.
new ICAL.Component(jcal) on Line 99 is outside the try/catch.
If ICAL.parse returns a structurally unexpected result, the Component constructor could also throw. Consider extending the existing try/catch to cover this line as well.
Suggested fix
let jcal;
+ let comp;
try {
jcal = ICAL.parse(icsContent);
+ comp = new ICAL.Component(jcal);
} catch (err) {
throw new Error(
`Invalid ICS content: ${err instanceof Error ? err.message : "parse failed"}`,
);
}
- const comp = new ICAL.Component(jcal);📝 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.
Suggested change
| export function parseICalContent(icsContent: string): ParsedCalendar { | |
| let jcal; | |
| try { | |
| jcal = ICAL.parse(icsContent); | |
| } catch (err) { | |
| throw new Error( | |
| `Invalid ICS content: ${err instanceof Error ? err.message : "parse failed"}`, | |
| ); | |
| } | |
| const comp = new ICAL.Component(jcal); | |
| export function parseICalContent(icsContent: string): ParsedCalendar { | |
| let jcal; | |
| let comp; | |
| try { | |
| jcal = ICAL.parse(icsContent); | |
| comp = new ICAL.Component(jcal); | |
| } catch (err) { | |
| throw new Error( | |
| `Invalid ICS content: ${err instanceof Error ? err.message : "parse failed"}`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/mobile/src/lib/ical-parser.ts` around lines 90 - 99, In
parseICalContent, the construction new ICAL.Component(jcal) can throw if jcal is
malformed; extend the existing try/catch to include the ICAL.Component creation
(i.e., move or add the Component instantiation inside the try block that calls
ICAL.parse) so any errors from ICAL.parse or new ICAL.Component(jcal) are
caught, and rethrow a descriptive Error preserving the original err.message (use
the same error formatting as the current catch).
Comment on lines
+120
to
+121
| const category = | ||
| (vevent.getFirstPropertyValue("categories") as string | null) ?? null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file in question and examine its content around lines 120-121
find . -type f -name "ical-parser.ts" -path "*/mobile/*"Repository: MrDemonWolf/conpaws
Length of output: 99
🏁 Script executed:
# Check for test fixture files with multiple CATEGORIES values
fd --type f '\.ics' --exec grep -i 'CATEGORIES' {}Repository: MrDemonWolf/conpaws
Length of output: 686
🏁 Script executed:
# Let's also understand the structure of the codebase
git ls-files "apps/mobile/src/lib/" | head -20Repository: MrDemonWolf/conpaws
Length of output: 633
🏁 Script executed:
# View the ical-parser.ts file around lines 120-121
cat -n apps/mobile/src/lib/ical-parser.ts | sed -n '110,130p'Repository: MrDemonWolf/conpaws
Length of output: 1044
🏁 Script executed:
# Search for getFirstPropertyValue definition and its behavior
rg "getFirstPropertyValue" apps/mobile/src/lib/ical-parser.ts -A 5 -B 5Repository: MrDemonWolf/conpaws
Length of output: 1063
🏁 Script executed:
# Check for any .ics files with comma-separated CATEGORIES on a single line
fd --type f '\.ics' --exec grep -E 'CATEGORIES:[^:]+,[^:]' {} +Repository: MrDemonWolf/conpaws
Length of output: 132
🏁 Script executed:
# Properly search for comma-separated CATEGORIES in .ics files
find . -type f -name "*.ics" | while read file; do
grep -H 'CATEGORIES:[^:]*,[^:]' "$file"
doneRepository: MrDemonWolf/conpaws
Length of output: 45
🏁 Script executed:
# Look at the test file for the parser
cat -n apps/mobile/src/lib/__tests__/ical-parser.test.ts | head -100Repository: MrDemonWolf/conpaws
Length of output: 3883
🏁 Script executed:
# Search for any test fixtures directory
find . -type d -name "*fixture*" -o -name "*test*data*" 2>/dev/null | grep -i icalRepository: MrDemonWolf/conpaws
Length of output: 45
🏁 Script executed:
# Find all .ics files to understand the test data
find . -type f -name "*.ics" -exec echo "File: {}" \; -exec head -20 {} \;Repository: MrDemonWolf/conpaws
Length of output: 1274
🏁 Script executed:
# Check complete content of test fixtures for any comma-separated CATEGORIES
cat test-data/small-test.ics | grep -i categoriesRepository: MrDemonWolf/conpaws
Length of output: 305
🏁 Script executed:
# Also check the indyfurcon file more thoroughly
cat test-data/indyfurcon2025.ics | grep -i categoriesRepository: MrDemonWolf/conpaws
Length of output: 443
🏁 Script executed:
# Search for any reference to ical.js library and how it handles CATEGORIES
rg "CATEGORIES" apps/mobile/src/lib/ -A 2 -B 2Repository: MrDemonWolf/conpaws
Length of output: 45
🏁 Script executed:
# Look for package.json to understand the ical.js version
cat apps/mobile/package.json | grep -i "ical" -A 2 -B 2Repository: MrDemonWolf/conpaws
Length of output: 222
🏁 Script executed:
# Let me check the complete structure of test fixtures to verify if any event has multiple CATEGORIES
# Get the full small-test.ics to see if any VEVENT has multiple CATEGORIES
awk '/BEGIN:VEVENT/,/END:VEVENT/ {print}' test-data/small-test.ics | grep -c "CATEGORIES"Repository: MrDemonWolf/conpaws
Length of output: 65
🏁 Script executed:
# More detailed check - show each event with its categories
awk '/BEGIN:VEVENT/{event=1; count=0} /CATEGORIES/{count++; categories[NR]=$0} /END:VEVENT/{if(count>1) print "Event at line " NR " has " count " CATEGORIES"; event=0}' test-data/small-test.icsRepository: MrDemonWolf/conpaws
Length of output: 45
🏁 Script executed:
# Check if ical.js has documentation on how it handles CATEGORIES
# Search for any comments or usage patterns in the codebase
rg "getPropertyValue|getAllPropertyValue" apps/mobile/src/lib/ -B 3 -A 3Repository: MrDemonWolf/conpaws
Length of output: 45
🏁 Script executed:
# Search for any usage of CATEGORIES in the entire codebase
rg "category|CATEGORIES" apps/mobile/src --type ts --type tsx -B 2 -A 2Repository: MrDemonWolf/conpaws
Length of output: 90
🏁 Script executed:
# Look at how the category is actually used in the parsed events
grep -n "category" apps/mobile/src/lib/ical-parser.tsRepository: MrDemonWolf/conpaws
Length of output: 167
🏁 Script executed:
# Check the type definition for ParsedEvent to understand what category is expected
rg "interface ParsedEvent|type ParsedEvent" apps/mobile/src/lib/ -A 20Repository: MrDemonWolf/conpaws
Length of output: 1226
🏁 Script executed:
# Let me examine the actual test fixture content more carefully to see event structure
head -100 test-data/small-test.icsRepository: MrDemonWolf/conpaws
Length of output: 3295
🏁 Script executed:
# One final check - let me search for any example of comma-separated CATEGORIES or multiple CATEGORIES in any ics file in the entire repo
find . -type f -name "*.ics" -exec sh -c 'echo "=== {} ===" && grep -c "CATEGORIES" "$1"' _ {} \;Repository: MrDemonWolf/conpaws
Length of output: 142
🏁 Script executed:
# Also check if there are any real-world calendar sources referenced
rg "indyfurcon|sched.com" apps/mobile/src -lRepository: MrDemonWolf/conpaws
Length of output: 267
Handle comma-separated CATEGORIES values per iCal specification.
The iCal CATEGORIES property (RFC 5545) can contain comma-separated values on a single property line (e.g., CATEGORIES:GAMING,SOCIAL), or multiple separate CATEGORIES properties per event. The current code using getFirstPropertyValue("categories") captures only the first property instance as a raw string, silently dropping any comma-separated values or additional property instances. While current test fixtures use only single-valued properties, properly-formatted iCal imports with multi-category events would lose category information.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/mobile/src/lib/ical-parser.ts` around lines 120 - 121, The code uses
vevent.getFirstPropertyValue("categories") which only returns the first raw
string and loses comma-separated values and additional CATEGORIES properties;
replace this with collecting all category property values (e.g.,
vevent.getAllPropertyValues("categories") or
vevent.getAllProperties("categories") then map to values), split each value on
commas, trim and flatten into a single array of category strings (filtering out
empties), assign that array to a new variable (e.g., categories) in place of the
single `category` value, and update any downstream uses of `category` to consume
the array of categories.
Comment on lines
+47
to
+64
| ``` | ||
| conpaws/ | ||
| ├── apps/mobile/ # Expo React Native app (MVP focus) | ||
| │ └── src/ | ||
| │ ├── app/ # Expo Router (file-based screens) | ||
| │ ├── components/ # UI components | ||
| │ ├── contexts/ # React contexts | ||
| │ ├── hooks/ # Custom hooks | ||
| │ ├── lib/ # Utilities, constants, i18n, parsers | ||
| │ ├── db/ # Drizzle schema + repositories | ||
| │ ├── types/ # TypeScript types | ||
| │ ├── locales/ # i18n translation files | ||
| │ ├── assets/ # Images, icons | ||
| │ └── global.css # Tailwind CSS entry point + design tokens | ||
| ├── apps/web/ # Next.js site (placeholder until Phase 4) | ||
| ├── packages/supabase/ # Supabase config + migrations (Phase 2) | ||
| └── pnpm-workspace.yaml | ||
| ``` |
There was a problem hiding this comment.
Fenced code blocks missing language specifier.
The directory-tree blocks on Lines 47 and 110 lack a language tag, flagged by markdownlint (MD040). Use ```text or ```plaintext to silence the lint warning.
Also applies to: 110-141
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 47-47: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 47 - 64, The markdown fenced directory-tree blocks
(the triple-backtick blocks containing the conpaws/ tree around the snippets
shown) lack a language tag and trigger MD040; update those fenced code blocks by
changing the opening fence from ``` to ```text or ```plaintext for both
occurrences (the conpaws/ directory tree block and the second directory-tree
block later in the file) so markdownlint no longer flags them.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apps/mobile,apps/web,packages/supabase)Test plan
pnpm installand verify monorepo workspace resolutionpnpm lintandpnpm type-checkfrom rootpnpm testto verify unit tests passpnpm startin apps/mobile and test onboarding flow🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Configuration & Infrastructure