Conversation
Added Improvements for Version 0.9.7
|
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis pull request implements notification telemetry tracking, invite code infrastructure, unseen entry tracking with refresh mechanisms, and comprehensive typography standardization. Changes span backend notification services with PostHog integration, frontend social features (friends, invites), vault entry management with real-time updates, and database schema extensions for invite codes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| # Determine recipients based on sharing settings | ||
| recipient_user_ids: List[str] = [] | ||
| # Remove owner from the list of recipients | ||
| recipient_user_ids: List[str] = [user_id for user_id in shared_with if user_id != owner_id] if isinstance(shared_with, list) else [] |
There was a problem hiding this comment.
Shared-with-everyone notifications no longer sent to friends
High Severity
The logic for handling shared_with_everyone entries was removed. Previously, when an entry had shared_with_everyone=True, the code fetched all friends via _get_user_friends() and sent notifications to them. Now the code only uses the shared_with list, completely ignoring the shared_with_everyone flag. Entries shared with "everyone" will not trigger notifications to any friends unless they're also explicitly in the shared_with array. The _get_user_friends helper method was also deleted.
| message: `${shareOptions.message}\n\n${shareOptions.url}`, | ||
| url: shareOptions.url, | ||
| }); | ||
| const result = await Share.share(shareOptions); |
There was a problem hiding this comment.
Share invite link missing URL on Android devices
Medium Severity
The Share.share() call was changed from explicitly concatenating the URL into the message to passing shareOptions directly. On Android, the url property in React Native's Share API is often ignored and not displayed. The original code used message: \${shareOptions.message}\n\n${shareOptions.url}`to ensure the invite link appeared in the shared content on all platforms. The existing implementation infriend-service.ts` still uses the correct pattern.
| .select(`*, profile:${TABLES.PROFILES}(*)`) | ||
| .contains('shared_with', [user.id]) | ||
| .gte('created_at', tenMinutesAgo) | ||
| .order('created_at', { ascending: false }) |
There was a problem hiding this comment.
Offline reconnection misses shared-with-everyone entries
Medium Severity
The offline reconnection handler only queries entries where shared_with contains the user ID (.contains('shared_with', [user.id])), but doesn't account for entries with shared_with_everyone=true. This is inconsistent with the realtime handler at lines 127-131, which correctly checks both sharedWithEveryone and sharedWith.includes(user.id). Users reconnecting after being offline will miss entries that were shared with everyone during their offline period.
| inviterName: inviterName, | ||
| inviterEmail: inviterProfile?.email || '', | ||
| inviterAvatar: inviterProfile?.avatar_url || 'https://images.pexels.com/photos/1239291/pexels-photo-1239291.jpeg?auto=compress&cs=tinysrgb&w=200', | ||
| inviterAvatar: inviterProfile?.avatar_url || getDefaultAvatarUrl(inviterName), |
There was a problem hiding this comment.
Missing null check causes crash with invalid invite code
High Severity
The query changed from using .single() (on the old INVITES table) to .maybeSingle() (on PROFILES table), but the null check wasn't updated. With .maybeSingle(), when no profile matches the invite code, the result is { data: null, error: null }. The code only checks if (error) at line 94, which passes when error is null. At line 111, accessing inviterProfile.id will throw a TypeError because inviterProfile is null. Users visiting invalid invite links will experience a crash.
![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 (13)
frontend/hooks/use-friend-invitation.ts (2)
20-20:⚠️ Potential issue | 🟠 Major
inviteLinkis always truthy — the guard on lines 24/38 is ineffective.
inviteLinkis built from a template literal, so even whenprofile?.invite_codeisundefined, it evaluates to something like"…/invite/undefined"— a truthy string. Theif (!inviteLink)guards never trigger, and the user could copy/share a malformed URL.Guard against a missing invite code directly:
Proposed fix
- const inviteLink = `${baseUrl}/invite/${profile?.invite_code}` + const inviteCode = profile?.invite_code; + const inviteLink = inviteCode ? `${baseUrl}/invite/${inviteCode}` : '';Also applies to: 24-26, 38-40
42-50:⚠️ Potential issue | 🟠 Major
urlis iOS-only in React Native'sShare.share— Android users won't see the link.On Android,
Share.shareonly usesmessage; theurlproperty is ignored. The currentmessagedoesn't contain the actual URL, so Android users will share just the text with no invite link.Proposed fix — append URL to message
const shareOptions: ShareOptions = { title: 'Join me on Keepsafe', - message: 'I\'d love to share moments with you on Keepsafe! Join me using this link:', + message: `I'd love to share moments with you on Keepsafe! Join me using this link: ${inviteLink}`, url: inviteLink, ...options, };frontend/hooks/use-invite-acceptance.ts (3)
84-121:⚠️ Potential issue | 🔴 CriticalNull-pointer crash when
invite_codedoesn't match any profile.
maybeSingle()returns{ data: null, error: null }when no row matches. The code checks forerroron line 94 but never checks whetherinviteitself isnull. The cast on line 106 will makeinviterProfilenull, and line 111 accessesinviterProfile.idwithout optional chaining — causing aTypeErrorat runtime for any invalid/unknown invite code.🐛 Proposed fix: add a null check for the query result
if (error) { setError('This invitation link is invalid or has expired.'); return null; } + if (!invite) { + setError('This invitation link is invalid or has expired.'); + return null; + } + //const isUsed = invite.current_uses >= invite.max_uses;
163-183:⚠️ Potential issue | 🟠 Major
declineInviteis a no-op — always returns success without performing any action.The
trybody is empty. The UI in[id].tsxwill show "Invitation Declined" and navigate the user away, but no backend state is modified. If declining should have an effect (e.g., deleting a pending friendship, recording a rejection), this needs implementation. If it's intentionally deferred, consider adding a// TODOcomment and/or not exposing the decline button until it's functional.
48-59:⚠️ Potential issue | 🟡 MinorSilently succeeding on duplicate friendship masks the "already connected" state from the user.
When an existing friendship is found, the mutation returns a result with
friendshipIdbut the caller (line 147-151) always shows "Invitation accepted successfully!" — the user never sees the'You are already connected with this user'message. Consider either throwing an error for duplicates or propagating the message to the caller so it can display appropriate feedback.frontend/supabase/migrations/20250825200450_misty_boat.sql (1)
340-365:⚠️ Potential issue | 🟡 MinorTrigger does not handle unique constraint collisions on
invite_code.
set_profile_invite_code()assigns a generated code but has no retry logic if the code collides with an existing one. With 62⁸ ≈ 218 trillion possibilities, collisions are extremely unlikely at small scale, but the INSERT will hard-fail with a unique violation if it ever happens. The backfill migration (the second file) correctly retries onunique_violation—consider applying similar retry logic here, or at minimum document the accepted risk.frontend/services/search-service.ts (1)
80-97:⚠️ Potential issue | 🟠 Major
onErroris invoked twice for non-OK responses.When the response is not OK:
- Line 87 calls
onError?.(error)- Line 97 throws the error
- The
catchblock at Line 171 callsonError(error)againThis results in duplicate error handling for the consumer.
Proposed fix: remove the early onError call since the throw will reach the catch block
if (!response.ok) { const error = new Error(`Search request failed with status ${response.status}`); logger.error('SearchService: non-OK response', { status: response.status, statusText: response.statusText, - }); - onError?.(error); try { const errorText = await response.text(); logger.error('SearchService: non-OK response body', { status: response.status, body: errorText, }); } catch { // ignore body-read errors here } throw error; }frontend/app/settings/blocked-users.tsx (1)
79-97:⚠️ Potential issue | 🟡 MinorAvatar placeholder branch is now unreachable.
getDefaultAvatarUrlalways returns a non-empty URL string, soavatarUrlon Line 79 is always truthy. Theelsebranch (Lines 91-97) with the placeholder initial is dead code.frontend/app/capture/details.tsx (1)
253-261:⚠️ Potential issue | 🟡 MinorDuplicate toast on every early return path.
Line 260 fires unconditionally within the outer
if, so the user always sees two toasts when validation fails. If!hasSelectedSharing(), they get "Please select who to share..." and "Cannot save entry". This appears to be a pre-existing bug in the function you're modifying.Proposed fix: remove the duplicate toast
if (!capture || !user || !hasSelectedSharing()) { if (!hasSelectedSharing()) { toast('Please select who to share this entry with', 'error'); } else { toast('Cannot save entry', 'error'); } - toast('Cannot save entry', 'error'); return; }frontend/hooks/use-user-entries.ts (1)
80-80:⚠️ Potential issue | 🟡 MinorMisleading staleTime comment.
The comment says "5 minutes" but the value is
1000 * 60 * 10which is 10 minutes.✏️ Proposed fix
- staleTime: 1000 * 60 * 10, // 5 minutes + staleTime: 1000 * 60 * 10, // 10 minutesfrontend/app/friends.tsx (1)
68-79:⚠️ Potential issue | 🟡 MinorSending a local notification to yourself on accepting a friend request is unusual.
LocalNotificationService.sendNotificationfires a device notification to the current user who just tapped "Accept." Typically the other party should be notified. If this is intentional as an in-app confirmation, a toast (like the other handlers use) would be less intrusive and more consistent. Theawaitalso means the UI blocks until the notification is dispatched (including the permission prompt on first use).Consider replacing with a toast for consistency with the other handlers, or at least making the notification fire-and-forget (remove
await).frontend/services/friend-service.ts (2)
316-322:⚠️ Potential issue | 🟠 MajorPostgREST filter injection via unsanitized email/phone values in
.or()/.in.()clauses.Emails and phone numbers are interpolated directly into the PostgREST filter string. The
isValidEmailregex (/^[^\s@]+@[^\s@]+\.[^\s@]+$/) permits commas and parentheses in the local part (e.g.,a]b,c)d@evil.com), which can break or manipulate the.in.(...)filter syntax when joined with commas.For example, a contact email like
x]y,z)@evil.comwould produce:email.in.(x]y,z)@evil.com,legit@example.com)This corrupts the PostgREST filter and may produce unexpected results or errors.
Mitigations (pick one or combine):
- Tighten the email regex to reject commas, parentheses, and brackets in the local part.
- Escape or URL-encode filter values before interpolation.
- Use parameterized RPC calls instead of client-side filter string construction.
Option 1: Tighten email regex
private static isValidEmail(email: string): boolean { if (!email || typeof email !== 'string') return false; - // RFC 5322 compliant email regex (simplified but practical) - const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + // Practical email regex that excludes characters unsafe for PostgREST filters + const emailRegex = /^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)+$/; return emailRegex.test(email.trim()) && email.length <= 254; }
156-177:⚠️ Potential issue | 🟠 MajorError and null checks happen after
datais already consumed.Line 156 uses
data?.map(...)(safe due to optional chaining), but the error check at Line 175 and the null-data guard at Line 173 both come after the map. If Supabase returns{ data: null, error: someError }, the code returns[]instead of throwing, silently swallowing the error.Reorder to check error first, then null-data:
Proposed fix
- const friends: FriendWithProfile[] = data?.map(friend => { + if (error) { + throw new Error(error.message); + } + + if (!data) return []; + + const friends: FriendWithProfile[] = data.map(friend => { const { user_profile: user, friend_profile: friend_, ...friend_record } = friend; const profile = friend.friend_id === userId ? user : friend_; return { ...friend_record, friend_profile: profile } - }) - if (!data) return []; - - if (error) { - throw new Error(error.message); - } - // Cache the friends data - if (data) { - await deviceStorage.setFriends(userId, friends); - } + await deviceStorage.setFriends(userId, friends); return friends as FriendWithProfile[];
🤖 Fix all issues with AI agents
In `@backend/services/notification_enqueue_service.py`:
- Around line 142-144: The body generation currently builds entry_type_display
as f"a {entry_type.capitalize()}" with only "audio" special-cased, causing "a
Entry" for the default "entry"; update the logic in
backend/services/notification_enqueue_service.py where entry_type_display is set
to compute the correct indefinite article by checking entry_type.lower()[0] in
"aeiou" (use 'an' for vowels, 'a' otherwise), then build entry_type_display
using the article and entry_type.lower() (or entry_type if you need different
casing), and leave the rest of the notification body (owner_name and title)
unchanged so the message becomes e.g. "an entry" or "a video" correctly.
In `@backend/services/notification_service.py`:
- Around line 109-117: The telemetry call in enqueue_notification synchronously
invokes _capture_notification_enqueued_event which performs blocking DB work via
_get_user_info_from_tokens, blocking the caller; update enqueue_notification to
offload that call to a background thread (use asyncio.to_thread) or make
enqueue_notification async and await asyncio.to_thread for
_capture_notification_enqueued_event, mirroring how the sent path handles
telemetry, so the DB queries no longer block the enqueue caller.
- Around line 779-780: Do not send raw user email to PostHog: stop setting
properties["email"] from the local email variable; instead only send non-PII
identifiers (e.g., user_id) or a pseudonymous value. Update the logic around the
properties dict (the properties variable and the email variable) to either
remove the email assignment entirely or gate it behind an explicit
config/consent flag (e.g., POSTHOG_ALLOW_PII) and/or replace the raw email with
a one-way hash (sha256) before adding to properties to ensure PII isn’t
transmitted by default.
- Around line 708-712: In the recipients loop in send/enqueue logic, the local
variable email is retrieved from token_user_info but never passed into the
enqueued event payload; update the enqueued-event creation (the enqueued event
handler code that mirrors the sent handler) to include email alongside user_id
(use token_user_info.get(token, {}).get("email")) so the enqueued event contains
the same email field the sent handler expects, or if email is truly unnecessary
remove the unused assignment; locate the loop iterating "for token in
recipients" and the enqueued event construction to make the change.
- Around line 248-257: The telemetry call via await
asyncio.to_thread(self._capture_notification_sent_event, ...) can raise and
bubble into the outer except, incorrectly incrementing stats["failed"]; wrap the
telemetry invocation in its own try/except so any exceptions from
asyncio.to_thread or self._capture_notification_sent_event are caught and logged
(or swallowed) without affecting stats, e.g., call asyncio.to_thread(...) inside
a try block and on failure log the exception and continue; ensure this change
references the existing asyncio.to_thread call and the
_capture_notification_sent_event method and does not alter the surrounding
success path that deletes the sent notification.
In `@backend/test_enqueue_service.py`:
- Around line 96-101: The validation guard in test_enqueue_service.py uses
placeholder strings that no longer match the constants; update the check to
validate against the actual default placeholders or the environment-variable
defaults so it can fail when not configured: change the condition that
references ENTRY_ID and OWNER_USER_ID to compare them to the placeholder/default
values used when loading from env (or check for empty/UUID-zero patterns) so the
guard triggers when ENTRY_ID or OWNER_USER_ID still hold the unconfigured
defaults; reference the constants ENTRY_ID and OWNER_USER_ID and adjust the
conditional accordingly.
- Around line 29-37: Replace the hardcoded real UUIDs and environment-specific
values in the test file: change ENTRY_ID, OWNER_USER_ID, and entries inside
SHARED_WITH_USER_IDS to non-production placeholders or read them from test
fixtures/environment variables, and keep ENTRY_TYPE as a controlled test
constant; update any test setup that references these variables (e.g.,
test_enqueue_service.py symbols ENTRY_ID, OWNER_USER_ID, SHARED_WITH_USER_IDS,
ENTRY_TYPE) so tests use injectable/fixture-provided IDs or os.getenv values
instead of committed real UUIDs.
In `@frontend/app/friends.tsx`:
- Around line 109-119: The comment is incorrect: handleRefresh only calls
refreshFriends() while refetchSuggestedFriends (destructured earlier) is unused;
update handleRefresh to call and await refetchSuggestedFriends as well (e.g.,
await Promise.all or sequential awaits inside handleRefresh) and change the
inline comment to accurately state both friends and suggested friends are being
refreshed; reference the handleRefresh function and the refetchSuggestedFriends
identifier so you update the correct spot.
In `@frontend/app/vault.tsx`:
- Around line 110-126: onViewableItemsChanged is being recreated because its
dependency array includes entriesByDate and unseenEntryIds, which breaks
FlashList; make the callback stable by moving its function to a ref-backed
handler: create refs (e.g., entriesByDateRef, unseenEntryIdsRef) that are
updated whenever entriesByDate or unseenEntryIds change, then define
onViewableItemsChanged once (no deps) using useCallback or store it in a ref so
it never changes; inside that stable handler read from entriesByDateRef.current
and unseenEntryIdsRef.current, compute visibleEntryIds as before and call
markEntriesAsSeen(visibleEntryIds). Ensure markEntriesAsSeen reference is stable
or read from a ref if it can change.
In `@frontend/components/error-boundary.tsx`:
- Around line 153-159: The style objects title, errorTitle, and buttonText in
frontend/components/error-boundary.tsx currently include both fontWeight and
fontFamily (e.g., fontWeight: '600' alongside fontFamily: 'Outfit-SemiBold');
remove the fontWeight entries from those style objects so only fontFamily is
used (aligning with the rest of the PR and avoiding Android rendering issues),
leaving title, errorTitle, and buttonText to specify only fontFamily and other
properties.
- Around line 153-159: The title style in the error boundary currently forces a
custom fontFamily ('Outfit-SemiBold'), which may not be available when the
boundary renders; update the title style in
frontend/components/error-boundary.tsx (the title style object) to use a safe
font stack or remove the custom font reference so it falls back to system fonts
(e.g., use a stacked value like 'Outfit-SemiBold, system-ui, -apple-system,
"Segoe UI", Roboto, "Helvetica Neue", Arial' or simply omit fontFamily) ensuring
the fallback UI renders legibly even if custom fonts haven't loaded.
In `@frontend/hooks/use-user-entries.ts`:
- Around line 95-101: The forEach callback in markEntriesAsSeen implicitly
returns the boolean from Set.delete (violating the linter); update the callback
to use a block body or a for...of loop so it does not return a value—e.g.,
within the setUnseenEntryIds updater for the updated Set, replace
entryIds.forEach(id => updated.delete(id)) with a non-returning form
(entryIds.forEach(id => { updated.delete(id); }) or for (const id of entryIds) {
updated.delete(id); }) ensuring the function name markEntriesAsSeen and state
updater setUnseenEntryIds are unchanged.
- Around line 226-232: Replace the async forEach usage
(missedEntries.forEach(async (entry) => { await deviceStorage.addEntry(user.id,
entry) ... })) with a Promise.all over missedEntries.map(...) so all addEntry
promises are tracked and failures are aggregated; either make the enclosing
.then() callback async and await Promise.all(mappedPromises) or return
Promise.all(mappedPromises) from the current .then() chain, and consolidate
error handling to catch and log any rejection from Promise.all for
deviceStorage.addEntry calls.
In `@frontend/lib/utils.ts`:
- Around line 90-106: groupBy, hasEntries, and getEntryCount produce
inconsistent date keys because groupBy builds YYYY-MM-DD from local Date methods
while the others use UTC via toISOString; fix this by deriving all date keys
through the shared timezone-aware helper getTimefromTimezone (call
getTimefromTimezone with the original timestamp or year/month/day inputs, then
use .toISOString().split('T')[0]) instead of using getFullYear/getMonth/getDate
or new Date(...).toISOString() conversions directly; update the groupBy
implementation (where it checks keys 'updated_at'/'created_at') to use
getTimefromTimezone, and ensure hasEntries and getEntryCount use the same
getTimefromTimezone-based YYYY-MM-DD extraction so all utilities produce
identical keys.
In `@frontend/services/search-service.ts`:
- Around line 82-86: The logger.error call in SearchService includes a trailing
comma after statusText, indicating a missing property; update the object passed
to logger.error (in the SearchService where logger.error is called) to either
remove the stray comma or add the intended field — e.g., include response.url
(or responseText/responseBody if you capture the body) so the log object
contains status, statusText, and the missing identifier, and ensure no stray
trailing comma remains.
In `@frontend/supabase/migrations/20250825200450_misty_boat.sql`:
- Around line 78-90: The generate_invite_code() function can pick index 0 due to
ceil(random()*62) when random() == 0; update the selection expression inside the
substring call to use a 1-based index by replacing ceil(random()*62) with
floor(random()*62)::int + 1 so the range is always 1..62 (update the expression
within the SELECT in function generate_invite_code to use
floor(random()*62)::int + 1).
🧹 Nitpick comments (18)
frontend/hooks/use-friend-invitation.ts (1)
53-56: Inconsistent error handling:shareInviteLinkre-throws whilecopyInviteLinkswallows errors.
copyInviteLinkcatches and returnsfalse, butshareInviteLinklogs and re-throws. Since both returnPromise<boolean>, callers likely expect uniform behavior. Consider returningfalseinstead of re-throwing, or document thatshareInviteLinkcan throw.frontend/hooks/use-invite-acceptance.ts (2)
130-161: Confusing parameter naming:inviteeIdis actually the inviter's ID.In
acceptInvite, the first parameterinviteeIdreceivesinviteData.idfrom the caller (the inviter's profile ID), whileuserIdis the current user. This makes the PostHog event on lines 141-144 accidentally correct (inviter_id: inviteeId) but the naming is misleading throughout the mutation as well (lines 39-46, 51, 66-67).Consider renaming the parameter to
inviterIdto match its semantic meaning and avoid future bugs.
99-104: Remove commented-out dead code.These commented-out lines for
isUsedchecking add noise. If this feature is planned for later, track it as a TODO or issue rather than leaving dead code in the file.backend/test_enqueue_service.py (1)
1-226: Consider whether this manual test script belongs in the repository.There are already proper integration tests at
backend/tests/test_notification_enqueue_service_integration.pywith mocked dependencies. This script hits a real database without mocks, has no assertions, and is essentially a developer utility. If it's needed, consider moving it to ascripts/ortools/directory and adding it to.gitignoreor documenting its purpose clearly.backend/services/notification_service.py (2)
636-688: Two DB round-trips per telemetry call — consider caching or batching.
_get_user_info_from_tokensissues two separate DB queries (push_tokens + profiles) every time telemetry fires. Since both enqueued and sent events call this, a single notification lifecycle triggers four queries purely for analytics. Consider caching the token→user mapping (it rarely changes) or passing user info through the message metadata to avoid these lookups entirely.
690-800: Significant duplication between_capture_notification_enqueued_eventand_capture_notification_sent_event.These two methods are nearly identical — only the event name and the email inclusion differ. Extract common logic into a shared helper.
Sketch
+ def _capture_notification_event( + self, + event_name: str, + metadata: Optional[Dict[str, Any]], + recipients: List[str], + title: str, + body: str, + priority: str, + include_email: bool = False, + ) -> None: + if not self.posthog_client: + return + try: + notification_type = metadata.get("notification_type", "unknown") if metadata else "unknown" + token_user_info = self._get_user_info_from_tokens(recipients) + for token in recipients: + user_info = token_user_info.get(token, {}) + user_id = user_info.get("user_id") + email = user_info.get("email") + properties: Dict[str, Any] = { + "notification_type": notification_type, + "title": title, + "body": body, + "priority": priority, + "recipient_count": len(recipients), + } + if user_id: + properties["user_id"] = user_id + if include_email and email: + properties["email"] = email + if metadata: + for key, value in metadata.items(): + if key not in properties and key != "notification_type": + properties[f"metadata_{key}"] = value + distinct_id = user_id if user_id else f"token_{token[:8]}" + self.posthog_client.capture(distinct_id=distinct_id, event=event_name, properties=properties) + self.posthog_client.flush() + except Exception as e: + logger.error(f"Error capturing {event_name} event to PostHog: {str(e)}")frontend/app/settings/index.tsx (2)
273-278: Avatar fallback is inconsistent withprofile.tsx.This file now uses
getDefaultAvatarUrl()for the avatar fallback (good), butfrontend/app/settings/profile.tsxat Line 110 still uses a hardcoded Pexels URL as the fallback. Consider applying the samegetDefaultAvatarUrlpattern there for consistency.
420-424: UnusedprofileEmailstyle.Per the summary, the profile email line was removed from the JSX, but the
profileEmailstyle definition remains. Consider removing it to avoid dead code.frontend/app/settings/profile.tsx (1)
108-112: Avatar fallback still uses a hardcoded URL.This screen still uses a hardcoded Pexels URL as avatar fallback, while
settings/index.tsxwas updated to usegetDefaultAvatarUrl(). Consider updating this for consistency.Proposed fix
+import { getDefaultAvatarUrl } from '@/lib/utils'; + // ... <Image source={{ - uri: profile?.avatar_url || 'https://images.pexels.com/photos/1239291/pexels-photo-1239291.jpeg?auto=compress&cs=tinysrgb&w=200' + uri: profile?.avatar_url || getDefaultAvatarUrl(profile?.full_name || 'Unknown User') }} style={styles.profilePhoto} />frontend/services/local-notification-service.ts (1)
25-33: Good defensive sanitization for badge values.The guard against non-integer/non-finite values is solid. One edge case: negative integers (e.g.,
-1) will pass validation but are invalid badge counts on iOS. Consider addingbadge < 0to the rejection condition.Optional: reject negative badge values
- if (typeof badge !== 'number' || !Number.isFinite(badge) || !Number.isInteger(badge)) { + if (typeof badge !== 'number' || !Number.isFinite(badge) || !Number.isInteger(badge) || badge < 0) { return undefined; }frontend/components/error-boundary.tsx (2)
15-49:deviceInfomissing from useEffect dependency array.
deviceInfois referenced inside the effect but not listed in[error]. SincegetDeviceInfo()produces a new object each render, adding it would cause infinite re-runs. The current approach works because the component only re-renders whenerrorchanges, but a linter may flag this. Consider moving thegetDeviceInfo()call inside the effect to be explicit and avoid the lint warning.Move getDeviceInfo inside the effect
function ErrorFallback({ error, resetError }: ErrorFallbackProps) { - // Collect device information - const deviceInfo = getDeviceInfo(); - // Capture error to PostHog React.useEffect(() => { + const deviceInfo = getDeviceInfo(); try { const errorProperties: Record<string, string> = {Then keep a separate
getDeviceInfo()call for the local logger call, or move the logger call inside the effect as well (since it's also a side effect).
122-133: Alignfallbackprop type withRNErrorBoundaryexpectations.The
fallbackprop is typed as(props: ErrorFallbackProps) => ReactNode, butRNErrorBoundary'sFallbackComponentexpectsReact.ComponentType<ErrorFallbackProps>. While function components work at runtime, the type mismatch could cause issues with stricter type checking. Align the typing toReact.ComponentType<ErrorFallbackProps>for precision.Suggested change
interface ErrorBoundaryProps { children: ReactNode; - fallback?: (props: ErrorFallbackProps) => ReactNode; + fallback?: React.ComponentType<ErrorFallbackProps>; }frontend/app/_layout.tsx (1)
45-74: Good placement of ErrorBoundary at the root level.Wrapping the entire provider/layout tree ensures any uncaught render error is caught centrally.
One nit: Line 57 (
<Stack.Screen name="vault" />) has inconsistent indentation compared to its siblings (lines 53–56, 58–65 all use extra leading whitespace).frontend/app/vault.tsx (1)
247-253: Redundant visibility guard.The JSX condition on line 247 (
unseenEntryIds.size > 0) already prevents rendering, makingvisible={unseenEntryIds.size > 0}on line 251 alwaystrue. You can simplify by always passingvisible={true}or removing the outer condition and letting the component's internal guard (line 12 innew-entries-indicator.tsx) handle it alone.frontend/hooks/use-suggested-friends.ts (1)
66-79: Side effect insidesetQueryDataupdater callback.
deviceStorage.setSuggestedFriends(updated)at line 73 is a side effect fired from within the updater function passed tosetQueryData. React Query's updater should be a pure transformation. While it works today, the updater can be called multiple times or batched in certain scenarios, leading to redundant storage writes. Move the storage sync outside:♻️ Proposed refactor
const removeContactFromList = useCallback((friendId: string) => { - queryClient.setQueryData<SuggestedFriend[]>(['suggested-friends'], (oldData) => { - if (!oldData) return []; - const updated = oldData.filter(contact => contact.id !== friendId); - - // Sync device storage optimistically - deviceStorage.setSuggestedFriends(updated).catch((error) => { - logger.warn('Failed to update device storage optimistically:', error); - }); - - return updated; - }); + const oldData = queryClient.getQueryData<SuggestedFriend[]>(['suggested-friends']) ?? []; + const updated = oldData.filter(contact => contact.id !== friendId); + queryClient.setQueryData<SuggestedFriend[]>(['suggested-friends'], updated); + + // Sync device storage optimistically + deviceStorage.setSuggestedFriends(updated).catch((error) => { + logger.warn('Failed to update device storage optimistically:', error); + }); }, [queryClient]);frontend/hooks/use-friends.ts (1)
236-253: Duplicated query logic betweenprefetchSuggestedFriendsanduseSuggestedFriends.The queryFn here (fetch → filter by profile.id → sync to device storage) is a near-duplicate of the queryFn in
use-suggested-friends.ts(lines 44–57). If one changes without the other, they'll silently diverge. Consider extracting a shared fetcher function, or havingprefetchSuggestedFriendssimply callqueryClient.prefetchQuerywith the same query key and letuseSuggestedFriendsdefine the sole queryFn via aqueryOptionsfactory.frontend/app/friends.tsx (1)
130-142:convertToFriendFormatusesany[]— loses type safety.Consider typing the parameter as
FriendWithProfile[](or the appropriate type from your types) to get compile-time checks on the field accesses (friend_profile,status, etc.).frontend/services/friend-service.ts (1)
62-64: Inconsistent logging:console.log/console.errorused alongsidelogger.Lines 64, 72, 84, 103–106, 108, 224, 348, etc. still use raw
console.*calls while the rest of the file uses the structuredlogger. This scatters log output across two systems and may leak info in production (theloggersuppresses debug in non-dev builds).Consider replacing all
console.*calls with theirloggerequivalents for consistency.
| entry_type_display = f"a {entry_type.capitalize()}" if entry_type != "audio" else "an audio recording" | ||
| title = "New Entry Shared" | ||
| body = f"{owner_name} shared a {entry_type_display} with you" | ||
| body = f"{owner_name} shared {entry_type_display} with you" |
There was a problem hiding this comment.
Grammatical bug: "a Entry" when entry_type is the default value "entry".
When entry_type defaults to "entry" (line 93), this produces "a Entry" — should be "an entry". The vowel check only handles "audio" but misses this case.
Proposed fix
- entry_type_display = f"a {entry_type.capitalize()}" if entry_type != "audio" else "an audio recording"
+ if entry_type == "audio":
+ entry_type_display = "an audio recording"
+ elif entry_type[0].lower() in "aeiou":
+ entry_type_display = f"an {entry_type}"
+ else:
+ entry_type_display = f"a {entry_type}"📝 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.
| entry_type_display = f"a {entry_type.capitalize()}" if entry_type != "audio" else "an audio recording" | |
| title = "New Entry Shared" | |
| body = f"{owner_name} shared a {entry_type_display} with you" | |
| body = f"{owner_name} shared {entry_type_display} with you" | |
| if entry_type == "audio": | |
| entry_type_display = "an audio recording" | |
| elif entry_type[0].lower() in "aeiou": | |
| entry_type_display = f"an {entry_type}" | |
| else: | |
| entry_type_display = f"a {entry_type}" | |
| title = "New Entry Shared" | |
| body = f"{owner_name} shared {entry_type_display} with you" |
🤖 Prompt for AI Agents
In `@backend/services/notification_enqueue_service.py` around lines 142 - 144, The
body generation currently builds entry_type_display as f"a
{entry_type.capitalize()}" with only "audio" special-cased, causing "a Entry"
for the default "entry"; update the logic in
backend/services/notification_enqueue_service.py where entry_type_display is set
to compute the correct indefinite article by checking entry_type.lower()[0] in
"aeiou" (use 'an' for vowels, 'a' otherwise), then build entry_type_display
using the article and entry_type.lower() (or entry_type if you need different
casing), and leave the rest of the notification body (owner_name and title)
unchanged so the message becomes e.g. "an entry" or "a video" correctly.
|
|
||
| # Capture PostHog event for notification enqueued | ||
| self._capture_notification_enqueued_event( | ||
| metadata=metadata, | ||
| recipients=recipients, | ||
| title=title, | ||
| body=body, | ||
| priority=priority | ||
| ) |
There was a problem hiding this comment.
Synchronous telemetry with DB queries blocks the enqueue caller.
_capture_notification_enqueued_event makes two synchronous DB queries via _get_user_info_from_tokens (lines 706, 651, 666). Unlike the sent path (line 250), this isn't offloaded to a thread, so it blocks the caller of enqueue_notification. Consider using the same asyncio.to_thread pattern, or making enqueue_notification async like the sent path does.
Note: If enqueue_notification is always called from an async context, you could make it async and use await asyncio.to_thread(...) here too. Otherwise, fire-and-forget via a thread would work.
🤖 Prompt for AI Agents
In `@backend/services/notification_service.py` around lines 109 - 117, The
telemetry call in enqueue_notification synchronously invokes
_capture_notification_enqueued_event which performs blocking DB work via
_get_user_info_from_tokens, blocking the caller; update enqueue_notification to
offload that call to a background thread (use asyncio.to_thread) or make
enqueue_notification async and await asyncio.to_thread for
_capture_notification_enqueued_event, mirroring how the sent path handles
telemetry, so the DB queries no longer block the enqueue caller.
|
|
||
| # Capture PostHog event for notification sent (offload blocking I/O to thread) | ||
| await asyncio.to_thread( | ||
| self._capture_notification_sent_event, | ||
| metadata=metadata, | ||
| recipients=recipients, | ||
| title=title, | ||
| body=body, | ||
| priority=priority | ||
| ) |
There was a problem hiding this comment.
Telemetry failure in to_thread will propagate and increment stats["failed"].
If _capture_notification_sent_event raises (e.g., DB timeout), the await asyncio.to_thread(...) on line 250 will raise into the outer except block (line 267), causing stats["failed"] to be incremented even though the notification was successfully sent and deleted. The telemetry call should be wrapped in its own try/except to avoid corrupting stats.
Proposed fix
logger.info(f"Successfully processed notification: msg_id={msg_id}")
- # Capture PostHog event for notification sent (offload blocking I/O to thread)
- await asyncio.to_thread(
- self._capture_notification_sent_event,
- metadata=metadata,
- recipients=recipients,
- title=title,
- body=body,
- priority=priority
- )
+ # Capture PostHog event for notification sent (offload blocking I/O to thread)
+ try:
+ await asyncio.to_thread(
+ self._capture_notification_sent_event,
+ metadata=metadata,
+ recipients=recipients,
+ title=title,
+ body=body,
+ priority=priority
+ )
+ except Exception:
+ logger.warning(f"Failed to capture telemetry for msg_id={msg_id}", exc_info=True)📝 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.
| # Capture PostHog event for notification sent (offload blocking I/O to thread) | |
| await asyncio.to_thread( | |
| self._capture_notification_sent_event, | |
| metadata=metadata, | |
| recipients=recipients, | |
| title=title, | |
| body=body, | |
| priority=priority | |
| ) | |
| # Capture PostHog event for notification sent (offload blocking I/O to thread) | |
| try: | |
| await asyncio.to_thread( | |
| self._capture_notification_sent_event, | |
| metadata=metadata, | |
| recipients=recipients, | |
| title=title, | |
| body=body, | |
| priority=priority | |
| ) | |
| except Exception: | |
| logger.warning(f"Failed to capture telemetry for msg_id={msg_id}", exc_info=True) |
🤖 Prompt for AI Agents
In `@backend/services/notification_service.py` around lines 248 - 257, The
telemetry call via await
asyncio.to_thread(self._capture_notification_sent_event, ...) can raise and
bubble into the outer except, incorrectly incrementing stats["failed"]; wrap the
telemetry invocation in its own try/except so any exceptions from
asyncio.to_thread or self._capture_notification_sent_event are caught and logged
(or swallowed) without affecting stats, e.g., call asyncio.to_thread(...) inside
a try block and on failure log the exception and continue; ensure this change
references the existing asyncio.to_thread call and the
_capture_notification_sent_event method and does not alter the surrounding
success path that deletes the sent notification.
| # Capture event for each recipient | ||
| for token in recipients: | ||
| user_info = token_user_info.get(token, {}) | ||
| user_id = user_info.get("user_id") | ||
| email = user_info.get("email") |
There was a problem hiding this comment.
email is fetched but never used in the enqueued event handler.
Static analysis confirms email on line 712 is assigned but never referenced. The sent event handler (line 779-780) does use it. Either this is an oversight and email should be included here too, or the fetch is unnecessary overhead.
If email should be included (matching the sent handler)
if user_id:
properties["user_id"] = user_id
+ if email:
+ properties["email"] = email🧰 Tools
🪛 Ruff (0.14.14)
[error] 712-712: Local variable email is assigned to but never used
Remove assignment to unused variable email
(F841)
🤖 Prompt for AI Agents
In `@backend/services/notification_service.py` around lines 708 - 712, In the
recipients loop in send/enqueue logic, the local variable email is retrieved
from token_user_info but never passed into the enqueued event payload; update
the enqueued-event creation (the enqueued event handler code that mirrors the
sent handler) to include email alongside user_id (use token_user_info.get(token,
{}).get("email")) so the enqueued event contains the same email field the sent
handler expects, or if email is truly unnecessary remove the unused assignment;
locate the loop iterating "for token in recipients" and the enqueued event
construction to make the change.
| if email: | ||
| properties["email"] = email |
There was a problem hiding this comment.
Capturing user email in PostHog events is a PII/compliance risk.
Sending email addresses to a third-party analytics service (PostHog) may conflict with GDPR/CCPA requirements. Consider whether user_id alone is sufficient for your analytics needs, or ensure your PostHog data retention and processing agreements cover PII.
🤖 Prompt for AI Agents
In `@backend/services/notification_service.py` around lines 779 - 780, Do not send
raw user email to PostHog: stop setting properties["email"] from the local email
variable; instead only send non-PII identifiers (e.g., user_id) or a
pseudonymous value. Update the logic around the properties dict (the properties
variable and the email variable) to either remove the email assignment entirely
or gate it behind an explicit config/consent flag (e.g., POSTHOG_ALLOW_PII)
and/or replace the raw email with a one-way hash (sha256) before adding to
properties to ensure PII isn’t transmitted by default.
| const markEntriesAsSeen = useCallback((entryIds: string[]) => { | ||
| setUnseenEntryIds(prev => { | ||
| const updated = new Set(prev); | ||
| entryIds.forEach(id => updated.delete(id)); | ||
| return updated; | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
Static analysis: forEach callback should not return a value.
Set.delete() returns a boolean, so the arrow function implicitly returns it. Use a block body to satisfy the linter.
✏️ Proposed fix
const markEntriesAsSeen = useCallback((entryIds: string[]) => {
setUnseenEntryIds(prev => {
const updated = new Set(prev);
- entryIds.forEach(id => updated.delete(id));
+ entryIds.forEach(id => { updated.delete(id); });
return updated;
});
}, []);📝 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.
| const markEntriesAsSeen = useCallback((entryIds: string[]) => { | |
| setUnseenEntryIds(prev => { | |
| const updated = new Set(prev); | |
| entryIds.forEach(id => updated.delete(id)); | |
| return updated; | |
| }); | |
| }, []); | |
| const markEntriesAsSeen = useCallback((entryIds: string[]) => { | |
| setUnseenEntryIds(prev => { | |
| const updated = new Set(prev); | |
| entryIds.forEach(id => { updated.delete(id); }); | |
| return updated; | |
| }); | |
| }, []); |
🧰 Tools
🪛 Biome (2.3.13)
[error] 98-98: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@frontend/hooks/use-user-entries.ts` around lines 95 - 101, The forEach
callback in markEntriesAsSeen implicitly returns the boolean from Set.delete
(violating the linter); update the callback to use a block body or a for...of
loop so it does not return a value—e.g., within the setUnseenEntryIds updater
for the updated Set, replace entryIds.forEach(id => updated.delete(id)) with a
non-returning form (entryIds.forEach(id => { updated.delete(id); }) or for
(const id of entryIds) { updated.delete(id); }) ensuring the function name
markEntriesAsSeen and state updater setUnseenEntryIds are unchanged.
| missedEntries.forEach(async (entry: EntryWithProfile) => { | ||
| try { | ||
| await deviceStorage.addEntry(user.id, entry); | ||
| } catch (storageError) { | ||
| console.error('Error saving missed entry to storage:', storageError); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the relevant file
fd 'use-user-entries.ts' --type fRepository: fortune710/keepsafe
Length of output: 97
🏁 Script executed:
# Read the relevant file, focusing on lines 190-240 to get full context
cat -n frontend/hooks/use-user-entries.ts | sed -n '190,240p'Repository: fortune710/keepsafe
Length of output: 2741
🏁 Script executed:
# Check for other instances of forEach(async pattern in the codebase
rg -n 'forEach\s*\(\s*async' --type=ts --type=tsx -g '!node_modules/**'Repository: fortune710/keepsafe
Length of output: 90
🏁 Script executed:
# Check for other instances of forEach(async pattern in the codebase (adjusted for available types)
rg -n 'forEach\s*\(\s*async' --type=ts -g '!node_modules/**'Repository: fortune710/keepsafe
Length of output: 178
🏁 Script executed:
# Let's check if there are any other async/await issues or patterns related to Supabase queries in this file
rg -n '\.then\s*\(' frontend/hooks/use-user-entries.ts -A 5 | head -50Repository: fortune710/keepsafe
Length of output: 332
forEach with async callbacks — unhandled promises and no operation completion tracking.
Each iteration spawns an independent, untracked promise. If the loop contains multiple entries, you have concurrent device storage writes with no aggregated error handling and no way for the caller to know when the operation completes. Use Promise.all with map to properly track completion and surface any failures across all entries.
Implementation note: The parent .then() at line 203 is not async. To use await Promise.all(...), either convert that callback to async or chain the Promise.all to an additional .then() block.
🔧 Proposed fix
- missedEntries.forEach(async (entry: EntryWithProfile) => {
- try {
- await deviceStorage.addEntry(user.id, entry);
- } catch (storageError) {
- console.error('Error saving missed entry to storage:', storageError);
- }
- });
+ Promise.all(
+ missedEntries.map(async (entry: EntryWithProfile) => {
+ try {
+ await deviceStorage.addEntry(user.id, entry);
+ } catch (storageError) {
+ console.error('Error saving missed entry to storage:', storageError);
+ }
+ })
+ );🤖 Prompt for AI Agents
In `@frontend/hooks/use-user-entries.ts` around lines 226 - 232, Replace the async
forEach usage (missedEntries.forEach(async (entry) => { await
deviceStorage.addEntry(user.id, entry) ... })) with a Promise.all over
missedEntries.map(...) so all addEntry promises are tracked and failures are
aggregated; either make the enclosing .then() callback async and await
Promise.all(mappedPromises) or return Promise.all(mappedPromises) from the
current .then() chain, and consolidate error handling to catch and log any
rejection from Promise.all for deviceStorage.addEntry calls.
| if ((key === 'updated_at' || key === 'created_at') && typeof item[key] === 'string') { | ||
| // Group by day (YYYY-MM-DD) if key is 'updated_at' | ||
| // Group by day (YYYY-MM-DD) in local timezone if key is 'updated_at' or 'created_at' | ||
| const date = new Date(item[key] as string); | ||
| groupKey = date.toISOString().slice(0, 10); | ||
| if (!isNaN(date.getTime())) { | ||
| // Use local date components to match DateContainer display | ||
| const year = date.getFullYear(); | ||
| const month = String(date.getMonth() + 1).padStart(2, '0'); | ||
| const day = String(date.getDate()).padStart(2, '0'); | ||
| groupKey = `${year}-${month}-${day}`; | ||
| } else { | ||
| // Invalid date - group under 'invalid-date' or skip | ||
| groupKey = 'invalid-date'; | ||
| } | ||
| } else { | ||
| groupKey = String(item[key]); | ||
| const value = item[key]; | ||
| groupKey = value != null ? String(value) : 'null'; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n frontend/lib/utils.ts | head -120Repository: fortune710/keepsafe
Length of output: 4657
🏁 Script executed:
wc -l frontend/lib/utils.tsRepository: fortune710/keepsafe
Length of output: 88
🏁 Script executed:
# Find the file first
find . -name "utils.ts" -path "*/frontend/lib/*"Repository: fortune710/keepsafe
Length of output: 86
🏁 Script executed:
sed -n '180,220p' frontend/lib/utils.tsRepository: fortune710/keepsafe
Length of output: 1574
🏁 Script executed:
# Search for getTimefromTimezone, hasEntries, getEntryCount
rg -n 'getTimfromTimezone|hasEntries|getEntryCount' frontend/lib/utils.ts -A5 -B2Repository: fortune710/keepsafe
Length of output: 691
🏁 Script executed:
# Also search for toISOString usage in the file
rg -n 'toISOString' frontend/lib/utils.ts -B2 -A2Repository: fortune710/keepsafe
Length of output: 892
🏁 Script executed:
# Search for getTimefromTimezone implementation
rg -n 'getTimfromTimezone' frontend/ -A10 -B2Repository: fortune710/keepsafe
Length of output: 45
🏁 Script executed:
# Also check how the groupBy output is actually used - search for calls to groupBy and entriesData construction
rg -n 'groupBy.*created_at|groupBy.*updated_at' frontend/ -B2 -A5Repository: fortune710/keepsafe
Length of output: 590
🏁 Script executed:
# Search for where entriesData is populated/created
rg -n 'entriesData|Record<string, number>' frontend/ -B3 -A3Repository: fortune710/keepsafe
Length of output: 4262
🏁 Script executed:
# See the actual entriesData construction in calendar/index.tsx
sed -n '22,35p' frontend/app/calendar/index.tsxRepository: fortune710/keepsafe
Length of output: 424
🏁 Script executed:
# Search for getTimfromTimezone definition (note: might be spelled differently)
rg -n 'export.*getTime.*[Tt]imezone' frontend/Repository: fortune710/keepsafe
Length of output: 142
🏁 Script executed:
sed -n '242,260p' frontend/lib/utils.tsRepository: fortune710/keepsafe
Length of output: 628
🏁 Script executed:
# Also check the actual flow - where does entriesByDate from groupBy get used?
rg -n 'entriesByDate' frontend/ -B2 -A2Repository: fortune710/keepsafe
Length of output: 2929
🏁 Script executed:
# Verify the actual timezone impact - what does TZDate.toISOString() return?
# Also check if groupBy is actually used in calendar context
rg -n 'entriesByDate' frontend/app/calendar/ -B3 -A3Repository: fortune710/keepsafe
Length of output: 45
🏁 Script executed:
# Check if there's any direct comparison between groupBy keys and hasEntries
rg -n 'hasEntries.*entriesByDate|entriesByDate.*hasEntries' frontend/Repository: fortune710/keepsafe
Length of output: 45
🏁 Script executed:
# Let's verify the actual behavior of the two approaches with a concrete example
python3 << 'EOF'
from datetime import datetime, timezone, timedelta
# Simulate what the code does for a date in positive UTC offset timezone
# Example: Feb 8, 2026 in JST (UTC+9)
# When parsed from ISO string "2026-02-08T10:00:00Z" (10 AM UTC)
# Local in JST: Feb 8, 2026 7 PM
# Approach 1: groupBy uses local date methods
# If someone is in JST and entry was created at 2026-02-08T10:00:00Z
# new Date parses UTC, but getDate() returns LOCAL date
iso_string = "2026-02-08T10:00:00Z"
print("ISO String from server:", iso_string)
# In JavaScript, new Date("2026-02-08T10:00:00Z") creates UTC date
# But getDate() returns the LOCAL date component
# For someone in JST (UTC+9): 10:00 UTC + 9 hours = 19:00 JST on Feb 8
# So getDate() would return 8
print("\nApproach 1 (groupBy - uses local getDate()):")
print(" Entry ISO: 2026-02-08T10:00:00Z")
print(" In JST (UTC+9): 2026-02-08 19:00:00")
print(" getDate() returns: 8")
print(" groupKey: 2026-02-08")
# Approach 2: hasEntries uses new Date(year, month, day).toISOString()
# new Date(2026, 1, 8) creates a LOCAL date (Feb 8 00:00:00 in local timezone)
# For JST user: 2026-02-08 00:00:00 JST = 2026-02-07 15:00:00 UTC
# toISOString() returns: "2026-02-07T15:00:00Z"
print("\nApproach 2 (hasEntries - creates local date, converts to UTC ISO):")
print(" new Date(2026, 1, 8) in JST: 2026-02-08 00:00:00 JST")
print(" In UTC: 2026-02-07 15:00:00 UTC")
print(" toISOString(): 2026-02-07T15:00:00Z")
print(" dateKey: 2026-02-07")
print("\n⚠️ MISMATCH: groupBy produces '2026-02-08' but hasEntries looks for '2026-02-07'")
EOFRepository: fortune710/keepsafe
Length of output: 571
🏁 Script executed:
# Now check what getTimefromTimezone + toISOString does
# This is used in calendar/index.tsx
python3 << 'EOF'
# Approach 3: calendar/index.tsx uses getTimefromTimezone + toISOString
# getTimefromTimezone creates a TZDate (timezone-aware)
# TZDate.toISOString() should return the correct UTC date regardless of local timezone
print("Approach 3 (calendar/index.tsx - uses getTimefromTimezone + TZDate):")
print(" Entry ISO: 2026-02-08T10:00:00Z (10:00 UTC)")
print(" getTimefromTimezone wraps in TZDate with device timezone")
print(" TZDate.toISOString() for JST should return: 2026-02-08T10:00:00Z")
print(" dateKey: 2026-02-08 (UTC date, which is correct)")
print("\n✓ This approach is timezone-aware and correct")
EOFRepository: fortune710/keepsafe
Length of output: 401
🏁 Script executed:
# Double-check: does the current code mix these approaches anywhere?
# Check if anyone calls groupBy output with hasEntries
rg -n 'groupBy' frontend/hooks/use-user-entries.ts -A10 -B5Repository: fortune710/keepsafe
Length of output: 1163
🏁 Script executed:
# Let's verify one more thing - does getTimefromTimezone exist and is it exported?
rg -n 'getTimefromTimezone|getDeviceTimezone' frontend/lib/utils.ts -B2 -A8Repository: fortune710/keepsafe
Length of output: 1289
🏁 Script executed:
# Check how the timezone situation impacts real usage - are there any timezone-aware date utilities?
rg -n 'getDeviceTimezone|TZDate' frontend/ -lRepository: fortune710/keepsafe
Length of output: 123
🏁 Script executed:
# Final check - verify if the LOCAL date keys from groupBy actually cause issues
# by checking if vault.tsx ever intersects with calendar date logic
rg -n 'getEntriesForDate|hasEntries|getEntryCount' frontend/app/vault.tsxRepository: fortune710/keepsafe
Length of output: 45
🏁 Script executed:
# Check date-container.tsx to see if it uses any of these functions
cat frontend/components/date-container.tsxRepository: fortune710/keepsafe
Length of output: 1381
Timezone inconsistency in date-key derivation across utility functions.
groupBy (lines 90–106) derives date keys using local date methods (getDate(), getMonth(), getFullYear()), producing keys like "2026-02-08". However, hasEntries and getEntryCount (lines 192–200) derive keys via new Date(year, month, day).toISOString().split('T')[0], which converts a locally-constructed date to UTC, producing different keys for the same entry in non-UTC timezones. For example, in JST (UTC+9), an entry created on Feb 8 would produce "2026-02-08" from groupBy but "2026-02-07" from hasEntries.
Currently, the code paths are isolated: groupBy is used only in vault.tsx via entriesByDate, while hasEntries/getEntryCount are used only in calendar/index.tsx. However, calendar/index.tsx itself correctly builds entriesData using getTimefromTimezone(...).toISOString().split('T')[0], which is timezone-aware. To prevent latent bugs if these utilities are ever combined, unify date-key derivation by using getTimefromTimezone consistently across all grouping, filtering, and counting operations.
🤖 Prompt for AI Agents
In `@frontend/lib/utils.ts` around lines 90 - 106, groupBy, hasEntries, and
getEntryCount produce inconsistent date keys because groupBy builds YYYY-MM-DD
from local Date methods while the others use UTC via toISOString; fix this by
deriving all date keys through the shared timezone-aware helper
getTimefromTimezone (call getTimefromTimezone with the original timestamp or
year/month/day inputs, then use .toISOString().split('T')[0]) instead of using
getFullYear/getMonth/getDate or new Date(...).toISOString() conversions
directly; update the groupBy implementation (where it checks keys
'updated_at'/'created_at') to use getTimefromTimezone, and ensure hasEntries and
getEntryCount use the same getTimefromTimezone-based YYYY-MM-DD extraction so
all utilities produce identical keys.
| logger.error('SearchService: non-OK response', { | ||
| status: response.status, | ||
| statusText: response.statusText, | ||
|
|
||
| }); |
There was a problem hiding this comment.
Incomplete log object — looks like a missing property.
Line 85 has a trailing comma after statusText with nothing following it, suggesting a property was accidentally deleted or never added.
🤖 Prompt for AI Agents
In `@frontend/services/search-service.ts` around lines 82 - 86, The logger.error
call in SearchService includes a trailing comma after statusText, indicating a
missing property; update the object passed to logger.error (in the SearchService
where logger.error is called) to either remove the stray comma or add the
intended field — e.g., include response.url (or responseText/responseBody if you
capture the body) so the log object contains status, statusText, and the missing
identifier, and ensure no stray trailing comma remains.
| -- Create function to generate invite code | ||
| CREATE OR REPLACE FUNCTION generate_invite_code() | ||
| RETURNS text AS $$ | ||
| BEGIN | ||
| RETURN array_to_string( | ||
| ARRAY( | ||
| SELECT substring('ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789' | ||
| FROM (ceil(random()*62))::int FOR 1) | ||
| FROM generate_series(1, 8) | ||
| ), '' | ||
| ); | ||
| END; | ||
| $$ LANGUAGE plpgsql VOLATILE; |
There was a problem hiding this comment.
ceil(random()*62) can produce 0, yielding an empty-string character.
random() returns [0.0, 1.0). When it returns 0.0, ceil(0.0*62) = ceil(0) = 0. substring(... FROM 0 FOR 1) in PostgreSQL returns '', so the resulting code will be fewer than 8 characters. Use floor(random()*62)::int + 1 instead to guarantee the range [1, 62].
🔧 Proposed fix
CREATE OR REPLACE FUNCTION generate_invite_code()
RETURNS text AS $$
BEGIN
RETURN array_to_string(
ARRAY(
SELECT substring('ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'
- FROM (ceil(random()*62))::int FOR 1)
+ FROM (floor(random()*62)::int + 1) FOR 1)
FROM generate_series(1, 8)
), ''
);
END;
$$ LANGUAGE plpgsql VOLATILE;📝 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.
| -- Create function to generate invite code | |
| CREATE OR REPLACE FUNCTION generate_invite_code() | |
| RETURNS text AS $$ | |
| BEGIN | |
| RETURN array_to_string( | |
| ARRAY( | |
| SELECT substring('ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789' | |
| FROM (ceil(random()*62))::int FOR 1) | |
| FROM generate_series(1, 8) | |
| ), '' | |
| ); | |
| END; | |
| $$ LANGUAGE plpgsql VOLATILE; | |
| -- Create function to generate invite code | |
| CREATE OR REPLACE FUNCTION generate_invite_code() | |
| RETURNS text AS $$ | |
| BEGIN | |
| RETURN array_to_string( | |
| ARRAY( | |
| SELECT substring('ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789' | |
| FROM (floor(random()*62)::int + 1) FOR 1) | |
| FROM generate_series(1, 8) | |
| ), '' | |
| ); | |
| END; | |
| $$ LANGUAGE plpgsql VOLATILE; |
🤖 Prompt for AI Agents
In `@frontend/supabase/migrations/20250825200450_misty_boat.sql` around lines 78 -
90, The generate_invite_code() function can pick index 0 due to
ceil(random()*62) when random() == 0; update the selection expression inside the
substring call to use a 1-based index by replacing ceil(random()*62) with
floor(random()*62)::int + 1 so the range is always 1..62 (update the expression
within the SELECT in function generate_invite_code to use
floor(random()*62)::int + 1).
1 participant
Note
Medium Risk
Touches push notification processing, analytics emission, and realtime data updates across backend/frontend; failures could affect notification delivery/metrics accuracy or cause duplicated/missed entry updates if edge cases aren’t handled.
Overview
Improves notification observability and UX by adding PostHog tracking for push notifications (captured on enqueue and on successful send, with recipient user/email resolved from push tokens) and by tweaking entry-share push payloads to deep-link to
vaultwith arefreshhint.On the client, adds a global
ErrorBoundarythat reports crashes to PostHog with device/app metadata, adds realtime entry ingestion for newly shared entries with an unseen entries indicator inVault, and enables pull-to-refresh + refresh-on-deeplink forFriends.Also includes assorted UX/polish changes: local notifications on entry save and friend-request acceptance, suggested-friends caching/sync via device storage, safer
groupBy(local-date grouping + null guards), invite acceptance now looks up inviter viaprofiles.invite_code, plus minor toast/layout scaling and font-family updates across settings screens.Written by Cursor Bugbot for commit c3f8982. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Style