Conversation
This comment has been minimized.
This comment has been minimized.
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThe change adds a complete chat platform with typed APIs, persisted optimistic messaging, SignalR events, chatbot sessions, channel and thread screens, message actions, attachments, moderation, push deep links, navigation, and localization. ChangesChat platform
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChatScreen
participant ChatStore
participant ChatAPI
participant ChatHub
User->>ChatScreen: open channel and compose message
ChatScreen->>ChatStore: send message
ChatStore->>ChatAPI: persist optimistic message
ChatStore->>ChatHub: join channel and send typing state
ChatHub-->>ChatStore: receive message and receipt events
ChatStore-->>ChatScreen: reconcile messages and update indicators
ChatScreen-->>User: render conversation state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pnpm-workspace.yaml (1)
1-7: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReplace every unresolved
allowBuildsplaceholder with an explicit boolean.Each entry currently uses the pnpm-generated placeholder and remains unapproved while it stays unchanged. This can keep install-time build scripts blocked for
@react-buoy/shared-ui,@sentry/cli,electron,electron-winstaller, orpostinstall-postinstall.🤖 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 `@pnpm-workspace.yaml` around lines 1 - 7, Replace the pnpm-generated “set this to true or false” values under allowBuilds with explicit boolean values for `@react-buoy/shared-ui`, `@sentry/cli`, electron, electron-winstaller, and postinstall-postinstall, selecting approval according to each package’s required install-time build behavior.
🟠 Major comments (19)
src/services/push-notification.ts-29-34 (1)
29-34: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAccept both chat deep-link prefix cases.
handleChatDeepLinkcurrently only matchest:andg:. Existing notification payload tests useT:9101andG:1121; current uppercase notifications returnfalseand do not navigate to the chat route. Accept uppercase chat prefixes too, or normalize only the chat prefix before navigating. Add response tests forhandleChatDeepLinkwith both cases.🤖 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 `@src/services/push-notification.ts` around lines 29 - 34, Update handleChatDeepLink to accept both lowercase and uppercase chat prefixes (t:, g:, T:, and G:) while preserving the existing channelId extraction and navigation behavior. Normalize or extend only the prefix matching, and add response tests covering lowercase and uppercase inputs.src/services/push-notification.ts-238-241 (1)
238-241: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winQueue chat notification deep-links until the router is ready.
handleNotificationResponsecallshandleChatDeepLink, which returnsfalseand discards the link whenrouter.pushis not ready during startup. Store or wait withuseRootNavigationStatebefore navigating so a tapped notification response does not open the app to the initial app screen without opening the intended chat.🤖 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 `@src/services/push-notification.ts` around lines 238 - 241, Update handleNotificationResponse and the handleChatDeepLink flow to defer chat deep-links until the root navigation state indicates the router is ready, rather than discarding links when router.push cannot run during startup. Preserve each tapped eventCode and navigate to the intended chat once navigation becomes available.src/api/chat/chat.ts-188-198 (1)
188-198: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove the manual
Content-Typeheader on the attachment upload.
uploadAttachmentposts a React NativeFormDataobject, but setsheaders: { 'Content-Type': 'multipart/form-data' }without a boundary. Let Axios/runtime set this header forFormDataso the multipart body has a valid boundary.🛠️ Proposed fix
const response = await api.post<ChatAttachmentUploadedResult>(`${CHAT}/UploadAttachment`, form, { params: { channelId, messageId }, - headers: { 'Content-Type': 'multipart/form-data' }, });🤖 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 `@src/api/chat/chat.ts` around lines 188 - 198, Remove the manually specified Content-Type header from the api.post options in uploadAttachment, allowing Axios/runtime to set the multipart FormData header with its boundary; preserve the existing params and request body.src/stores/chat/store.ts-306-347 (1)
306-347: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPersisted outbox items cannot rebuild their optimistic bubble after a restart.
Only
outboxis persisted (line 750);messagesByChannelis not. On app restart with pending items,drainOutboxresends them, but no optimistic bubble exists in the meantime. The user sees the message disappear until the server row arrives. Two supporting defects:
sendMessagenever setsoutboxItem.SenderDisplayName, althoughChatOutboxItemdocuments that field as the snapshot for optimistic rendering. The field is currently dead, and the optimistic message hardcodesSenderDisplayName: ''(line 328).args.localAttachmentUriis stored only on the in-memory optimistic message (line 341), never inoutboxItem. An image queued while offline loses its local URI on restart, so the resend posts anImagemessage with no attachment and no way to recover the file.Persist the display name and the attachment URI on the outbox item, and rehydrate optimistic bubbles from the outbox on store init.
🤖 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 `@src/stores/chat/store.ts` around lines 306 - 347, Update sendMessage to persist SenderDisplayName and localAttachmentUri on ChatOutboxItem, then reuse those fields when constructing the optimistic message instead of hardcoded or transient values. During store initialization, rebuild pending optimistic bubbles from persisted outbox items—including their channel, sender, message, metadata, and attachment fields—before drainOutbox resends them, using the existing optimistic message/upsert flow.src/stores/chat/store.ts-358-363 (1)
358-363: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
drainOutboxretries permanently failed messages forever with no backoff or re-entrancy guard.
handleChatConnected(line 719) callsdrainOutboxon every chat hub connect and reconnect.drainOutboxresends every queued item unconditionally. Three consequences:
- An item that can never succeed stays queued forever. A
400on an oversized body or a403/404on a deleted channel is caught bysendOutboxItem, markedfailed, and kept inoutbox(line 795). Each reconnect retries it again. On a flapping connection this is an unbounded retry loop against the API with no backoff.- The outbox grows without bound. Nothing evicts items by age or attempt count, and the queue is persisted, so it survives restarts.
- No re-entrancy guard exists. Two overlapping
onChatConnectedevents rundrainOutboxconcurrently over the same snapshot and double-post every item.ClientMessageIdprevents duplicate server rows, but the duplicate requests are still sent.Add an
Attemptscounter and aLastAttemptAtfield toChatOutboxIteminsrc/models/v4/chat/outbox.ts. Skip items that exceed a maximum attempt count or a maximum age, distinguish permanent4xxfailures from transient ones, and add anisDrainingguard.🤖 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 `@src/stores/chat/store.ts` around lines 358 - 363, The outbox drain needs bounded retries and concurrency protection. Extend ChatOutboxItem with Attempts and LastAttemptAt, update sendOutboxItem to record attempts and timestamps, classify permanent 4xx failures so they are removed or no longer retried, and skip items exceeding the configured attempt or age limits. Add an isDraining guard around drainOutbox so overlapping handleChatConnected calls cannot process the same snapshot concurrently.src/stores/chat/store.ts-544-557 (1)
544-557: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
chatbotTypingcan staytrueindefinitely.
sendChatbotMessagesetschatbotTyping: trueand clears it in only two places: the send-failurecatch(line 555) andhandleChatbotMessageReceived(line 675). The send itself succeeds even when the asynchronous bot reply never arrives, for example on a bot-side error, a droppedchatbotMessageReceivedSignalR event, or a disconnect between the send and the reply. In those cases the typing indicator stays visible with no recovery path, because the flag is not reset on reconnect either.Add a timeout that clears
chatbotTypingafter a bounded wait, and clear it inhandleChatConnected.Note on line 554:
markOutboxFailedis called for a chatbot message that was never added tooutbox. The call works only because the helper patchesmessagesByChannelbyClientMessageId. Rename the helper or usepatchMessagedirectly, so the intent is clear.🤖 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 `@src/stores/chat/store.ts` around lines 544 - 557, The chatbot typing state needs bounded recovery and clearer failure handling. In sendChatbotMessage, add a bounded timeout after setting chatbotTyping that resets it if no reply arrives, clear the state in handleChatConnected, and preserve existing reply/failure cleanup. Rename markOutboxFailed or replace it with patchMessage for the chatbot failure path so it clearly updates the failed message rather than implying an outbox entry.src/stores/chat/store.ts-219-228 (1)
219-228: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftStore actions swallow every failure with no state the UI can render.
fetchChannelscatches the error, logs it, and clears the loading flag. The state carries no error field, so a failed fetch is indistinguishable from an empty channel list. The same pattern applies toloadInitialMessages(line 251),loadOlderMessages(line 279),editMessage(line 371),deleteMessage(line 380),moderatorDeleteMessage(line 389),addReaction(line 413),removeReaction(line 432),acknowledgeMessage(line 441),togglePin(line 478), andflagMessage(line 486).The optimistic actions are the higher risk.
addReaction,removeReaction,togglePin, anddeleteMessagemutate local state first and never roll back on failure. The user sees a reaction or a deletion that the server rejected, and the local state stays wrong until a full reload.Add an error slice (or a per-action error) so screens can show feedback, and roll back the optimistic mutation in the
catchofaddReaction,removeReaction, andtogglePin.As per coding guidelines: "Handle errors gracefully and provide user feedback".
🤖 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 `@src/stores/chat/store.ts` around lines 219 - 228, Update the chat store state and actions to expose fetch/mutation failures through an error slice or per-action errors, clearing errors on successful operations so the UI can provide feedback; apply this consistently to fetchChannels, loadInitialMessages, loadOlderMessages, editMessage, deleteMessage, moderatorDeleteMessage, addReaction, removeReaction, acknowledgeMessage, togglePin, and flagMessage. In the catch blocks for addReaction, removeReaction, and togglePin, restore the exact pre-mutation state before recording the error; preserve loading-state cleanup and existing logging.Source: Coding guidelines
src/stores/chat/store.ts-589-608 (1)
589-608: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
handleMessageReceivedinflatesUnreadCounton duplicate delivery.
upsertMessagededuplicates the message itself, but line 600 incrementsUnreadCountunconditionally for any message that is not own and not in the active channel. The same message can reach this handler more than once:
- SignalR redelivers the event after a reconnect.
loadNewerMessagesalready fetched the message duringhandleChatConnected(line 722), and the hub event arrives afterwards.Each delivery adds one to the badge, so the unread count drifts above the real number and only corrects on the next
fetchChannels.Increment only when the message is new to the list.
🐛 Proposed fix
set((s) => { - const list = upsertMessage(s.messagesByChannel[msg.ChatChannelId] ?? [], { ...msg, _localStatus: 'sent' }); + const previous = s.messagesByChannel[msg.ChatChannelId] ?? []; + const isNew = !previous.some((m) => m.ChatMessageId === msg.ChatMessageId || (!!msg.ClientMessageId && m.ClientMessageId === msg.ClientMessageId)); + const list = upsertMessage(previous, { ...msg, _localStatus: 'sent' }); const channels = s.channels.map((c) => c.ChatChannelId === msg.ChatChannelId - ? { ...c, LastMessageSeq: Math.max(c.LastMessageSeq, msg.MessageSeq), LastMessageOn: msg.SentOn, UnreadCount: isActive || isOwn ? c.UnreadCount : c.UnreadCount + 1 } + ? { + ...c, + LastMessageSeq: Math.max(c.LastMessageSeq, msg.MessageSeq), + LastMessageOn: msg.SentOn, + UnreadCount: isActive || isOwn || !isNew ? c.UnreadCount : c.UnreadCount + 1, + } : c );🤖 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 `@src/stores/chat/store.ts` around lines 589 - 608, Update handleMessageReceived to determine whether msg.MessageSeq is already present in the channel’s existing message list before calling set. Increment UnreadCount only for a new, non-own message in an inactive channel; preserve upsertMessage deduplication and leave the count unchanged for duplicate deliveries.src/stores/signalr/signalr-store.ts-622-632 (1)
622-632: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winA stale
isChatHubConnectedflag permanently blocks chat reconnection.Line 624 returns early when
isChatHubConnectedistrue, but nothing reconciles that flag against the real hub state.checkConnectionState(line 502) reconciles onlyisUpdateHubConnected. The flag is cleared only bydisconnectChatHub.The chat hub can drop without
disconnectChatHubrunning.signalr.service.tsgives up afterMAX_RECONNECT_ATTEMPTSand deletes the connection and its config (lines 789-802) without notifying this store.isChatHubConnectedthen staystruewhile the hub is gone. Every laterconnectChatHubcall returns immediately, including thehandleAppResumecall insrc/hooks/use-signalr-lifecycle.ts. Chat realtime stays dead until the app restarts.Check the real connection state in the guard, as
signalRServicealready exposes it.🐛 Proposed fix
connectChatHub: async () => { try { - if (get().isChatHubConnected) { + if (get().isChatHubConnected && signalRService.isHubConnected(Env.CHAT_HUB_NAME)) { return; } + // The flag is stale; the hub dropped without an explicit disconnect. + if (get().isChatHubConnected) { + logger.warn({ message: 'Chat hub flag was stale, reconnecting' }); + stopChatHeartbeat(); + unregisterChatHubHandlers(); + set({ isChatHubConnected: false }); + }Also consider extending
checkConnectionStateto reconcileisChatHubConnected.🤖 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 `@src/stores/signalr/signalr-store.ts` around lines 622 - 632, Update connectChatHub to guard against the actual chat hub state exposed by signalRService rather than relying solely on the stale isChatHubConnected flag; only return early when the stored flag and live connection both indicate an active chat hub, otherwise continue reconnection. Also extend checkConnectionState to reconcile isChatHubConnected with the service’s current chat connection state.src/stores/signalr/signalr-store.ts-668-693 (1)
668-693: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThe
connectChatHubfailure path leaves chat handlers registered and skips the recovery hook.Two problems in this block.
First, the
catchat line 688 records the error but performs no cleanup. IfsignalRService.invoke(..., 'Connect')at line 677 rejects, the 15 handlers registered at lines 662-674 stay subscribed onsignalRService, and any heartbeat from a previous session keeps running.isChatHubConnectedstaysfalse, so the store reports "not connected" while the chat store still receives and applies hub events. CallstopChatHeartbeat()andunregisterChatHubHandlers()in thecatch.Second, line 678 sets
isChatHubConnected: trueimmediately after theConnectinvoke, independent of theonChatConnectedevent.handleChatConnectedruns only from the event handler at line 671. If the hub does not raiseonChatConnected, the store reports a connected chat hub, butfetchChannels,fetchPendingAcks, anddrainOutboxnever run. Queued outbound messages stay unsent. Call the recovery path after a successfulConnectinvoke as well, and makehandleChatConnectedidempotent.🛡️ Proposed fix
// Announce chat presence to the hub, then begin the periodic heartbeat. await signalRService.invoke(Env.CHAT_HUB_NAME, 'Connect'); set({ isChatHubConnected: true }); + // The hub may not raise onChatConnected; run recovery from the invoke result too. + useChatStore.getState().handleChatConnected(); stopChatHeartbeat(); chatHeartbeatTimer = setInterval(() => { signalRService.invoke(Env.CHAT_HUB_NAME, 'Heartbeat').catch(() => { // Heartbeat is best-effort; ignore transient failures. }); }, CHAT_HEARTBEAT_INTERVAL_MS); logger.info({ message: 'Chat hub handlers registered successfully' }); } catch (error) { const err = error instanceof Error ? error : new Error('Unknown error occurred'); + // Do not leave handlers or the heartbeat attached to a failed connection. + stopChatHeartbeat(); + unregisterChatHubHandlers(); + set({ isChatHubConnected: false }); logger.error({ message: 'Failed to connect to chat SignalR hub', context: { error: err } }); set({ error: err }); }🤖 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 `@src/stores/signalr/signalr-store.ts` around lines 668 - 693, Update connectChatHub so its catch path calls stopChatHeartbeat() and unregisterChatHubHandlers() before recording the error. After a successful Connect invoke, trigger the same chat-connected recovery path as the onChatConnected handler instead of only setting isChatHubConnected, and make handleChatConnected idempotent so event and invoke-based paths cannot duplicate recovery work.src/stores/chat/store.ts-455-471 (1)
455-471: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
markChannelReadadvances the dedupe marker before the request succeeds, so a failed markRead is never retried.Line 459 sets
lastMarkedSeqfor the channel before thechatApi.markReadcall. IfmarkReadrejects, thecatchat line 468 only logs. The guard at line 458 then blocks every later call for that sequence, becauselastMarkedSeqalready holds it. The localUnreadCountis also set to0(line 462). Result: the channel reads as read on the device but stays unread on the server until a newer message arrives.Advance
lastMarkedSeqonly after the request succeeds, or roll it back in thecatch.🐛 Proposed fix
const seq = highestRealSeq(get().messagesByChannel[channelId]); if (seq <= 0) return; - if ((lastMarkedSeq.get(channelId) ?? 0) >= seq) return; - lastMarkedSeq.set(channelId, seq); + const previousMarkedSeq = lastMarkedSeq.get(channelId) ?? 0; + if (previousMarkedSeq >= seq) return; + lastMarkedSeq.set(channelId, seq); set((s) => ({ channels: s.channels.map((c) => (c.ChatChannelId === channelId ? { ...c, UnreadCount: 0, MyLastReadSeq: seq } : c)), })); void safeInvoke('MarkRead', channelId, seq); try { await chatApi.markRead(channelId, { Seq: seq }); } catch (error) { + // Allow a later attempt to re-mark this sequence. + lastMarkedSeq.set(channelId, previousMarkedSeq); logger.debug({ message: 'chat: markRead failed', context: { error, channelId } }); }🤖 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 `@src/stores/chat/store.ts` around lines 455 - 471, Update markChannelRead so lastMarkedSeq is advanced only after chatApi.markRead succeeds, or restore its previous value when the request fails. Ensure failed requests remain retryable and avoid permanently treating the channel as read locally until the server call succeeds.src/app/chat/thread/[messageId].tsx-62-64 (1)
62-64: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe GIF and image buttons do nothing in a thread.
handleSendGifhas an empty body, andonSendImageis() => undefined.MessageComposerstill renders both controls as active. The image control callshandlePickImage, which requests photo-library permission and opens the picker, then discards the selected image. The user receives no feedback.Add optional capability flags to
MessageComposerand hide the unsupported controls in the thread screen.🐛 Proposed change
In
src/components/chat/message-composer.tsx:onSendText: (body: string, urgent: boolean) => void; - onSendImage: (uri: string, urgent: boolean) => void; - onSendLocation: (latitude: number, longitude: number, urgent: boolean) => void; - onOpenGif: () => void; + onSendImage?: (uri: string, urgent: boolean) => void; + onSendLocation?: (latitude: number, longitude: number, urgent: boolean) => void; + onOpenGif?: () => void;Render each control only when its handler is present.
In this file:
- const handleSendGif = useCallback(() => { - // GIFs in threads are sent as text-less messages via the composer's gif flow; kept minimal here. - }, []);<MessageComposer onSendText={handleSendText} - onSendImage={() => undefined} onSendLocation={handleSendLocation} - onOpenGif={handleSendGif} onTyping={() => undefined} placeholder={t('chat.reply_placeholder')} />Also applies to: 121-128
🤖 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 `@src/app/chat/thread/`[messageId].tsx around lines 62 - 64, Update MessageComposer to accept optional GIF and image handler capabilities and render each corresponding control only when its handler is provided. In the thread screen, stop exposing the unsupported controls by omitting the empty handleSendGif and onSendImage callbacks from MessageComposer, while preserving supported composer behavior.src/app/chat/thread/[messageId].tsx-30-37 (1)
30-37: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftFetch the root message and report a thread load failure.
rootis read only frommessagesByChannel[channelId]. A push deep link opens this screen without a populated channel cache, so the original message is not rendered. ThegetThreadfailure path also only writes to the log, and the screen shows an empty list with no error state and no loading state.Load the root message from the API when it is absent from the cache. Pass an
AbortSignaltogetThread; the API supports one (src/api/chat/chat.ts:111-117). Show a loading state and an error state.🤖 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 `@src/app/chat/thread/`[messageId].tsx around lines 30 - 37, Update the thread screen’s root-message and reply-loading flow: when `root` is absent from `channelMessages`, fetch the original message from the API and render it once loaded; create an `AbortController`, pass its signal to `getThread`, and abort it during effect cleanup. Track loading and failure state around the request, displaying appropriate loading and error UI instead of an empty thread, while preserving the existing successful reply rendering.src/components/chat/message-composer.tsx-117-132 (1)
117-132: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply
disabledto the attachment, GIF, and location controls.
src/app/chat/[channelId].tsxpassesdisabled={channel?.IsLocked && !isModerator}. OnlyTextareaInputand the send button honor that value. In a locked channel the user can still send an image, a GIF, or a location, because those handlers callonSendImage,onOpenGif, andonSendLocationdirectly.🐛 Proposed fix
- <Pressable className="p-2" onPress={handlePickImage} accessibilityLabel={t('chat.add_image')}> + <Pressable className="p-2" onPress={handlePickImage} disabled={disabled} accessibilityLabel={t('chat.add_image')}> <ImagePlus size={22} color="`#6b7280`" /> </Pressable> - <Pressable className="p-2" onPress={onOpenGif} accessibilityLabel={t('chat.add_gif')}> + <Pressable className="p-2" onPress={onOpenGif} disabled={disabled} accessibilityLabel={t('chat.add_gif')}> <Sparkles size={22} color="`#6b7280`" /> </Pressable> - <Pressable className="p-2" onPress={handleShareLocation} accessibilityLabel={t('chat.share_location')}> + <Pressable className="p-2" onPress={handleShareLocation} disabled={disabled} accessibilityLabel={t('chat.share_location')}> <MapPin size={22} color="`#6b7280`" /> </Pressable>🤖 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 `@src/components/chat/message-composer.tsx` around lines 117 - 132, Apply the existing disabled state to the image, GIF, and location Pressable controls in the message composer, alongside their current handlers. Ensure the locked-channel condition passed from the chat screen prevents handlePickImage, onOpenGif, and handleShareLocation from being invoked, while preserving current behavior when enabled.src/app/chat/[channelId].tsx-142-165 (1)
142-165: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftThe image upload is skipped silently when the confirmed message is not found.
The code awaits
sendMessage, then searches the store for a message whose_localAttachmentUrimatches and whose id is not prefixed withlocal-. If the send failed, or if the store has not yet replaced the optimistic row,sentisundefined. The function then returns without uploading and without user feedback. The bubble keeps only the device-local uri, so the image disappears on the next app start and other participants never receive it.Return the confirmed message id from
sendMessageinsrc/stores/chat/store.ts, then upload against that id. Report a failure to the user when no confirmed id is available.🛡️ Interim fix for the missing feedback
const sent = [...list].reverse().find((m) => m._localAttachmentUri === uri && !m.ChatMessageId.startsWith('local-')); - if (sent) { - try { - await uploadAttachment(channelId, sent.ChatMessageId, { uri, name, type: 'image/jpeg' }); - } catch { - useToastStore.getState().showToast('error', t('chat.attachment_failed')); - } + if (!sent) { + useToastStore.getState().showToast('error', t('chat.attachment_failed')); + return; + } + try { + await uploadAttachment(channelId, sent.ChatMessageId, { uri, name, type: 'image/jpeg' }); + } catch { + useToastStore.getState().showToast('error', t('chat.attachment_failed')); }🤖 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 `@src/app/chat/`[channelId].tsx around lines 142 - 165, Update sendMessage in the chat store to return the confirmed server message ID, then use that returned ID directly in handleSendImage for uploadAttachment instead of searching messagesByChannel. When sendMessage does not provide a confirmed ID, show the existing attachment failure toast and skip the upload.src/app/(app)/chat.tsx-27-42 (1)
27-42: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not define
Leadingduring render.
Leadingis created on everyChannelRowrender. React treats it as a new component type each time and unmounts the previous subtree. Inline the JSX or move the component to module scope and passchannelas a prop.♻️ Proposed fix
+function ChannelLeading({ channel }: { channel: ChatChannelResultData }) { + if (channel.ChannelType === ChatChannelType.DirectMessage) { + return ( + <Avatar size="md"> + <AvatarFallbackText>{getChannelDisplayName(channel)}</AvatarFallbackText> + </Avatar> + ); + } + const isIncident = channel.ChannelType === ChatChannelType.Incident || channel.ChannelType === ChatChannelType.IncidentLane || channel.ChannelType === ChatChannelType.IncidentCommand; + const Icon = channel.ChannelType === ChatChannelType.Chatbot ? Sparkles : isIncident ? Network : Users; + return ( + <Box className="size-10 items-center justify-center rounded-full bg-primary-100"> + <Icon size={20} color="`#2563eb`" /> + </Box> + ); +} + function ChannelRow({ channel, onPress }: { channel: ChatChannelResultData; onPress: () => void }) { const unread = channel.UnreadCount > 0; - const isDm = channel.ChannelType === ChatChannelType.DirectMessage; - - const Leading = () => { - ... - };Then render
<ChannelLeading channel={channel} />in place of<Leading />.🤖 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 `@src/app/`(app)/chat.tsx around lines 27 - 42, Move the Leading component out of the ChannelRow render scope to module scope, accepting channel (and the existing isDm behavior) as props, then render it through the stable ChannelLeading component. Preserve the current avatar, incident/chatbot icon selection, and styling.Source: Linters/SAST tools
src/components/chat/gif-picker-sheet.tsx-30-54 (1)
30-54: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCancel in-flight GIF searches and clear the debounce timer.
runSearchdoes not pass anAbortSignal, andsearchGifsaccepts one (src/api/chat/chat.ts:230-236). A slow earlier request can resolve after a later request and overwritegifswith stale results. The debounce timer is also never cleared on unmount, so a search runs after the sheet closes.♻️ Proposed fix
const debounceRef = useRef<ReturnType<typeof setTimeout> | null>(null); + const abortRef = useRef<AbortController | null>(null); const runSearch = useCallback(async (q: string) => { + abortRef.current?.abort(); + const controller = new AbortController(); + abortRef.current = controller; setLoading(true); try { - const response = await searchGifs(q || undefined, 24, 0); + const response = await searchGifs(q || undefined, 24, 0, controller.signal); setGifs(response.Data ?? []); } catch (error) { + if (controller.signal.aborted) return; logger.debug({ message: 'chat: gif search failed', context: { error } }); setGifs([]); } finally { - setLoading(false); + if (!controller.signal.aborted) setLoading(false); } }, []); + useEffect( + () => () => { + if (debounceRef.current) clearTimeout(debounceRef.current); + abortRef.current?.abort(); + }, + [] + ); + useEffect(() => { if (!isOpen) return; runSearch(''); }, [isOpen, runSearch]);🤖 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 `@src/components/chat/gif-picker-sheet.tsx` around lines 30 - 54, Update runSearch to create and pass an AbortSignal to searchGifs, aborting the previous request before starting a new one and ignoring expected abort failures so stale results cannot update gifs. In the useEffect cleanup for isOpen, clear debounceRef and abort any active request so no search runs after unmount or closure.src/components/chat/chat-utils.ts-99-102 (1)
99-102: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
hasLinkreturns alternating results becauseURL_REGEXis global.
URL_REGEXhas thegflag.RegExp.prototype.testadvanceslastIndexon a match and keeps it across calls. A second call with the same body starts the search after the previous match and can returnfalse. Use a separate non-global regex for the predicate.🐛 Proposed fix
const URL_REGEX = /(https?:\/\/[^\s]+)/g; +const URL_TEST_REGEX = /https?:\/\/[^\s]+/; export interface TextSegment {export function hasLink(body?: string | null): boolean { if (!body) return false; - return URL_REGEX.test(body); + return URL_TEST_REGEX.test(body); }🤖 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 `@src/components/chat/chat-utils.ts` around lines 99 - 102, Update hasLink to use a non-global regular expression for its predicate check instead of the shared global URL_REGEX, ensuring repeated calls with the same body return consistent results. Preserve the existing false result for empty or null bodies.src/components/chat/message-composer.tsx-43-54 (1)
43-54: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winSend the typing signal once, and stop it automatically.
handleChangecallsonTyping(true)on every keystroke. Insrc/app/chat/[channelId].tsxthat prop callsuseChatStore.getState().sendTyping(channelId, isTyping), so each character produces one realtime call.typingActiveis set but never used to suppress repeats. The component also never stops typing on unmount, so other users keep seeing the indicator after the screen closes.♻️ Proposed fix
const typingActive = useRef(false); + const typingTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); const stopTyping = useCallback(() => { + if (typingTimeoutRef.current) { + clearTimeout(typingTimeoutRef.current); + typingTimeoutRef.current = null; + } if (typingActive.current) { typingActive.current = false; onTyping(false); } }, [onTyping]); + useEffect(() => stopTyping, [stopTyping]); + const handleChange = useCallback( (value: string) => { setText(value); if (value.length > 0) { - typingActive.current = true; - onTyping(true); + if (!typingActive.current) { + typingActive.current = true; + onTyping(true); + } + if (typingTimeoutRef.current) clearTimeout(typingTimeoutRef.current); + typingTimeoutRef.current = setTimeout(stopTyping, 3000); } else { stopTyping(); } }, [onTyping, stopTyping] );🤖 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 `@src/components/chat/message-composer.tsx` around lines 43 - 54, Update handleChange to call onTyping(true) only when typingActive.current is false, then set the flag after sending; retain stopTyping for empty input and ensure cleanup invokes stopTyping on component unmount so the typing indicator is cleared.
🟡 Minor comments (8)
src/translations/en.json-394-394 (1)
394-394: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle the
thread_repliessingular case.
message-bubble.tsxpassesmessage.ThreadReplyCountdirectly tot('chat.thread_replies', { count: message.ThreadReplyCount }). If one reply exists, this renders1 replies. Split this into a singular label for one reply and a plural label for multiple replies.🤖 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 `@src/translations/en.json` at line 394, Update the chat translation and its usage in message-bubble.tsx so a ThreadReplyCount of 1 renders a singular reply label, while counts greater than 1 use the plural label. Adjust the thread_replies translation key or add the necessary singular key, and select the appropriate key where t receives message.ThreadReplyCount.src/services/push-notification.ts-32-34 (1)
32-34: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winConstrain and pass
channelIdas a route parameter.
handleChatDeepLinkcurrently builds/chat/${channelId}and the parser accepts any remainder after the prefix. Add a chat event-code contract, validate the channel ID before navigation, and callrouter.push({ pathname: '/chat/[channelId]', params: { channelId } })so values such as spaces or/are handled as one parameter instead of malformed segments.🤖 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 `@src/services/push-notification.ts` around lines 32 - 34, Update handleChatDeepLink to enforce the chat event-code contract and validate the parsed channelId before navigation, rejecting values that do not meet the allowed channel-ID constraints. Replace the interpolated /chat/${channelId} navigation with router.push using pathname '/chat/[channelId]' and params containing channelId, while preserving the existing handling for invalid deep links.src/stores/chat/store.ts-1-22 (1)
1-22: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe
testcheck fails on import and export ordering in two files. The repository sort rule is not applied to the new chat modules. One lint autofix run resolves both sites.
src/stores/chat/store.ts#L1-L22: run the lint autofix to sort the import statements, including the named type imports inside the@/models/v4/chatblock, whereChatMemberResultDataandChatMentionInputare placed afterChatMessageType.src/models/v4/chat/index.ts#L1-L6: run the lint autofix to sort the barrel exports into ASCII order:chatEnums,chatEvents,chatInputs,chatModels,chatbotModels,outbox.🤖 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 `@src/stores/chat/store.ts` around lines 1 - 22, Apply the repository’s lint autofix to both affected sites: reorder imports in src/stores/chat/store.ts, including the named imports from `@/models/v4/chat`, and reorder barrel exports in src/models/v4/chat/index.ts into ASCII order: chatEnums, chatEvents, chatInputs, chatModels, chatbotModels, outbox.Source: Linters/SAST tools
src/components/chat/chat-utils.ts-40-44 (1)
40-44: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTranslate the channel display fallbacks.
'Direct Message'and'Channel'render in the channel list and in the conversation header. Both strings bypassreact-i18next. Pass a translator or return a translation key so callers resolve the label witht().♻️ Proposed change
-export function getChannelDisplayName(channel: ChatChannelResultData): string { +export function getChannelDisplayNameKey(channel: ChatChannelResultData): { name?: string; key?: string } { if (channel.Name && channel.Name.trim().length > 0) return channel.Name; - if (channel.ChannelType === ChatChannelType.DirectMessage) return 'Direct Message'; - return 'Channel'; + if (channel.ChannelType === ChatChannelType.DirectMessage) return { key: 'chat.direct_message' }; + return { key: 'chat.channel' }; }As per coding guidelines: "Ensure all text is wrapped in
t()fromreact-i18nextfor translations with the dictionary files stored insrc/translations".🤖 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 `@src/components/chat/chat-utils.ts` around lines 40 - 44, Update getChannelDisplayName to avoid returning hardcoded fallback labels for DirectMessage and generic channels; either accept and use a react-i18next translator or return translation keys that both the channel list and conversation header resolve through t(). Preserve the existing named-channel behavior and ensure both fallback strings are sourced from the translation dictionaries.Source: Coding guidelines
src/components/chat/message-bubble.tsx-88-100 (1)
88-100: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winValidate the location metadata before you build the map URL.
parseLocationMetadatacasts unvalidated JSON toChatLocationMetadata.MetadataJsonarrives from other users through the API, soLatitudeandLongitudecan hold arbitrary strings. The values are interpolated into the URL without encoding. Check that both values are finite numbers, then encode them.🛡️ Proposed fix
if (message.MessageType === ChatMessageType.Location) { const loc = parseLocationMetadata(message.MetadataJson); - if (loc) { + const latitude = Number(loc?.Latitude); + const longitude = Number(loc?.Longitude); + if (loc && Number.isFinite(latitude) && Number.isFinite(longitude)) { return ( - <Pressable onPress={() => Linking.openURL(`https://maps.google.com/?q=${loc.Latitude},${loc.Longitude}`)}> + <Pressable onPress={() => Linking.openURL(`https://maps.google.com/?q=${encodeURIComponent(`${latitude},${longitude}`)}`)}>🤖 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 `@src/components/chat/message-bubble.tsx` around lines 88 - 100, Update the location branch in the message rendering logic around parseLocationMetadata to accept metadata only when Latitude and Longitude are finite numbers, rejecting invalid values before rendering the Pressable. When constructing the Google Maps URL in the Pressable handler, encode both validated coordinate values before interpolation.src/components/chat/new-conversation-sheet.tsx-56-59 (1)
56-59: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReport the recipient load failure to the user.
A failed
getRecipientscall only writes to the log.recipientsstays empty, so the sheet showschat.no_people. The user cannot tell an empty roster from a network error. Use the same toast pattern asstartDirectMessage.♻️ Proposed fix
getRecipients(true, false) .then((result) => setRecipients((result.Data ?? []).filter(isPersonRecipient))) - .catch((error) => logger.error({ message: 'chat: failed to load recipients', context: { error } })) + .catch((error) => { + logger.error({ message: 'chat: failed to load recipients', context: { error } }); + useToastStore.getState().showToast('error', t('chat.load_people_failed')); + }) .finally(() => setLoading(false)); - }, [isOpen]); + }, [isOpen, t]);Add the
chat.load_people_failedkey tosrc/translations/en.json.As per coding guidelines: "Handle errors gracefully and provide user feedback".
🤖 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 `@src/components/chat/new-conversation-sheet.tsx` around lines 56 - 59, Update the getRecipients failure handler in the new-conversation sheet to retain the existing logging and show the user an error toast using the same toast pattern as startDirectMessage, with the chat.load_people_failed translation key. Add that key to the English translations.Source: Coding guidelines
src/app/chat/[channelId].tsx-103-106 (1)
103-106: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpired typing entries stay visible until the next store update.
typingNamesreadsDate.now()when the memo runs. The memo depends ontypingandtonly. When a typing entry expires and no further store update arrives, the indicator keeps rendering. Add a periodic tick so the memo re-evaluates.♻️ Proposed fix
+ const [typingTick, setTypingTick] = useState(0); + + useEffect(() => { + if (!typing || typing.length === 0) return; + const id = setInterval(() => setTypingTick((prev) => prev + 1), 1000); + return () => clearInterval(id); + }, [typing]); + const typingNames = useMemo(() => { const now = Date.now(); return (typing ?? []).filter((u) => u.expiresAt > now).map((u) => u.displayName || t('chat.someone')); - }, [typing, t]); + }, [typing, t, typingTick]);🤖 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 `@src/app/chat/`[channelId].tsx around lines 103 - 106, Update the typing indicator logic around typingNames to include a periodic state tick that changes while the component is mounted, causing the useMemo expiration filter to re-evaluate even without typing store updates. Add the tick dependency to typingNames and clean up the interval on unmount, preserving the existing display-name fallback and expiry filtering.src/components/chat/message-actions-sheet.tsx-141-151 (1)
141-151: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHide pin and unpin for deleted messages.
The moderator-delete item at line 160 checks
!isDeleted. The pin item does not. A moderator can pin a deleted message, andMessageBubblethen renders a pin icon next to the "message deleted" placeholder.🐛 Proposed fix
- {isModerator ? ( + {isModerator && !isDeleted ? ( <ActionsheetItem onPress={() => { onTogglePin(message, !isPinned);🤖 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 `@src/components/chat/message-actions-sheet.tsx` around lines 141 - 151, Update the moderator pin/unpin action in the message-actions rendering to require both isModerator and !isDeleted, matching the existing moderator-delete visibility condition. Keep the current pin toggle behavior unchanged for non-deleted messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1352f0be-6410-41f7-bd64-c5ca7bb6d830
📒 Files selected for processing (31)
env.jspnpm-workspace.yamlsrc/api/chat/chat.tssrc/api/chat/chatbot.tssrc/app/(app)/_layout.tsxsrc/app/(app)/chat.tsxsrc/app/(app)/chatbot.tsxsrc/app/chat/[channelId].tsxsrc/app/chat/thread/[messageId].tsxsrc/components/chat/ack-banner.tsxsrc/components/chat/chat-utils.tssrc/components/chat/gif-picker-sheet.tsxsrc/components/chat/message-actions-sheet.tsxsrc/components/chat/message-bubble.tsxsrc/components/chat/message-composer.tsxsrc/components/chat/new-conversation-sheet.tsxsrc/components/chat/typing-indicator.tsxsrc/components/sidebar/side-menu.tsxsrc/hooks/use-signalr-lifecycle.tssrc/models/v4/chat/chatEnums.tssrc/models/v4/chat/chatEvents.tssrc/models/v4/chat/chatInputs.tssrc/models/v4/chat/chatModels.tssrc/models/v4/chat/chatbotModels.tssrc/models/v4/chat/index.tssrc/models/v4/chat/outbox.tssrc/services/push-notification.tssrc/services/signalr.service.tssrc/stores/chat/store.tssrc/stores/signalr/signalr-store.tssrc/translations/en.json
| @@ -0,0 +1,195 @@ | |||
| import { type Href, Stack, useFocusEffect, useRouter } from 'expo-router'; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win
Run the lint autofix. The import order breaks the pipeline.
yarn check-all fails with simple-import-sort/imports on every file listed below. The shared root cause is that the new files were not passed through the ESLint autofix. Run yarn lint --fix.
src/app/(app)/chat.tsx#L1-L1: sort the import block;expo-router,lucide-react-native,react,react-i18next, andreact-nativemust precede the@/group in the configured order.src/app/(app)/chatbot.tsx#L1-L1: moveimport useAuthStore from '@/stores/auth/store';beforeimport { useChatStore } from '@/stores/chat/store';.src/app/chat/[channelId].tsx#L1-L1: moveimport { Image } from 'expo-image';into the external group beforereact-native, moveKeyboardAvoidingViewaboveSpinner, and moveuseAuthStoreaboveuseChatStore.src/app/chat/thread/[messageId].tsx#L1-L1: moveimport useAuthStore from '@/stores/auth/store';beforeimport { useChatStore } from '@/stores/chat/store';.src/components/chat/chat-utils.ts#L1-L1: sort the two@/imports.src/components/chat/message-bubble.tsx#L1-L1: sort the import block.src/components/chat/message-actions-sheet.tsx#L1-L1: sort the import block.
🧰 Tools
🪛 GitHub Actions: React Native CI/CD / 3_test.txt
[error] 1-1: ESLint: Imports are not sorted. Run autofix to sort these imports. (simple-import-sort/imports). The 'yarn check-all' command failed during 'yarn run lint'.
🪛 GitHub Actions: React Native CI/CD / test
[error] 1-1: ESLint: Imports are not sorted. Run autofix to sort these imports. (simple-import-sort/imports) Command: yarn check-all
🪛 GitHub Check: test
[failure] 1-1:
Run autofix to sort these imports!
📍 Affects 7 files
src/app/(app)/chat.tsx#L1-L1(this comment)src/app/(app)/chatbot.tsx#L1-L1src/app/chat/[channelId].tsx#L1-L1src/app/chat/thread/[messageId].tsx#L1-L1src/components/chat/chat-utils.ts#L1-L1src/components/chat/message-bubble.tsx#L1-L1src/components/chat/message-actions-sheet.tsx#L1-L1
🤖 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 `@src/app/`(app)/chat.tsx at line 1, Run the ESLint autofix to sort imports in
src/app/(app)/chat.tsx (line 1), src/app/(app)/chatbot.tsx (line 1),
src/app/chat/[channelId].tsx (line 1), src/app/chat/thread/[messageId].tsx (line
1), src/components/chat/chat-utils.ts (line 1),
src/components/chat/message-bubble.tsx (line 1), and
src/components/chat/message-actions-sheet.tsx (line 1). Ensure external imports
precede the `@/` group, auth-store imports precede chat-store imports, and the
specified named imports follow the configured order.
Sources: Linters/SAST tools, Pipeline failures
| * channel over SignalR (chatbotMessageReceived). Idempotent via clientMessageId. | ||
| */ | ||
| export const sendChatbotMessage = async (text: string, clientMessageId: string) => { | ||
| const response = await api.post<ChatbotSendResponse>(`${CHATBOT}/SendChatMessage`, { |
There was a problem hiding this comment.
Unhandled external HTTP POST call in chatbot.ts violates team rules requiring network calls to be wrapped in try/catch. Wrap the call in a try/catch block, add structured context like op and clientMessageId, and map the error to a domain-level exception.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/api/chat/chatbot.ts:
Line 18:
Unhandled external HTTP POST call in `chatbot.ts` violates team rules requiring network calls to be wrapped in try/catch. Wrap the call in a try/catch block, add structured context like `op` and `clientMessageId`, and map the error to a domain-level exception.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const CHATBOT = '/Chatbot'; | ||
|
|
||
| /** Gets (creating if needed) the caller's chatbot conversation channel. */ | ||
| export const getChatbotChannel = async (signal?: AbortSignal) => { |
There was a problem hiding this comment.
Missing JSDoc on getChatbotChannel omits Promise return type and rejection conditions, violating documentation rules. Add @returns {Promise<ChatbotChannelResponse>} and document network/auth rejection conditions.
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File src/api/chat/chatbot.ts:
Line 8:
Missing JSDoc on `getChatbotChannel` omits Promise return type and rejection conditions, violating documentation rules. Add `@returns {Promise<ChatbotChannelResponse>}` and document network/auth rejection conditions.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <VStack className="mb-2"> | ||
| <Text className="px-4 pb-1 pt-3 text-xs font-semibold uppercase text-typography-400">{title}</Text> | ||
| {channels.map((channel) => ( | ||
| <ChannelRow key={channel.ChatChannelId} channel={channel} onPress={() => onOpen(channel.ChatChannelId)} /> |
There was a problem hiding this comment.
Inline arrow function in the onPress JSX prop creates a new function on every render, degrading performance. Move the function definition outside the render method.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/(app)/chat.tsx:
Line 74:
Inline arrow function in the `onPress` JSX prop creates a new function on every render, degrading performance. Move the function definition outside the render method.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const isLoading = useChatStore((s) => s.isLoadingChannels); | ||
| const pendingAcks = useChatStore((s) => s.pendingAcks); | ||
| const [fabOpen, setFabOpen] = useState(false); | ||
| const [newMode, setNewMode] = useState<'dm' | 'group' | null>(null); |
There was a problem hiding this comment.
Duplicated TypeScript literal types and runtime string values for conversation modes risk structural drift. Declare const CONVERSATION_MODES = ['dm', 'group'] as const; and derive the type using typeof CONVERSATION_MODES[number].
Kody rule violation: Derive TypeScript types from validation schemas
Prompt for LLM
File src/app/(app)/chat.tsx:
Line 87:
Duplicated TypeScript literal types and runtime string values for conversation modes risk structural drift. Declare `const CONVERSATION_MODES = ['dm', 'group'] as const;` and derive the type using `typeof CONVERSATION_MODES[number]`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| useFocusEffect( | ||
| useCallback(() => { | ||
| const store = useChatStore.getState(); | ||
| void store.initChatbot(); |
There was a problem hiding this comment.
Unhandled promise rejection occurs because initChatbot() is fired with void and lacks error handling, which can crash the app or hide bugs. Append .catch(err => logger.error('initChatbot failed', { err })) or wrap the await in a try/catch block.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/app/(app)/chatbot.tsx:
Line 34:
Unhandled promise rejection occurs because `initChatbot()` is fired with `void` and lacks error handling, which can crash the app or hide bugs. Append `.catch(err => logger.error('initChatbot failed', { err }))` or wrap the await in a try/catch block.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| import { useChatStore } from '@/stores/chat/store'; | ||
| import useAuthStore from '@/stores/auth/store'; | ||
|
|
||
| export default function ChatbotScreen() { |
There was a problem hiding this comment.
Default export reduces maintainability and increases refactoring risks. Change to export function ChatbotScreen() or add a named export alongside the default export if strictly required by expo-router.
Kody rule violation: Avoid default exports
Prompt for LLM
File src/app/(app)/chatbot.tsx:
Line 23:
Default export reduces maintainability and increases refactoring risks. Change to `export function ChatbotScreen()` or add a named export alongside the default export if strictly required by `expo-router`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| {imageUri ? ( | ||
| <Center className="w-full p-2"> | ||
| <Image source={{ uri: imageUri }} style={{ width: '100%', height: 400, borderRadius: 12 }} contentFit="contain" /> | ||
| </Center> |
There was a problem hiding this comment.
Missing Authorization header causes the full-screen image preview to silently fail when loading server-hosted chat attachments. Pass the auth header in the preview source using getChatAttachmentImageSource(imageUri) or store both the URI and headers from the original tap.
{imageUri ? (
<Center className="w-full p-2">
<Image source={getChatAttachmentImageSource(imageUri)} style={{ width: '100%', height: 400, borderRadius: 12 }} contentFit="contain" />
</Center>
) : null}Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 302 to 305:
Missing Authorization header causes the full-screen image preview to silently fail when loading server-hosted chat attachments. Pass the auth header in the preview source using `getChatAttachmentImageSource(imageUri)` or store both the URI and headers from the original tap.
Suggested Code:
{imageUri ? (
<Center className="w-full p-2">
<Image source={getChatAttachmentImageSource(imageUri)} style={{ width: '100%', height: 400, borderRadius: 12 }} contentFit="contain" />
</Center>
) : null}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| title, | ||
| headerShown: true, | ||
| headerBackTitle: '', | ||
| headerRight: () => (isDm ? <Circle size={12} color={otherOnline ? '#22c55e' : '#9ca3af'} fill={otherOnline ? '#22c55e' : '#9ca3af'} /> : undefined), |
There was a problem hiding this comment.
Duplicated hard-coded magic values for UI constants prevent theming and single-source adjustments. Extract hex colors #22c55e and #9ca3af, and icon size 12, into named constants like ONLINE_COLOR, OFFLINE_COLOR, and STATUS_ICON_SIZE.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 209:
Duplicated hard-coded magic values for UI constants prevent theming and single-source adjustments. Extract hex colors `#22c55e` and `#9ca3af`, and icon size `12`, into named constants like `ONLINE_COLOR`, `OFFLINE_COLOR`, and `STATUS_ICON_SIZE`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } catch { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Swallowed parse exception in the catch block hides operational failures by returning null without logging. Add a structured log like logger.warn('parseMetadata failed', { err }) before returning null.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/components/chat/chat-utils.ts:
Line 59 to 61:
Swallowed parse exception in the catch block hides operational failures by returning `null` without logging. Add a structured log like `logger.warn('parseMetadata failed', { err })` before returning `null`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| export function parseMetadata<T>(metadataJson?: string | null): T | null { | ||
| if (!metadataJson) return null; | ||
| try { | ||
| return JSON.parse(metadataJson) as T; |
There was a problem hiding this comment.
Unvalidated JSON payload parsed via JSON.parse and cast with as T risks runtime errors and vulnerabilities from structural mismatches. Validate the parsed structure using a schema validator like zod or explicit field checks before returning.
Kody rule violation: Always validate JSON parsing
Prompt for LLM
File src/components/chat/chat-utils.ts:
Line 58:
Unvalidated JSON payload parsed via `JSON.parse` and cast with `as T` risks runtime errors and vulnerabilities from structural mismatches. Validate the parsed structure using a schema validator like `zod` or explicit field checks before returning.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const handleChange = (value: string) => { | ||
| setQuery(value); | ||
| if (debounceRef.current) clearTimeout(debounceRef.current); | ||
| debounceRef.current = setTimeout(() => runSearch(value), 400); |
There was a problem hiding this comment.
Uncleared setTimeout in handleChange causes wasted network calls and a setState-after-unmount error if the sheet unmounts during debounce. Add a useEffect cleanup hook to call clearTimeout(debounceRef.current) on teardown.
Kody rule violation: Clear timers on teardown/unmount
Prompt for LLM
File src/components/chat/gif-picker-sheet.tsx:
Line 53:
Uncleared `setTimeout` in `handleChange` causes wasted network calls and a `setState`-after-unmount error if the sheet unmounts during debounce. Add a `useEffect` cleanup hook to call `clearTimeout(debounceRef.current)` on teardown.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <Text className={textTone}> | ||
| {segments.map((segment, index) => | ||
| segment.isLink ? ( | ||
| <Text key={index} className={`underline ${isOwn ? 'text-white' : 'text-primary-600'}`} onPress={() => Linking.openURL(segment.text)}> |
There was a problem hiding this comment.
Array index used as a React list key in message-bubble.tsx causes component reordering issues and unexpected behavior. Replace the index with a unique identifier.
Kody rule violation: Avoid array indexes as keys in React lists
Prompt for LLM
File src/components/chat/message-bubble.tsx:
Line 109:
Array index used as a React list key in `message-bubble.tsx` causes component reordering issues and unexpected behavior. Replace the index with a unique identifier.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
|
|
||
| if (message.MessageType === ChatMessageType.Image) { | ||
| const attachment = message.Attachments[0]; |
There was a problem hiding this comment.
Unguarded array index access on message.Attachments causes unexpected failures when the array is empty. Guard the access with a length check or use a safe accessor before assigning to attachment.
Kody rule violation: Check query results before accessing indices
Prompt for LLM
File src/components/chat/message-bubble.tsx:
Line 68:
Unguarded array index access on `message.Attachments` causes unexpected failures when the array is empty. Guard the access with a length check or use a safe accessor before assigning to `attachment`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| key={emoji} | ||
| className="p-2" | ||
| onPress={() => { | ||
| setText((prev) => prev + emoji); |
There was a problem hiding this comment.
String concatenation using the + operator reduces readability and risks implicit type coercion bugs. Replace it with a template literal: setText((prev) => \${prev}${emoji}`)`.
Kody rule violation: Use Template Literals Instead of String Concatenation
Prompt for LLM
File src/components/chat/message-composer.tsx:
Line 154:
String concatenation using the `+` operator reduces readability and risks implicit type coercion bugs. Replace it with a template literal: `setText((prev) => \`${prev}${emoji}\`)`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const filtered = useMemo(() => { | ||
| const q = query.trim().toLowerCase(); | ||
| if (!q) return recipients; | ||
| return recipients.filter((r) => r.Name.toLowerCase().includes(q)); |
There was a problem hiding this comment.
Null pointer dereference occurs when calling .toLowerCase() on an unguarded r.Name property, throwing a TypeError. Use optional chaining with a fallback like (r.Name ?? '').toLowerCase().includes(q).
Kody rule violation: Add null checks before accessing properties
Prompt for LLM
File src/components/chat/new-conversation-sheet.tsx:
Line 65:
Null pointer dereference occurs when calling `.toLowerCase()` on an unguarded `r.Name` property, throwing a `TypeError`. Use optional chaining with a fallback like `(r.Name ?? '').toLowerCase().includes(q)`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| interface NewConversationSheetProps { | ||
| isOpen: boolean; | ||
| onClose: () => void; | ||
| mode: 'dm' | 'group'; |
There was a problem hiding this comment.
Inline string literals defining the mode prop type lack a single source of truth and allow runtime typos in comparisons. Define a const record like const ConversationMode = { Dm: 'dm', Group: 'group' } as const; and derive the type from it.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/components/chat/new-conversation-sheet.tsx:
Line 27:
Inline string literals defining the `mode` prop type lack a single source of truth and allow runtime typos in comparisons. Define a const record like `const ConversationMode = { Dm: 'dm', Group: 'group' } as const;` and derive the type from it.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
|
|
||
| export interface ChatbotSessionResponse { | ||
| success: boolean; |
There was a problem hiding this comment.
Casing inconsistency on the success property violates team rules requiring PascalCase for response models. Rename the property to Success to match sibling properties like ChatChannelId and LastMessageSeq.
Kody rule violation: Use proper naming conventions
Prompt for LLM
File src/models/v4/chat/chatbotModels.ts:
Line 20:
Casing inconsistency on the `success` property violates team rules requiring PascalCase for response models. Rename the property to `Success` to match sibling properties like `ChatChannelId` and `LastMessageSeq`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| }, | ||
|
|
||
| reset: () => { |
There was a problem hiding this comment.
Cross-session data leak occurs because reset() never clears the MMKV-persisted outbox and is not invoked from auth.logout(), causing handleChatConnected() to trigger drainOutbox() and expose user A's private message content to user B. Add outbox: [] to the reset() state update and call useChatStore.getState().reset() from the auth store's logout() method.
reset: () => {
typingTimers.forEach((t) => clearTimeout(t));
typingTimers.clear();
lastTypingSentAt.clear();
lastMarkedSeq.clear();
set({
channels: [],
messagesByChannel: {},
membersByChannel: {},
typingByChannel: {},
presence: new Set<string>(),
pendingAcks: [],
activeChannelId: null,
chatbotChannelId: null,
chatbotTyping: false,
hasMoreByChannel: {},
loadingMessagesByChannel: {},
outbox: [],
});
},Prompt for LLM
File src/stores/chat/store.ts:
Line 726:
Cross-session data leak occurs because `reset()` never clears the MMKV-persisted outbox and is not invoked from `auth.logout()`, causing `handleChatConnected()` to trigger `drainOutbox()` and expose user A's private message content to user B. Add `outbox: []` to the `reset()` state update and call `useChatStore.getState().reset()` from the auth store's `logout()` method.
Suggested Code:
reset: () => {
typingTimers.forEach((t) => clearTimeout(t));
typingTimers.clear();
lastTypingSentAt.clear();
lastMarkedSeq.clear();
set({
channels: [],
messagesByChannel: {},
membersByChannel: {},
typingByChannel: {},
presence: new Set<string>(),
pendingAcks: [],
activeChannelId: null,
chatbotChannelId: null,
chatbotTyping: false,
hasMoreByChannel: {},
loadingMessagesByChannel: {},
outbox: [],
});
},
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| drainOutbox: async () => { | ||
| const items = [...get().outbox]; | ||
| for (const item of items) { | ||
| await sendOutboxItem(item, set, get); |
There was a problem hiding this comment.
N+1 API call pattern occurs because chatApi.sendMessage is triggered sequentially via sendOutboxItem inside a for-loop. Refactor to use a bulk send endpoint or Promise.allSettled to parallelize the independent network requests.
Kody rule violation: Detect N+1 style queries and suggest batching
Prompt for LLM
File src/stores/chat/store.ts:
Line 361:
N+1 API call pattern occurs because `chatApi.sendMessage` is triggered sequentially via `sendOutboxItem` inside a for-loop. Refactor to use a bulk send endpoint or `Promise.allSettled` to parallelize the independent network requests.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const incoming = response.Data ?? []; | ||
| set((state) => { | ||
| let list = state.messagesByChannel[channelId] ?? []; | ||
| for (const m of incoming) list = upsertMessage(list, { ...m, _localStatus: 'sent' }); |
There was a problem hiding this comment.
Duplicated upsert-and-mark-sent loop logic in loadOlderMessages and loadNewerMessages increases maintenance burden and divergence risk. Extract the loop into a shared helper function like mergeIncoming(list, incoming).
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File src/stores/chat/store.ts:
Line 244:
Duplicated upsert-and-mark-sent loop logic in `loadOlderMessages` and `loadNewerMessages` increases maintenance burden and divergence risk. Extract the loop into a shared helper function like `mergeIncoming(list, incoming)`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| drainOutbox: async () => { | ||
| const items = [...get().outbox]; | ||
| for (const item of items) { | ||
| await sendOutboxItem(item, set, get); |
There was a problem hiding this comment.
Sequential await inside a for-loop in drainOutbox underutilizes concurrency and blocks subsequent items if one hangs. Replace the loop with await Promise.allSettled(items.map(item => sendOutboxItem(item, set, get))) to parallelize sends and inspect failures.
Kody rule violation: Use Promise.allSettled for batch operations with partial failures
Prompt for LLM
File src/stores/chat/store.ts:
Line 361:
Sequential await inside a for-loop in `drainOutbox` underutilizes concurrency and blocks subsequent items if one hangs. Replace the loop with `await Promise.allSettled(items.map(item => sendOutboxItem(item, set, get)))` to parallelize sends and inspect failures.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| useChatStore.getState().handleChatConnected(); | ||
| }; | ||
| chatHubHandlers.onChatConnected = onChatConnected; | ||
| signalRService.on('onChatConnected', onChatConnected); |
There was a problem hiding this comment.
Raw string literal 'onChatConnected' duplicates an entry in CHAT_HUB_METHODS, risking silent drift during refactoring. Extract 'onChatConnected' into a shared named constant or reference its element directly from CHAT_HUB_METHODS.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 674:
Raw string literal `'onChatConnected'` duplicates an entry in `CHAT_HUB_METHODS`, risking silent drift during refactoring. Extract `'onChatConnected'` into a shared named constant or reference its element directly from `CHAT_HUB_METHODS`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/components/chat/chat-utils.ts (3)
78-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep sentence punctuation out of link targets.
URL_REGEXstops only at whitespace. A sentence such ashttps://example.com.produces a link segment containing the period. Parentheses and commas have the same problem. Split trailing punctuation into a plain-text segment and add regression tests.Proposed fix
- segments.push({ text: match[0], isLink: true }); - lastIndex = index + match[0].length; + const rawUrl = match[0]; + const url = rawUrl.replace(/[.,!?;:)\]}]+$/, ''); + segments.push({ text: url, isLink: true }); + if (url.length < rawUrl.length) { + segments.push({ text: rawUrl.slice(url.length), isLink: false }); + } + lastIndex = index + rawUrl.length;🤖 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 `@src/components/chat/chat-utils.ts` around lines 78 - 105, Update URL_REGEX and linkifySegments so trailing sentence punctuation such as periods, commas, and parentheses is excluded from link segments and emitted as plain text instead. Preserve valid URL content and hasLink behavior, and add regression tests covering these punctuation cases.
107-123: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd a native clipboard implementation for chat copying.
copyToClipboard()only checksnavigator.clipboard.writeText, then returnsfalse. Add the repository’s native clipboard module for iOS/Android and keep the web path for web/Electron.🤖 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 `@src/components/chat/chat-utils.ts` around lines 107 - 123, Update copyToClipboard to use the repository’s native clipboard module on iOS and Android, while preserving the existing navigator.clipboard.writeText path for web and Electron. Return true only after either implementation succeeds, and retain the false fallback when neither clipboard implementation is available or an operation fails.Source: Coding guidelines
1-40: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftAdd Jest coverage for the missing utility paths.
chat-utils.test.tscoversgetChannelDisplayName,linkifySegments,hasLink, andgetImageMimeType, but it does not covergroupChannels,copyToClipboard, orformatShortTime. Add cases for archived filtering and group ordering; for clipboard writes and invalid dates, use Jest mocks and injected date behavior so these tests remain deterministic.🤖 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 `@src/components/chat/chat-utils.ts` around lines 1 - 40, Add Jest coverage in chat-utils.test.ts for groupChannels, copyToClipboard, and formatShortTime. Verify archived channels are excluded, channel types are assigned to the correct groups, and each group is ordered by most recent LastMessageOn; mock clipboard writes and inject deterministic dates to cover successful clipboard behavior and invalid-date handling without relying on real browser APIs or current time.Source: Coding guidelines
src/stores/chat/store.ts (2)
440-447: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winGive moderators failure feedback on
moderatorDeleteMessage.Every other mutating action in this file (
editMessage,deleteMessage,togglePin,flagMessage) shows a toast on failure.moderatorDeleteMessageonly logs the error and leaves the moderator without feedback that the delete failed. Add a toast call here to match the established pattern.🤖 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 `@src/stores/chat/store.ts` around lines 440 - 447, Update moderatorDeleteMessage to display an error toast in its catch block, matching the established failure-feedback pattern used by editMessage, deleteMessage, togglePin, and flagMessage, while retaining the existing logger.error call.
452-496: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReaction rollback can overwrite a concurrent realtime update.
addReactionandremoveReactionsnapshotpreviousReactionsbefore the optimistic update (Line 454, Line 478) and, on API failure, restore that exact snapshot viapatchMessage(set, channelId, messageId, { Reactions: previousReactions })(Line 471, Line 493). If ahandleReactionUpdatedevent for the same message arrives while thechatApi.addReaction/chatApi.removeReactioncall is pending, the rollback discards that newer server state and replaces it with the stale snapshot. Restore by removing only the locally-added or locally-removed reaction from the current state at rollback time, instead of replacing the whole array with the pre-optimistic snapshot.🤖 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 `@src/stores/chat/store.ts` around lines 452 - 496, Update the rollback logic in addReaction and removeReaction to inspect the message’s current Reactions when the API call fails, rather than restoring previousReactions wholesale. For addReaction, remove only the locally added user/emoji reaction; for removeReaction, re-add only the locally removed reaction if it is still absent, preserving any newer realtime reactions from handleReactionUpdated. Remove the stale snapshot-based patching while keeping the existing error logging and toast behavior.src/components/chat/new-conversation-sheet.tsx (1)
78-91: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard against a nullable
recipient.Namebefore calling.toLowerCase().Line 81 calls
r.Name.toLowerCase()without a null check. This was flagged in a previous review. If the recipients API can return an entry with noName, typing in the search field throws aTypeErrorand crashes the new-conversation flow. Use(r.Name ?? '').toLowerCase().includes(q)here, and the same guard whererecipient.Nameis rendered directly (Line 180, Line 184).#!/bin/bash # Description: Check RecipientsResultData.Name for nullability. rg -n -B2 -A15 'interface RecipientsResultData' src/models🛡️ Proposed fix
- return recipients.filter((r) => r.Name.toLowerCase().includes(q)); + return recipients.filter((r) => (r.Name ?? '').toLowerCase().includes(q));🤖 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 `@src/components/chat/new-conversation-sheet.tsx` around lines 78 - 91, Guard nullable recipient names in the `filtered` callback by using an empty-string fallback before `toLowerCase()`, and apply the same fallback wherever `recipient.Name` is rendered directly in the new-conversation sheet. Preserve the existing filtering and display behavior for non-null names.src/services/push-notification.ts (1)
236-239: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse the established capital
eventCodeprefixes before deep-linking.
handleChatDeepLinkonly accepts lowercaset:andg:, but the stored modal data uses uppercaseT:andG:for chat. This makes chat notification responses returnfalseand skip navigation, while non-chat codes also become no-ops. NormalizeeventCodehere, or update the chat helper to accept both conventions.🤖 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 `@src/services/push-notification.ts` around lines 236 - 239, Update the deep-link handling around handleChatDeepLink so stored chat event codes using uppercase T: and G: prefixes are accepted before navigation. Normalize data.eventCode at this call site or extend handleChatDeepLink to support both uppercase and lowercase prefixes, while preserving no-op behavior for non-chat event codes.
🧹 Nitpick comments (2)
src/components/chat/gif-picker-sheet.tsx (1)
94-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an accessibility label to each GIF selection button.
The
Pressablewrapping each GIFImage(Line 95-105) has noaccessibilityLabel. A screen reader user cannot distinguish between GIF options. Add a label, for examplegif.Titlewith a fallback string.♿ Proposed fix
<Pressable key={gif.Id} className="mb-2" style={{ width: '48%' }} + accessibilityLabel={gif.Title || t('chat.gif')} onPress={() => { onSelect(gif); onClose(); }} >As per coding guidelines, "Ensure the app is accessible, following WCAG guidelines for mobile applications."
🤖 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 `@src/components/chat/gif-picker-sheet.tsx` around lines 94 - 106, Update the GIF option Pressable in the gifs.map rendering to include an accessibilityLabel derived from gif.Title, with a fallback string when the title is missing, so screen readers can distinguish each selection button.Source: Coding guidelines
src/stores/chat/store.ts (1)
300-318: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd an iteration cap to the delta-sync loop in
loadNewerMessages.The
for (;;)loop keeps callingchatApi.getMessagesAfteras long as each page returns 200 items, with no maximum iteration count and no delay between requests. A large backlog, or a server that always returns exactly 200 items due to a paging bug, turns this into a tight request loop on reconnect. Add a maximum number of pages (or a total message cap) to bound worst-case behavior.🤖 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 `@src/stores/chat/store.ts` around lines 300 - 318, Bound the pagination loop in loadNewerMessages by enforcing a maximum number of pages or messages processed, while preserving the existing early returns for empty and short responses. Ensure the loop stops once the configured cap is reached, preventing unbounded getMessagesAfter requests when every page contains 200 items.
🤖 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 `@src/components/chat/chat-utils.ts`:
- Around line 1-4: Run the repository’s import-sort autofix and update the
imports in the chat utility module so the TFunction import follows the
configured simple-import-sort order alongside getAvatarUrl and the chat model
imports.
In `@src/lib/navigation.ts`:
- Around line 6-27: Validate RouterPushRetryOptions before entering the retry
loop in routerPushWithRetry: require maxAttempts to be a finite positive integer
and retryDelayMs to be finite and non-negative, rejecting invalid values
immediately. Preserve the existing defaults and retry behavior for valid
options, while ensuring maxAttempts = 0 cannot trigger a navigation attempt.
In `@src/stores/chat/store.ts`:
- Around line 393-416: Track in-flight ClientMessageId values at module scope
and have sendOutboxItem register and clear each ID with guaranteed cleanup
around the API request. Update drainOutbox to skip items whose ClientMessageId
is already in flight, while preserving existing retry and eligibility handling
so sendMessage and retryOutboxItem cannot issue duplicate sends.
In `@src/translations/de.json`:
- Line 1460: Update the chat.thread_replies translations to use i18next plural
variants: add the locale-appropriate plural keys in
src/translations/de.json:1460-1460, src/translations/es.json:1460-1460,
src/translations/fr.json:1460-1460, and src/translations/it.json:1460-1460; add
Arabic zero, one, two, few, many, and other forms in
src/translations/ar.json:1460-1460; and add few, many, and other forms in
src/translations/pl.json:1460-1460 and src/translations/uk.json:1460-1460.
Preserve the existing count interpolation and singular wording so a single reply
uses the singular form.
---
Outside diff comments:
In `@src/components/chat/chat-utils.ts`:
- Around line 78-105: Update URL_REGEX and linkifySegments so trailing sentence
punctuation such as periods, commas, and parentheses is excluded from link
segments and emitted as plain text instead. Preserve valid URL content and
hasLink behavior, and add regression tests covering these punctuation cases.
- Around line 107-123: Update copyToClipboard to use the repository’s native
clipboard module on iOS and Android, while preserving the existing
navigator.clipboard.writeText path for web and Electron. Return true only after
either implementation succeeds, and retain the false fallback when neither
clipboard implementation is available or an operation fails.
- Around line 1-40: Add Jest coverage in chat-utils.test.ts for groupChannels,
copyToClipboard, and formatShortTime. Verify archived channels are excluded,
channel types are assigned to the correct groups, and each group is ordered by
most recent LastMessageOn; mock clipboard writes and inject deterministic dates
to cover successful clipboard behavior and invalid-date handling without relying
on real browser APIs or current time.
In `@src/components/chat/new-conversation-sheet.tsx`:
- Around line 78-91: Guard nullable recipient names in the `filtered` callback
by using an empty-string fallback before `toLowerCase()`, and apply the same
fallback wherever `recipient.Name` is rendered directly in the new-conversation
sheet. Preserve the existing filtering and display behavior for non-null names.
In `@src/services/push-notification.ts`:
- Around line 236-239: Update the deep-link handling around handleChatDeepLink
so stored chat event codes using uppercase T: and G: prefixes are accepted
before navigation. Normalize data.eventCode at this call site or extend
handleChatDeepLink to support both uppercase and lowercase prefixes, while
preserving no-op behavior for non-chat event codes.
In `@src/stores/chat/store.ts`:
- Around line 440-447: Update moderatorDeleteMessage to display an error toast
in its catch block, matching the established failure-feedback pattern used by
editMessage, deleteMessage, togglePin, and flagMessage, while retaining the
existing logger.error call.
- Around line 452-496: Update the rollback logic in addReaction and
removeReaction to inspect the message’s current Reactions when the API call
fails, rather than restoring previousReactions wholesale. For addReaction,
remove only the locally added user/emoji reaction; for removeReaction, re-add
only the locally removed reaction if it is still absent, preserving any newer
realtime reactions from handleReactionUpdated. Remove the stale snapshot-based
patching while keeping the existing error logging and toast behavior.
---
Nitpick comments:
In `@src/components/chat/gif-picker-sheet.tsx`:
- Around line 94-106: Update the GIF option Pressable in the gifs.map rendering
to include an accessibilityLabel derived from gif.Title, with a fallback string
when the title is missing, so screen readers can distinguish each selection
button.
In `@src/stores/chat/store.ts`:
- Around line 300-318: Bound the pagination loop in loadNewerMessages by
enforcing a maximum number of pages or messages processed, while preserving the
existing early returns for empty and short responses. Ensure the loop stops once
the configured cap is reached, preventing unbounded getMessagesAfter requests
when every page contains 200 items.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: adb1b67a-8f5c-4fb7-b017-184b64cd5102
📒 Files selected for processing (21)
src/api/chat/chat.tssrc/app/(app)/chat.tsxsrc/app/chat/[channelId].tsxsrc/components/chat/__tests__/chat-utils.test.tssrc/components/chat/chat-utils.tssrc/components/chat/gif-picker-sheet.tsxsrc/components/chat/message-composer.tsxsrc/components/chat/new-conversation-sheet.tsxsrc/lib/navigation.tssrc/models/v4/chat/outbox.tssrc/services/push-notification.tssrc/stores/chat/store.tssrc/translations/ar.jsonsrc/translations/de.jsonsrc/translations/en.jsonsrc/translations/es.jsonsrc/translations/fr.jsonsrc/translations/it.jsonsrc/translations/pl.jsonsrc/translations/sv.jsonsrc/translations/uk.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/models/v4/chat/outbox.ts
- src/translations/en.json
- src/app/(app)/chat.tsx
- src/app/chat/[channelId].tsx
| export interface RouterPushRetryOptions { | ||
| maxAttempts?: number; | ||
| retryDelayMs?: number; | ||
| } | ||
|
|
||
| /** | ||
| * Pushes an expo-router href, retrying when the router has not mounted yet | ||
| * (cold-start deep links). Throws the last error once every attempt fails. | ||
| */ | ||
| export const routerPushWithRetry = async (href: Href, options?: RouterPushRetryOptions): Promise<void> => { | ||
| const maxAttempts = options?.maxAttempts ?? 1; | ||
| const retryDelayMs = options?.retryDelayMs ?? 250; | ||
|
|
||
| for (let attempt = 1; ; attempt++) { | ||
| try { | ||
| router.push(href); | ||
| return; | ||
| } catch (error) { | ||
| if (attempt >= maxAttempts) { | ||
| throw error; | ||
| } | ||
| await new Promise((resolve) => setTimeout(resolve, retryDelayMs)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject invalid retry options before entering the retry loop.
maxAttempts is not validated. If it is NaN or Infinity, attempt >= maxAttempts never becomes true when navigation keeps failing, so the promise retries indefinitely. maxAttempts = 0 also performs one attempt. Validate a finite positive integer and a finite non-negative retryDelayMs.
Proposed guard
const maxAttempts = options?.maxAttempts ?? 1;
const retryDelayMs = options?.retryDelayMs ?? 250;
+ if (!Number.isInteger(maxAttempts) || maxAttempts < 1) {
+ throw new RangeError('maxAttempts must be a positive integer');
+ }
+ if (!Number.isFinite(retryDelayMs) || retryDelayMs < 0) {
+ throw new RangeError('retryDelayMs must be a finite non-negative number');
+ }📝 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.
| export interface RouterPushRetryOptions { | |
| maxAttempts?: number; | |
| retryDelayMs?: number; | |
| } | |
| /** | |
| * Pushes an expo-router href, retrying when the router has not mounted yet | |
| * (cold-start deep links). Throws the last error once every attempt fails. | |
| */ | |
| export const routerPushWithRetry = async (href: Href, options?: RouterPushRetryOptions): Promise<void> => { | |
| const maxAttempts = options?.maxAttempts ?? 1; | |
| const retryDelayMs = options?.retryDelayMs ?? 250; | |
| for (let attempt = 1; ; attempt++) { | |
| try { | |
| router.push(href); | |
| return; | |
| } catch (error) { | |
| if (attempt >= maxAttempts) { | |
| throw error; | |
| } | |
| await new Promise((resolve) => setTimeout(resolve, retryDelayMs)); | |
| export interface RouterPushRetryOptions { | |
| maxAttempts?: number; | |
| retryDelayMs?: number; | |
| } | |
| /** | |
| * Pushes an expo-router href, retrying when the router has not mounted yet | |
| * (cold-start deep links). Throws the last error once every attempt fails. | |
| */ | |
| export const routerPushWithRetry = async (href: Href, options?: RouterPushRetryOptions): Promise<void> => { | |
| const maxAttempts = options?.maxAttempts ?? 1; | |
| const retryDelayMs = options?.retryDelayMs ?? 250; | |
| if (!Number.isInteger(maxAttempts) || maxAttempts < 1) { | |
| throw new RangeError('maxAttempts must be a positive integer'); | |
| } | |
| if (!Number.isFinite(retryDelayMs) || retryDelayMs < 0) { | |
| throw new RangeError('retryDelayMs must be a finite non-negative number'); | |
| } | |
| for (let attempt = 1; ; attempt++) { | |
| try { | |
| router.push(href); | |
| return; | |
| } catch (error) { | |
| if (attempt >= maxAttempts) { | |
| throw error; | |
| } | |
| await new Promise((resolve) => setTimeout(resolve, retryDelayMs)); |
🤖 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 `@src/lib/navigation.ts` around lines 6 - 27, Validate RouterPushRetryOptions
before entering the retry loop in routerPushWithRetry: require maxAttempts to be
a finite positive integer and retryDelayMs to be finite and non-negative,
rejecting invalid values immediately. Preserve the existing defaults and retry
behavior for valid options, while ensuring maxAttempts = 0 cannot trigger a
navigation attempt.
| drainOutbox: async () => { | ||
| if (isDrainingOutbox) return; | ||
| isDrainingOutbox = true; | ||
| try { | ||
| const userId = currentUserId(); | ||
| const now = Date.now(); | ||
| let nextEligibleIn: number | null = null; | ||
| for (const item of [...get().outbox]) { | ||
| if (item.SenderUserId && item.SenderUserId !== userId) continue; | ||
| const attempts = item.Attempts ?? 0; | ||
| if (attempts >= MAX_OUTBOX_ATTEMPTS) continue; | ||
| const delay = outboxRetryDelayMs(attempts); | ||
| const elapsed = now - (item.LastAttemptAt ?? 0); | ||
| if (attempts > 0 && elapsed < delay) { | ||
| nextEligibleIn = Math.min(nextEligibleIn ?? Number.MAX_SAFE_INTEGER, delay - elapsed); | ||
| continue; | ||
| } | ||
| await sendOutboxItem(item, set, get); | ||
| } | ||
| if (nextEligibleIn !== null) scheduleOutboxDrain(nextEligibleIn); | ||
| } finally { | ||
| isDrainingOutbox = false; | ||
| } | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether the send-message endpoint deduplicates by ClientMessageId server-side or client-side elsewhere.
rg -n 'ClientMessageId' src/api/chat/chat.ts src/stores/chat/store.ts -B2 -A2Repository: Resgrid/Dispatch
Length of output: 6121
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant store implementation around sendMessage, outbox insertion, sending, and retry.
sed -n '300,425p' src/stores/chat/store.ts
printf '\n--- sendOutboxItem ---\n'
sed -n '835,885p' src/stores/chat/store.ts
printf '\n--- retryOutboxItem/markOutboxFailed ---\n'
sed -n '360,385p' src/stores/chat/store.ts
sed -n '938,975p' src/stores/chat/store.ts
printf '\n--- ClientMessageId occurrences in store ---\n'
rg -n 'ClientMessageId|isDrainingOutbox|sendOutboxItem|handleChatConnected' src/stores/chat/store.tsRepository: Resgrid/Dispatch
Length of output: 12108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check send-message endpoint and all sendMessage usages to understand whether ClientMessageId deduplication is implemented anywhere.
rg -n 'sendMessage|ClientMessageId' src/api src -g '!**/node_modules/**' | sed -n '1,220p'Repository: Resgrid/Dispatch
Length of output: 3918
Prevent concurrent sends for the same outbox item.
sendMessage and retryOutboxItem can call sendOutboxItem at the same time as drainOutbox, because drainOutbox only guards against one drain in progress and the item is removed from outbox only after the API call resolves. This can POST the same ClientMessageId to chatApi.sendMessage more than once per store. Track in-flight ClientMessageIds at the module level and skip an item in drainOutbox if another caller is already sending it.
🤖 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 `@src/stores/chat/store.ts` around lines 393 - 416, Track in-flight
ClientMessageId values at module scope and have sendOutboxItem register and
clear each ID with guaranteed cleanup around the API request. Update drainOutbox
to skip items whose ClientMessageId is already in flight, while preserving
existing retry and eligibility handling so sendMessage and retryOutboxItem
cannot issue duplicate sends.
| "urgent": "Dringend", | ||
| "urgent_will_send": "Diese Nachricht wird als dringend gesendet", | ||
| "shared_location": "Geteilter Standort", | ||
| "thread_replies": "{{count}} Antworten", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked translation files around target key:"
for f in src/translations/de.json src/translations/ar.json src/translations/es.json src/translations/fr.json src/translations/it.json src/translations/pl.json src/translations/uk.json; do
if [ -f "$f" ]; then
line=$(grep -n '"thread_replies"' "$f" | head -n1 | cut -d: -f1 || true)
echo "--- $f line $line"
sed -n "$((line>3?line-3:1)),$((line+3))p" "$f" | cat -n
else
echo "MISSING $f"
fi
done
echo
echo "Search for thread_replies usages:"
rg -n "thread_replies|\.t\(.*thread_replies|thread_replies" src -S || true
echo
echo "i18next config/key namespaces hints:"
rg -n "i18next|i18n|useTranslation|changeLanguage|savedKeys|resources|thread_replies" src -S | head -n 120 || trueRepository: Resgrid/Dispatch
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Lockfile/package references for i18next variants:"
for f in package.json pnpm-lock.yaml yarn.lock package-lock.json; do
if [ -f "$f" ]; then
echo "--- $f"
rg -n "i18next|`@lingui`|plural-forms|react-i18next" "$f" || true
fi
done
echo
echo "i18next config declarations:"
rg -n "i18next.init|init\\(|react-i18next|pluralSeparator|plurals|resStore|resources|fallbackLng|supportedLngs" src package.json -S --max-count 200 || true
echo
echo "Translation files for thread_replies variant keys (focused parse):"
python3 - <<'PY'
import json, os, re
targets = ["de","ar","es","fr","it","pl","uk","en","sv"]
key = "chat.thread_replies"
variants = [
("de", ["singular","plural"]),
("ar", ["zero","one","two","few","many","other"]),
("es", ["one","other"]),
("fr", ["one","other"]),
("it", ["one","other"]),
("pl", ["one","few","many","other"]),
("uk", ["one","few","many","other"]),
]
for loc in targets:
path = f"src/translations/{loc}.json"
if not os.path.isfile(path):
print(f"MISSING {path}")
continue
data=json.load(open(path, encoding="utf-8"))
chat=data.get("chat")
candidates=[]
if isinstance(chat, dict):
for k,v in chat.items():
if k == key or k.startswith(key+"."):
candidates.append((k,v))
print(f"--- {path}: {len(candidates)} candidates")
for k,v in candidates:
print(f" {k}:{v!r}")
for base, keys in variants:
print(f"{loc} candidates: {[k for k,v in candidates if k.startswith(f'{base}.')]}")
PY
echo
echo "Component context:"
sed -n '148,174p' src/components/chat/message-bubble.tsx | cat -nRepository: Resgrid/Dispatch
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Focused manifest dependency refs:"
sed -n '1,220p' package.json | rg -n "\"(i18next|react-i18next|i18next|packages|i18next[^"])":" package.json || true
echo
echo "i18next init/config files:"
for f in src/lib/i18n/index.tsx src/lib/i18n/resources.ts src/lib/i18n/utils.tsx src/lib/i18n/resources-*.ts; do
if [ -f "$f" ]; then
echo "--- $f"
sed -n '1,160p' "$f" | cat -n
fi
done
echo
echo "Focused translation key occurrences:"
python3 - <<'PY'
import json, pathlib, re
for f in sorted(pathlib.Path("src/translations").glob("*.json")):
data=json.loads(f.read_text())
chat=data.get("chat") or {}
hits=[(k,v) for k,v in chat.items() if k == "thread_replies" or k.startswith("thread_replies.")]
if hits:
print("---", f)
for k,v in hits: print(f" {k}: {v!r}")
PY
echo
echo "Component locale/call site:"
sed -n '1,220p' src/components/chat/message-bubble.tsx | cat -nRepository: Resgrid/Dispatch
Length of output: 241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Focused manifest dependency refs:"
python3 - <<'PY'
import json
from pathlib import Path
p=Path("package.json")
data=json.loads(p.read_text())
for section in ("dependencies","devDependencies","peerDependencies","optionalDependencies"):
vals=data.get(section) or {}
matches={k:v for k,v in vals.items() if "i18next" in k}
if matches:
print(section, matches)
PY
echo
echo "i18next init/config files:"
for f in src/lib/i18n/index.tsx src/lib/i18n/resources.ts src/lib/i18n/utils.tsx; do
if [ -f "$f" ]; then
echo "--- $f"
sed -n '1,160p' "$f" | cat -n
fi
done
echo
echo "Focused translation key occurrences:"
python3 - <<'PY'
import json
from pathlib import Path
for f in sorted(Path("src/translations").glob("*.json")):
data=json.loads(f.read_text())
chat=data.get("chat") or {}
hits=[(k,v) for k,v in chat.items() if k == "thread_replies" or k.startswith("thread_replies.")]
if hits:
print("---", f)
for k,v in hits:
print(f" {k}: {v!r}")
PY
echo
echo "Component locale/call site:"
sed -n '1,220p' src/components/chat/message-bubble.tsx | cat -nRepository: Resgrid/Dispatch
Length of output: 15791
🌐 Web query:
i18next 23 plural keys compatibilityJSON v3 example key_one key_other
💡 Result:
In i18next versions 21 through 23, setting compatibilityJSON: 'v3' allows you to use older JSON format pluralization rules, which differ from the standard v4 format [1][2][3]. Under v3 compatibility mode, pluralization does not strictly require the _one and _other suffixes used in the modern Intl.PluralRules-based (v4) format [4][2][5]. Instead, v3 typically relies on suffixes like _plural or numerical suffixes (e.g., _0, _1, _2) depending on the specific language's plural requirements [1][6][3]. If you are currently using or migrating to a version of i18next newer than v23, please note that the compatibilityJSON: 'v3' option has been removed [1][2][5]. Since i18next v24, the Intl.PluralRules API is mandatory [5]. If your environment (such as certain React Native configurations) lacks this API, you must use a polyfill (like intl-pluralrules) rather than relying on the v3 compatibility mode [4][5]. Example of modern (v4) plural keys (the standard for v24+): { "key_one": "item", "key_other": "items" } If you are using v21-v23 and specifically want to enforce v3 behavior, you would configure it as follows [1][2]: i18next.init({ compatibilityJSON: 'v3' }); For environments requiring migration, the i18next team provides conversion tools to update existing v3-style translation files to the modern v4 format [4][7].
Citations:
- 1: https://www.i18next.com/misc/json-format
- 2: https://www.i18next.com/misc/migration-guide
- 3: https://www.i18next.com/misc/json-format.md
- 4: https://www.i18next.com/translation-function/plurals
- 5: https://www.i18next.com/how-to/faq
- 6:
key_oneandkey_otherinstead ofkeyandkey_plurali18next/i18next-scanner#228 - 7: https://www.i18next.com/misc/migration-guide.md
Add locale-specific plural variants for chat.thread_replies.
src/components/chat/message-bubble.tsx passes count, but the affected translation files only define one plural noun form. Add the i18next plural keys needed for each locale so single-reply threads do not use the plural form.
- German, Spanish, French, Italian: add
chat.thread_repliesplural forms alongsidechat.thread_replies/singular. - Arabic: add the required Arabic count-specific forms.
- Polish, Ukrainian: add
few,many, andotherforms as needed for local grammar.
📍 Affects 7 files
src/translations/de.json#L1460-L1460(this comment)src/translations/ar.json#L1460-L1460src/translations/es.json#L1460-L1460src/translations/fr.json#L1460-L1460src/translations/it.json#L1460-L1460src/translations/pl.json#L1460-L1460src/translations/uk.json#L1460-L1460
🤖 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 `@src/translations/de.json` at line 1460, Update the chat.thread_replies
translations to use i18next plural variants: add the locale-appropriate plural
keys in src/translations/de.json:1460-1460, src/translations/es.json:1460-1460,
src/translations/fr.json:1460-1460, and src/translations/it.json:1460-1460; add
Arabic zero, one, two, few, many, and other forms in
src/translations/ar.json:1460-1460; and add few, many, and other forms in
src/translations/pl.json:1460-1460 and src/translations/uk.json:1460-1460.
Preserve the existing count interpolation and singular wording so a single reply
uses the singular form.
| const unsubscribe = useChatStore.subscribe((state) => { | ||
| const sent = (state.messagesByChannel[channelId] ?? []).find((m) => m._localAttachmentUri === uri && !m.ChatMessageId.startsWith('local-')); | ||
| if (!sent) return; | ||
| unsubscribe(); | ||
| void (async () => { | ||
| try { | ||
| await uploadAttachment(channelId, sent.ChatMessageId, { uri, name, type }); | ||
| } catch { | ||
| useToastStore.getState().showToast('error', t('chat.attachment_failed')); | ||
| } | ||
| })(); | ||
| }); |
There was a problem hiding this comment.
Subscription leak: handleSendImage creates a useChatStore.subscribe listener (line 155) that only unsubscribes on message reconciliation, so a terminal 4xx failure or exhausted MAX_OUTBOX_ATTEMPTS leaves a permanent subscription that fires an O(N) .find() over channel messages on every store update. Add a _localStatus === 'failed' check inside the callback to unsubscribe on terminal failure.
const unsubscribe = useChatStore.subscribe((state) => {
const msg = (state.messagesByChannel[channelId] ?? []).find((m) => m._localAttachmentUri === uri);
if (!msg) return;
if (msg._localStatus === 'failed') { unsubscribe(); return; }
if (msg.ChatMessageId.startsWith('local-')) return;
unsubscribe();
void (async () => {
try {
await uploadAttachment(channelId, msg.ChatMessageId, { uri, name, type });
} catch {
useToastStore.getState().showToast('error', t('chat.attachment_failed'));
}
})();
});Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 155 to 166:
Subscription leak: `handleSendImage` creates a `useChatStore.subscribe` listener (line 155) that only unsubscribes on message reconciliation, so a terminal 4xx failure or exhausted `MAX_OUTBOX_ATTEMPTS` leaves a permanent subscription that fires an O(N) `.find()` over channel messages on every store update. Add a `_localStatus === 'failed'` check inside the callback to unsubscribe on terminal failure.
Suggested Code:
const unsubscribe = useChatStore.subscribe((state) => {
const msg = (state.messagesByChannel[channelId] ?? []).find((m) => m._localAttachmentUri === uri);
if (!msg) return;
if (msg._localStatus === 'failed') { unsubscribe(); return; }
if (msg.ChatMessageId.startsWith('local-')) return;
unsubscribe();
void (async () => {
try {
await uploadAttachment(channelId, msg.ChatMessageId, { uri, name, type });
} catch {
useToastStore.getState().showToast('error', t('chat.attachment_failed'));
}
})();
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const unsubscribe = useChatStore.subscribe((state) => { | ||
| const sent = (state.messagesByChannel[channelId] ?? []).find((m) => m._localAttachmentUri === uri && !m.ChatMessageId.startsWith('local-')); | ||
| if (!sent) return; | ||
| unsubscribe(); | ||
| void (async () => { | ||
| try { | ||
| await uploadAttachment(channelId, sent.ChatMessageId, { uri, name, type }); | ||
| } catch { | ||
| useToastStore.getState().showToast('error', t('chat.attachment_failed')); | ||
| } | ||
| })(); | ||
| }); |
There was a problem hiding this comment.
Subscription leak: useChatStore.subscribe (line 155) never unsubscribes if the component unmounts before reconciliation or the message hits a terminal failure, accumulating permanent listeners that perform an O(N) .find() on every store state change. Track the subscription in a ref with useEffect cleanup, or unsubscribe when _localStatus transitions to 'failed'.
// Store unsubscribe in a ref and clean up on unmount or terminal failure:
// const subRef = useRef<(() => void) | null>(null);
// subRef.current = useChatStore.subscribe((state) => {
// const msg = (state.messagesByChannel[channelId] ?? []).find(
// (m) => m._localAttachmentUri === uri
// );
// if (!msg) return;
// if (!msg.ChatMessageId.startsWith('local-')) {
// subRef.current?.();
// void uploadAttachment(channelId, msg.ChatMessageId, { uri, name, type });
// } else if (msg._localStatus === 'failed') {
// subRef.current?.(); // stop watching permanently-failed messages
// }
// });Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 155 to 166:
Subscription leak: `useChatStore.subscribe` (line 155) never unsubscribes if the component unmounts before reconciliation or the message hits a terminal failure, accumulating permanent listeners that perform an O(N) `.find()` on every store state change. Track the subscription in a ref with `useEffect` cleanup, or unsubscribe when `_localStatus` transitions to `'failed'`.
Suggested Code:
// Store unsubscribe in a ref and clean up on unmount or terminal failure:
// const subRef = useRef<(() => void) | null>(null);
// subRef.current = useChatStore.subscribe((state) => {
// const msg = (state.messagesByChannel[channelId] ?? []).find(
// (m) => m._localAttachmentUri === uri
// );
// if (!msg) return;
// if (!msg.ChatMessageId.startsWith('local-')) {
// subRef.current?.();
// void uploadAttachment(channelId, msg.ChatMessageId, { uri, name, type });
// } else if (msg._localStatus === 'failed') {
// subRef.current?.(); // stop watching permanently-failed messages
// }
// });
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } catch { | ||
| useToastStore.getState().showToast('error', t('chat.attachment_failed')); |
There was a problem hiding this comment.
Swallowed exception: the catch block discards the error without logging, permanently losing debug details and violating rule [28]. Capture the error parameter and log it with structured context (channelId, name, err) before showing the toast.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 162 to 163:
Swallowed exception: the `catch` block discards the error without logging, permanently losing debug details and violating rule [28]. Capture the error parameter and log it with structured context (`channelId`, `name`, `err`) before showing the toast.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| })(); | ||
| }); | ||
|
|
||
| void useChatStore.getState().sendMessage({ |
There was a problem hiding this comment.
Unhandled promise rejection: void useChatStore.getState().sendMessage(...) discards rejections without a .catch() handler, violating rule [1]. Add .catch((e) => logger.error('sendMessage failed', { channelId, err: e })) or wrap the call in a try/catch inside an async IIFE.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 168:
Unhandled promise rejection: `void useChatStore.getState().sendMessage(...)` discards rejections without a `.catch()` handler, violating rule [1]. Add `.catch((e) => logger.error('sendMessage failed', { channelId, err: e }))` or wrap the call in a try/catch inside an async IIFE.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| abortRef.current = controller; | ||
| setLoading(true); | ||
| try { | ||
| const response = await searchGifs(q || undefined, 24, 0, controller.signal); |
There was a problem hiding this comment.
Magic numbers 24 and 0 inlined in the searchGifs call obscure pagination intent. Extract them to named constants such as GIFS_PER_PAGE and INITIAL_OFFSET at module or component scope.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/components/chat/gif-picker-sheet.tsx:
Line 39:
Magic numbers `24` and `0` inlined in the `searchGifs` call obscure pagination intent. Extract them to named constants such as `GIFS_PER_PAGE` and `INITIAL_OFFSET` at module or component scope.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <Pressable className="p-2" onPress={handleShareLocation} disabled={disabled} accessibilityLabel={t('chat.share_location')}> | ||
| <MapPin size={22} color="#6b7280" /> | ||
| </Pressable> | ||
| <Pressable className="p-2" onPress={() => setUrgent((prev) => !prev)} disabled={disabled} accessibilityLabel={t('chat.urgent')}> |
There was a problem hiding this comment.
Inline arrow function in JSX prop creates a new function on every render, impacting performance. Extract the setUrgent toggle into a useCallback-wrapped handler outside the JSX.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/components/chat/message-composer.tsx:
Line 142:
Inline arrow function in JSX prop creates a new function on every render, impacting performance. Extract the `setUrgent` toggle into a `useCallback`-wrapped handler outside the JSX.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (attempt >= maxAttempts) { | ||
| throw error; | ||
| } | ||
| await new Promise((resolve) => setTimeout(resolve, retryDelayMs)); |
There was a problem hiding this comment.
Uncancellable setTimeout in the retry loop may fire navigation after the caller unmounts or abandons the promise. Store the timer ID and provide a cancellation mechanism (e.g., AbortSignal) that calls clearTimeout(id).
Kody rule violation: Clear timers on teardown/unmount
Prompt for LLM
File src/lib/navigation.ts:
Line 27:
Uncancellable `setTimeout` in the retry loop may fire navigation after the caller unmounts or abandons the promise. Store the timer ID and provide a cancellation mechanism (e.g., AbortSignal) that calls `clearTimeout(id)`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /** | ||
| * Pushes an expo-router href, retrying when the router has not mounted yet | ||
| * (cold-start deep links). Throws the last error once every attempt fails. | ||
| */ | ||
| export const routerPushWithRetry = async (href: Href, options?: RouterPushRetryOptions): Promise<void> => { |
There was a problem hiding this comment.
Missing @returns and @throws JSDoc tags on routerPushWithRetry omit the resolution type and rejection conditions callers need for safe await usage. Add @returns {Promise<void>} and @throws {Error} When all retry attempts fail to the JSDoc block.
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File src/lib/navigation.ts:
Line 11 to 15:
Missing `@returns` and `@throws` JSDoc tags on `routerPushWithRetry` omit the resolution type and rejection conditions callers need for safe await usage. Add `@returns {Promise<void>}` and `@throws {Error} When all retry attempts fail` to the JSDoc block.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (itemIdx >= 0) { | ||
| const attempts = (s.outbox[itemIdx].Attempts ?? 0) + 1; | ||
| if (terminal || attempts >= MAX_OUTBOX_ATTEMPTS) { | ||
| outbox = s.outbox.filter((o) => o.ClientMessageId !== clientMessageId); | ||
| } else { | ||
| outbox = s.outbox.slice(); | ||
| outbox[itemIdx] = { ...outbox[itemIdx], Attempts: attempts, LastAttemptAt: Date.now() }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Dead retry path: markOutboxFailed increments Attempts and sets LastAttemptAt but never calls scheduleOutboxDrain, leaving items that fail during the initial sendMessage path (line 368) with backoff metadata and no active timer. Call scheduleOutboxDrain(outboxRetryDelayMs(attempts)) in the non-terminal, under-max-attempts branch after incrementing attempts.
} else {
outbox = s.outbox.slice();
outbox[itemIdx] = { ...outbox[itemIdx], Attempts: attempts, LastAttemptAt: Date.now() };
}
}
// Schedule retry for non-terminal failures that remain in the outbox.
// Must be outside the set() callback since scheduleOutboxDrain uses setTimeout.
// (Add after the set() call: if the item is still in outbox and non-terminal,
// call scheduleOutboxDrain(outboxRetryDelayMs(newAttempts)))Prompt for LLM
File src/stores/chat/store.ts:
Line 953 to 961:
Dead retry path: `markOutboxFailed` increments `Attempts` and sets `LastAttemptAt` but never calls `scheduleOutboxDrain`, leaving items that fail during the initial `sendMessage` path (line 368) with backoff metadata and no active timer. Call `scheduleOutboxDrain(outboxRetryDelayMs(attempts))` in the non-terminal, under-max-attempts branch after incrementing attempts.
Suggested Code:
} else {
outbox = s.outbox.slice();
outbox[itemIdx] = { ...outbox[itemIdx], Attempts: attempts, LastAttemptAt: Date.now() };
}
}
// Schedule retry for non-terminal failures that remain in the outbox.
// Must be outside the set() callback since scheduleOutboxDrain uses setTimeout.
// (Add after the set() call: if the item is still in outbox and non-terminal,
// call scheduleOutboxDrain(outboxRetryDelayMs(newAttempts)))
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| let after = highestRealSeq(get().messagesByChannel[channelId]); | ||
| for (;;) { | ||
| const response = await chatApi.getMessagesAfter(channelId, after, 200); | ||
| const incoming = response.Data ?? []; | ||
| if (incoming.length === 0) return; | ||
| set((s) => { | ||
| let list = s.messagesByChannel[channelId] ?? []; | ||
| for (const m of incoming) list = upsertMessage(list, { ...m, _localStatus: 'sent' }); | ||
| return { messagesByChannel: { ...s.messagesByChannel, [channelId]: list } }; | ||
| }); | ||
| if (incoming.length < 200) return; | ||
| after = highestRealSeq(get().messagesByChannel[channelId]); | ||
| } |
There was a problem hiding this comment.
Infinite loop risk: loadNewerMessages uses an unbounded for (;;) loop (line 303) that re-fetches the same 200-message batch indefinitely if the server returns without advancing the seq cursor, triggering full FlatList re-renders on every reconnect via handleChatConnected (line 793). Add a maximum iteration count (e.g., 50 batches) as a safety bound.
let after = highestRealSeq(get().messagesByChannel[channelId]);
for (let batch = 0; batch < 50; batch++) {
const response = await chatApi.getMessagesAfter(channelId, after, 200);
const incoming = response.Data ?? [];
if (incoming.length === 0) return;Prompt for LLM
File src/stores/chat/store.ts:
Line 302 to 314:
Infinite loop risk: `loadNewerMessages` uses an unbounded `for (;;)` loop (line 303) that re-fetches the same 200-message batch indefinitely if the server returns without advancing the seq cursor, triggering full FlatList re-renders on every reconnect via `handleChatConnected` (line 793). Add a maximum iteration count (e.g., 50 batches) as a safety bound.
Suggested Code:
let after = highestRealSeq(get().messagesByChannel[channelId]);
for (let batch = 0; batch < 50; batch++) {
const response = await chatApi.getMessagesAfter(channelId, after, 200);
const incoming = response.Data ?? [];
if (incoming.length === 0) return;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| }, | ||
|
|
||
| drainOutbox: async () => { |
There was a problem hiding this comment.
Missing retry scheduling: outbox items that fail their first send attempt via sendMessage (line 368) or drainOutbox (line 410) sit permanently marked 'failed' because scheduleOutboxDrain is only invoked for items already in backoff at drainOutbox start (line 406). After sendOutboxItem fails, re-check whether the item remains in the outbox with Attempts > 0 and schedule a drain for its backoff delay.
if (attempts > 0 && elapsed < delay) {
nextEligibleIn = Math.min(nextEligibleIn ?? Number.MAX_SAFE_INTEGER, delay - elapsed);
continue;
}
await sendOutboxItem(item, set, get);
// After a failure, re-check whether the item is now in backoff and needs scheduling.
const after = get().outbox.find((o) => o.ClientMessageId === item.ClientMessageId);
if (after && (after.Attempts ?? 0) > 0) {
const remaining = outboxRetryDelayMs(after.Attempts ?? 0);
nextEligibleIn = Math.min(nextEligibleIn ?? Number.MAX_SAFE_INTEGER, remaining);
}
}
if (nextEligibleIn !== null) scheduleOutboxDrain(nextEligibleIn);Prompt for LLM
File src/stores/chat/store.ts:
Line 393:
Missing retry scheduling: outbox items that fail their first send attempt via `sendMessage` (line 368) or `drainOutbox` (line 410) sit permanently marked 'failed' because `scheduleOutboxDrain` is only invoked for items already in backoff at `drainOutbox` start (line 406). After `sendOutboxItem` fails, re-check whether the item remains in the outbox with `Attempts > 0` and schedule a drain for its backoff delay.
Suggested Code:
if (attempts > 0 && elapsed < delay) {
nextEligibleIn = Math.min(nextEligibleIn ?? Number.MAX_SAFE_INTEGER, delay - elapsed);
continue;
}
await sendOutboxItem(item, set, get);
// After a failure, re-check whether the item is now in backoff and needs scheduling.
const after = get().outbox.find((o) => o.ClientMessageId === item.ClientMessageId);
if (after && (after.Attempts ?? 0) > 0) {
const remaining = outboxRetryDelayMs(after.Attempts ?? 0);
nextEligibleIn = Math.min(nextEligibleIn ?? Number.MAX_SAFE_INTEGER, remaining);
}
}
if (nextEligibleIn !== null) scheduleOutboxDrain(nextEligibleIn);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| let after = highestRealSeq(get().messagesByChannel[channelId]); | ||
| for (;;) { | ||
| const response = await chatApi.getMessagesAfter(channelId, after, 200); | ||
| const incoming = response.Data ?? []; | ||
| if (incoming.length === 0) return; | ||
| set((s) => { | ||
| let list = s.messagesByChannel[channelId] ?? []; | ||
| for (const m of incoming) list = upsertMessage(list, { ...m, _localStatus: 'sent' }); | ||
| return { messagesByChannel: { ...s.messagesByChannel, [channelId]: list } }; | ||
| }); | ||
| if (incoming.length < 200) return; | ||
| after = highestRealSeq(get().messagesByChannel[channelId]); | ||
| } |
There was a problem hiding this comment.
Unbounded fetch loop: loadNewerMessages (line 303) issues sequential blocking HTTP calls on every SignalR reconnect via handleChatConnected (line 793), holding the JS thread with no cancellation path for backlog-heavy channels. Add a maximum iteration cap and/or an AbortSignal.
// Add a max iteration cap (e.g., 10 pages = 2000 messages) and/or an AbortSignal:
// const MAX_PAGES = 10;
// for (let page = 0; page < MAX_PAGES; page++) { ... }Prompt for LLM
File src/stores/chat/store.ts:
Line 302 to 314:
Unbounded fetch loop: `loadNewerMessages` (line 303) issues sequential blocking HTTP calls on every SignalR reconnect via `handleChatConnected` (line 793), holding the JS thread with no cancellation path for backlog-heavy channels. Add a maximum iteration cap and/or an AbortSignal.
Suggested Code:
// Add a max iteration cap (e.g., 10 pages = 2000 messages) and/or an AbortSignal:
// const MAX_PAGES = 10;
// for (let page = 0; page < MAX_PAGES; page++) { ... }
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| pendingChatbotMessages.delete(clientMessageId); | ||
| patchMessage(set, pending.channelId, `local-${clientMessageId}`, { _localStatus: 'sent' }); | ||
| } catch (error) { | ||
| logger.error({ message: 'chat: chatbot resend failed', context: { error } }); |
There was a problem hiding this comment.
Incomplete error context: the chatbot resend failure log omits clientMessageId and channelId, violating rule [3] which requires the operation name and relevant identifiers as structured fields. Add both fields to the context object alongside error.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File src/stores/chat/store.ts:
Line 386:
Incomplete error context: the chatbot resend failure log omits `clientMessageId` and `channelId`, violating rule [3] which requires the operation name and relevant identifiers as structured fields. Add both fields to the context object alongside `error`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
|
||
| <AckBanner acks={pendingAcks} onAcknowledge={(messageId) => useChatStore.getState().acknowledgeMessage(messageId)} /> | ||
|
|
||
| <ScrollView className="flex-1" refreshControl={<RefreshControl refreshing={isLoading} onRefresh={() => useChatStore.getState().fetchChannels()} />}> |
There was a problem hiding this comment.
Performance degradation results from using inline arrow functions in JSX props, generating new function allocations on every render. Move these function definitions outside the render method in src/app/(app)/chat.tsx, src/app/(app)/chatbot.tsx:80-80, src/app/chat/[channelId].tsx:271-271, src/app/chat/thread/[messageId].tsx:115-115, and src/components/chat/new-conversation-sheet.tsx:171-171.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/(app)/chat.tsx:
Line 124:
Performance degradation results from using inline arrow functions in JSX props, generating new function allocations on every render. Move these function definitions outside the render method in `src/app/(app)/chat.tsx`, `src/app/(app)/chatbot.tsx:80-80`, `src/app/chat/[channelId].tsx:271-271`, `src/app/chat/thread/[messageId].tsx:115-115`, and `src/components/chat/new-conversation-sheet.tsx:171-171`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const userId = recipientUserId(recipient); | ||
| const isSelected = selected.has(userId); | ||
| return ( | ||
| <Pressable key={recipient.Id} className="py-2" onPress={() => (mode === 'dm' ? startDirectMessage(recipient) : toggle(userId))} disabled={submitting}> |
There was a problem hiding this comment.
Magic string vulnerability detected where the literal 'dm' evaluates against the mode variable, risking silent bugs from typos. Extract conversation modes into a const tuple (e.g., const ConversationMode = { Dm: 'dm', Group: 'group' } as const) and compare against ConversationMode.Dm.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/components/chat/new-conversation-sheet.tsx:
Line 171:
Magic string vulnerability detected where the literal `'dm'` evaluates against the `mode` variable, risking silent bugs from typos. Extract conversation modes into a const tuple (e.g., `const ConversationMode = { Dm: 'dm', Group: 'group' } as const`) and compare against `ConversationMode.Dm`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
2 participants
Description
This PR introduces Chat and Chatbot (Assistant) functionality to the Resgrid Dispatch app, providing real-time messaging capabilities including direct messages, group channels, incident channels, and an AI assistant.
Key Features Added
Chat System:
Chatbot Assistant:
Infrastructure:
CHAT_HUB_NAMEenvironment configurationinvokemethod to support variadic argumentsSummary by CodeRabbit