Changes :#40
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR introduces a full connectivity and error-handling layer: a ChangesConnectivity, Error Handling, and Sync System
Account Deletion Hardening and Backend Fixes
Sequence Diagram(s)sequenceDiagram
participant NetInfo
participant ConnectivityProvider
participant useConnectivity
participant AiChatScreen
participant requestSync
participant useSyncStore
NetInfo->>ConnectivityProvider: onNetInfoChange(state)
ConnectivityProvider->>ConnectivityProvider: debounce offline confirmation (offlineConfirmationMs)
ConnectivityProvider-->>useConnectivity: ConnectivityState {status}
AiChatScreen->>useConnectivity: connectivity.status
AiChatScreen-->>AiChatScreen: isOffline → block send, show OfflineNotice
Note over AiChatScreen,requestSync: When device comes back online
ConnectivityProvider-->>AutoSyncManager: connectivity.status = "online"
AutoSyncManager->>requestSync: requestSync({reason:"reconnect", userId, ...})
requestSync->>useSyncStore: setSyncAttempt(timestamp, userId)
requestSync->>requestSync: profile → journal → achievements
requestSync->>useSyncStore: setSyncSuccess or setSyncFailure(errorCode)
useSyncStore-->>SyncStatusRow: SyncStatusSnapshot updated
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
supabase/functions/delete-account/index.ts (1)
335-388: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffConsider batching large file deletions for resilience.
The function accumulates all files across all pages into
filesToRemovebefore callingremove()once. For users with very large storage (thousands of files), this could cause issues with payload size limits or request timeouts on the bulk delete call.Consider batching deletions in chunks (e.g., 1000 files per call) to improve reliability for edge cases:
♻️ Suggested batching approach
+const REMOVE_BATCH_SIZE = 1000; + async function deleteStoragePrefix( client: ReturnType<typeof createClient>, bucket: string, userId: string, path: string, ): Promise<{ ok: true } | { ok: false }> { const prefix = path ? `${userId}/${path}` : userId; const filesToRemove: string[] = []; const nestedPaths: string[] = []; let offset = 0; while (true) { // ... pagination logic unchanged ... for (const item of items) { const itemPath = path ? `${path}/${item.name}` : item.name; if (item.id === null) { nestedPaths.push(itemPath); continue; } filesToRemove.push(`${userId}/${itemPath}`); } + // Batch delete when threshold reached + if (filesToRemove.length >= REMOVE_BATCH_SIZE) { + const removeResult = await client.storage.from(bucket).remove(filesToRemove); + if (removeResult.error) { + return { ok: false }; + } + filesToRemove.length = 0; + } if (items.length < storageListPageLimit) { break; } offset += storageListPageLimit; } // ... rest unchanged, handles remaining files ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/functions/delete-account/index.ts` around lines 335 - 388, The current implementation accumulates all files in the `filesToRemove` array before calling `client.storage.from(bucket).remove()` once with the entire batch, which can cause payload size or timeout issues for large deletions. Instead of the single `removeResult` call at the end, implement a batching approach that processes `filesToRemove` in chunks (e.g., 1000 files per batch). Loop through the accumulated files, slicing them into smaller batches and calling the remove operation for each batch separately, ensuring all batches complete successfully before returning the final result.hooks/useAutoSync.ts (1)
124-126:⚠️ Potential issue | 🟡 MinorRemove unused
startOfLocalDayhelper function from hooks/useAutoSync.ts.The function at line 124 is not called anywhere within this file. Since the PR removed its consumers (
getReflectionStreakandgetLocalDateKey), this helper is now dead code and should be deleted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hooks/useAutoSync.ts` around lines 124 - 126, The startOfLocalDay helper function is no longer being used in the file since its consumers (getReflectionStreak and getLocalDateKey) were removed in this PR. Delete the entire startOfLocalDay function definition to remove the dead code and keep the file clean.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/ai-chat/ai-chat-screen.tsx`:
- Around line 351-363: Extract the hardcoded hex color values used for the
online/offline connection state indicator in the ai-chat-screen component into
shared color tokens or constants. The colors being repeated are the background
colors (for the View with the rounded-full styling), the indicator dot colors,
and the text colors for both offline and online states. Create centralized
constants (such as CONNECTION_STATE_COLORS with offline and online properties)
and replace all hardcoded hex values with references to these constants. This
includes the backgroundColor properties in the View components and the color
properties in the Text component, as well as any similar color usage mentioned
in lines 400-407.
In `@components/connectivity/OfflineNotice.tsx`:
- Around line 6-10: The OfflineNotice component contains multiple hardcoded
color values (bg-[`#FFF7ED`], bg-white, color="`#C2410C`", text-[`#9A3412`]) that are
scattered across the View and Text elements as well as the WifiOff icon color
prop. Extract these hardcoded colors along with their related typography tokens
into shared constants or theme classes in your design system. Replace each
hardcoded value in the OfflineNotice component's className attributes and the
WifiOff color prop with references to these shared constants to ensure
consistent styling across the application.
In `@components/errors/FeatureErrorBoundary.tsx`:
- Around line 42-43: The FeatureErrorBoundary component contains hardcoded color
values (bg-[`#FFF1F5`] and text-[`#9F1239`]) in the Tailwind classes that are
repeated across multiple error components including InlineErrorMessage and
RootErrorFallback. Extract these literal color values into shared design token
constants (either in a constants file or global styles configuration) with
semantic names that describe their purpose (e.g., errorBackgroundColor,
errorTextColor), then replace all occurrences of the hardcoded hex colors in
FeatureErrorBoundary, InlineErrorMessage, and RootErrorFallback with references
to these shared tokens to maintain consistency and simplify future updates.
- Around line 50-63: The Retry button in the FeatureErrorBoundary component is
conditionally rendered only when onRetry is provided, leaving users with no
recovery path when this prop is absent. Remove the conditional check around the
Pressable component so the button is always rendered. Keep the existing onPress
logic that resets the error state with this.setState({ error: null }) and calls
this.props.onRetry?.() which will safely handle cases where onRetry is not
provided through optional chaining. This ensures users always have a local
recovery action available, even when the boundary is used without an onRetry
handler.
In `@components/journal-editor/journal-editor-screen.tsx`:
- Around line 829-835: The conditional logic for determining the save label has
incorrect precedence. When checking `entrySyncStatus === "synced"` second, it
can incorrectly return "Synced" even when there are unsaved edits (wasSaved is
false) on a previously synced entry. Reorder the conditions so that the check
for unsaved edits takes precedence. The `entrySyncStatus === "synced"` condition
should only be evaluated and returned after confirming that all changes have
been saved (wasSaved is true). This ensures that "Save" is returned whenever
there are unsaved edits, regardless of the current sync status.
In `@components/legal/legal-document-screen.tsx`:
- Around line 18-48: The constants legalDocumentColors, legalDocumentSpacing,
and legalDocumentLayout define inline style values that should be moved to the
Tailwind theme configuration instead. Add these color and spacing values to your
Tailwind config under the appropriate theme sections (extend colors and
spacing), then replace usage of these constants with NativeWind className
utilities (e.g., bg-legal-card, text-legal-heading) instead of style props. The
shadow constants legalDocumentCardShadow and legalDocumentIconShadow can remain
as inline styles per the guidelines exception for shadows.
In `@components/profile/profile-screen.tsx`:
- Around line 147-154: The setSupabaseAccessTokenProvider(null) call is being
executed before the signOut() operation completes. If signOut() fails and throws
an exception caught by the catch block, the token provider will already be
nulled, breaking subsequent authenticated requests. Move the
setSupabaseAccessTokenProvider(null) call into the try block after the await
signOut() call succeeds so that the token is only cleared when sign-out is
guaranteed to succeed. This ensures that if the operation fails, the token
provider remains intact and the user can attempt authentication again.
In `@components/sync/auto-sync-manager.tsx`:
- Around line 125-160: The `retryAttemptRef.current += 1` statement executes
synchronously when the effect runs, but if the effect re-runs before the timeout
fires (due to dependency changes like connectivity status or pending changes),
the cleanup cancels the timeout while the counter has already been incremented.
Move the increment statement from before the setTimeout call into the callback
function passed to setTimeout, so that the retry counter only increments when
the retry actually executes rather than when the effect re-runs.
In `@components/sync/SyncStatusIndicator.tsx`:
- Around line 18-52: The statusTheme object contains hardcoded hex color values
that are repeated across the sync UI components, creating maintenance and
consistency risks. Extract all backgroundColor and color hex values from the
statusTheme object (for the failed, idle, paused, saved_locally, synced,
syncing, and waiting_for_network status states) into centralized design token
constants in a shared location (such as a design tokens file or constants
directory). Then replace each hardcoded hex value in statusTheme with references
to the corresponding constants to ensure consistency and reduce drift across
sync UI components.
In `@components/sync/SyncStatusRow.tsx`:
- Around line 22-31: The SyncStatusRow component hardcodes multiple color values
throughout its styling, including `#27272A`, `#71717B`, and `#FF2056` (mentioned at
lines 22-31, 58-59, and 75-79). Extract all these hardcoded color literals into
shared color constants or global style definitions, then replace each hardcoded
color value in the component with references to the appropriate constant to
maintain consistency across sync surfaces and follow the coding guidelines for
reusable design tokens.
- Around line 55-65: The accessibilityLabel prop on the button element is static
and always set to copy.retryLabel, but screen readers should announce the actual
state of the button when isRetrying is true. Update the accessibilityLabel to be
conditional so that it displays "Syncing..." when isRetrying is true and
copy.retryLabel when isRetrying is false, matching the visual text content
changes already implemented in the Text component.
In `@lib/sync/requestSync.ts`:
- Around line 105-113: In the block where isSupabaseConfigured is false, the
retryable field in the returned object is currently set to true, but
configuration errors cannot be resolved by retrying. Change the retryable
property from true to false in the return statement within the
!isSupabaseConfigured condition to properly reflect that this is a configuration
or environment issue that requires manual intervention, not a transient failure
that can be automatically retried.
- Around line 199-211: The filter operation has a performance issue where
pendingEntryIds.includes(entry.id) performs an O(n) lookup for each entry being
filtered, resulting in O(n×m) complexity. To fix this, convert pendingEntryIds
to a Set before the filter operation (store it in a new variable like
pendingEntryIdsSet), then use the Set.has() method instead of Array.includes()
within the filter predicate to achieve O(1) lookup time per entry.
In `@store/useSyncStore.ts`:
- Around line 183-187: The isRecord type guard function is duplicated in
multiple files including useSyncStore.ts, lib/errors/normalizeAppError.ts,
lib/account/accountDeletionService.ts, and
supabase/functions/delete-account/index.ts. Create a new shared utility file at
lib/utils/typeGuards.ts and move the isRecord function definition there, then
update all four files to import and use the isRecord function from this
centralized utility instead of maintaining their own local copies.
- Around line 189-202: The hardcoded appErrorCodes array in useSyncStore.ts can
become out of sync with the AppErrorCode type definition in types/appError.ts.
Move the appErrorCodes array to types/appError.ts and use a const assertion on
it, then derive the AppErrorCode type from the array using (typeof
appErrorCodes)[number] pattern. This ensures TypeScript will verify the type and
array stay synchronized. Finally, import appErrorCodes from types/appError.ts in
useSyncStore.ts instead of maintaining a duplicate array declaration.
In `@types/appError.ts`:
- Around line 16-28: The AppErrorCode union type contains two error codes,
sync_conflict and invalid_data, that are never actually used or produced
anywhere in the codebase. Remove these two unused string literals from the
AppErrorCode type definition to keep the union type accurate and maintainable.
Verify after removal that no other code references these specific error code
strings.
---
Outside diff comments:
In `@hooks/useAutoSync.ts`:
- Around line 124-126: The startOfLocalDay helper function is no longer being
used in the file since its consumers (getReflectionStreak and getLocalDateKey)
were removed in this PR. Delete the entire startOfLocalDay function definition
to remove the dead code and keep the file clean.
In `@supabase/functions/delete-account/index.ts`:
- Around line 335-388: The current implementation accumulates all files in the
`filesToRemove` array before calling `client.storage.from(bucket).remove()` once
with the entire batch, which can cause payload size or timeout issues for large
deletions. Instead of the single `removeResult` call at the end, implement a
batching approach that processes `filesToRemove` in chunks (e.g., 1000 files per
batch). Loop through the accumulated files, slicing them into smaller batches
and calling the remove operation for each batch separately, ensuring all batches
complete successfully before returning the final result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1113f796-4b15-424b-9caf-e7179bfeb2bc
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (34)
app/(tabs)/ai-chat/index.tsxapp/_layout.tsxapp/insights/report/[periodType].tsxapp/settings/privacy.tsxcomponents/ai-chat/ai-chat-screen.tsxcomponents/auth/auth-screen.tsxcomponents/connectivity/OfflineNotice.tsxcomponents/errors/FeatureErrorBoundary.tsxcomponents/errors/InlineErrorMessage.tsxcomponents/errors/RootErrorFallback.tsxcomponents/journal-editor/journal-editor-screen.tsxcomponents/legal/legal-document-screen.tsxcomponents/profile/profile-screen.tsxcomponents/sync/SyncStatusIndicator.tsxcomponents/sync/SyncStatusRow.tsxcomponents/sync/auto-sync-manager.tsxhooks/useAIInsightReport.tshooks/useAutoSync.tshooks/useConnectivity.tshooks/useSyncStatus.tslib/account/accountDeletionService.tslib/account/clearLocalUserData.tslib/errors/normalizeAppError.tslib/errors/reportAppError.tslib/sync/requestSync.tspackage.jsonproviders/ConnectivityProvider.tsxstore/useSyncStore.tssupabase/config.tomlsupabase/functions/delete-account/index.tstypes/appError.tstypes/connectivity.tstypes/syncResult.tstypes/syncStatus.ts
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.
[+] Implemented Reliable Global Error Handling, Offline Awareness, and Sync Status Clarity.
[+] Implemented code-rabbit's suggested bug fixes and improvements.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements