Conversation
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe change adds a complete chat platform with typed APIs, persisted optimistic state, SignalR events, chatbot support, conversation screens, message tools, attachments, notifications, navigation, and localization. ChangesChat Platform
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChatScreen
participant useChatStore
participant SignalRStore
participant ChatHub
User->>ChatScreen: open conversation
ChatScreen->>useChatStore: load channel and messages
User->>ChatScreen: send message
ChatScreen->>useChatStore: create optimistic message
useChatStore->>ChatHub: invoke send operation
SignalRStore->>ChatHub: maintain chat connection
ChatHub-->>SignalRStore: deliver message event
SignalRStore->>useChatStore: apply realtime event
useChatStore-->>ChatScreen: reconcile message 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: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (26)
src/stores/chat/store.ts-1-23 (1)
1-23: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe
simple-import-sortcheck fails in two new files. Thetestworkflow reports "Run autofix to sort these imports!" and "Run autofix to sort these exports!". The shared root cause is that the new chat files were not passed through the lint autofix before commit.
src/stores/chat/store.ts#L1-L23: reorder the import block to the ordersimple-import-sortexpects, and switch theEnvimport to the@envalias.src/models/v4/chat/index.ts#L1-L6: reorder the sixexport * fromstatements alphabetically.Run the repository lint task with
--fixand commit the result, so the check passes for both files at once.🤖 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 - 23, In src/stores/chat/store.ts lines 1-23, run import sorting and change Env to use the `@env` alias; in src/models/v4/chat/index.ts lines 1-6, alphabetize all six export statements. Run the repository lint task with --fix to apply both changes.Sources: Coding guidelines, Linters/SAST tools
src/stores/chat/store.ts-564-577 (1)
564-577: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
chatbotTypingcan staytrueindefinitely.The store sets
chatbotTyping: truewhen the send starts. It clears the flag only in thecatchblock, inhandleChatbotMessageReceived, or on achatbotTypingevent. If the request succeeds but the bot never replies (server-side failure, dropped SignalR event, hub reconnect), the typing indicator never clears.Add a timeout that clears
chatbotTypingafter a bounded interval, and cancel it when a reply arrives.🤖 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 564 - 577, The chatbot send flow around the typing state update must prevent chatbotTyping from remaining true when no reply arrives. Add a bounded timeout after setting chatbotTyping in the relevant store action, clear the flag when it expires, and retain the timeout handle so reply handling in handleChatbotMessageReceived can cancel it before clearing the indicator.src/stores/signalr/signalr-store.ts-451-463 (1)
451-463: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClear
isChatHubConnectedin thecatchblock.The function stops the heartbeat and unregisters all handlers before
disconnectFromHub. IfdisconnectFromHubthrows,isChatHubConnectedstaystruewhile no handlers are registered and no heartbeat runs. The guard at line 381 then blocks any laterconnectChatHub()call.🐛 Proposed fix
} catch (error) { const err = error instanceof Error ? error : new Error('Unknown error occurred'); logger.error({ message: 'Failed to disconnect from chat SignalR hub', context: { error: err } }); - set({ error: err }); + set({ error: err, isChatHubConnected: false }); }🤖 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 451 - 463, Update the catch block in disconnectChatHub to set isChatHubConnected to false when disconnectFromHub fails, alongside the existing error state update, so a later connectChatHub call is not blocked by stale connection state.src/stores/chat/store.ts-172-185 (1)
172-185: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCollapse the
findIndexpredicate to satisfy Prettier.The
testcheck reports a Prettier violation at line 174. The print width is 220, so the predicate fits on one line.🔧 Proposed fix
- const idx = next.findIndex( - (m) => m.ChatMessageId === incoming.ChatMessageId || (!!incoming.ClientMessageId && !!m.ClientMessageId && m.ClientMessageId === incoming.ClientMessageId) - ); + const idx = next.findIndex((m) => m.ChatMessageId === incoming.ChatMessageId || (!!incoming.ClientMessageId && !!m.ClientMessageId && m.ClientMessageId === incoming.ClientMessageId));🤖 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 172 - 185, Update the findIndex predicate in upsertMessage to a single line so it conforms to the configured Prettier print width, preserving its existing matching logic.Source: Linters/SAST tools
src/stores/chat/store.ts-473-491 (1)
473-491: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReset
lastMarkedSeqwhenmarkReadfails.The store writes
lastMarkedSeq.set(channelId, seq)before the request. Thecatchblock only logs. A failedmarkReadtherefore blocks any retry for that sameseq, while the localUnreadCountalready shows0. The server keeps the channel unread until a newer message raisesseq.Delete the map entry in the
catchblock so the next call retries.🐛 Proposed fix
} catch (error) { + // Allow a retry for this seq — the pointer was not recorded server-side. + if ((lastMarkedSeq.get(channelId) ?? 0) === seq) lastMarkedSeq.delete(channelId); 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 473 - 491, Update the catch block in markChannelRead to delete the channel’s entry from lastMarkedSeq when chatApi.markRead fails, before or alongside the existing debug log, so a later call can retry the same sequence.src/stores/chat/store.ts-619-627 (1)
619-627: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
UnreadCountincrements on duplicate deliveries.
upsertMessagededuplicates byChatMessageIdandClientMessageId, but theUnreadCountincrement runs unconditionally. If the hub redelivers an event after a reconnect, or the same message arrives through bothloadNewerMessagesandchatMessageReceived, the badge inflates while the message list stays correct.Increment only when the message is new.
🐛 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 ); return { messagesByChannel: { ...s.messagesByChannel, [msg.ChatChannelId]: list }, channels }; });🤖 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 619 - 627, Update the state update around upsertMessage in chatMessageReceived to determine whether msg is already present by ChatMessageId or ClientMessageId before insertion. Increment UnreadCount only for a new message when the channel is neither active nor owned by the sender; preserve the existing count for duplicate deliveries and keep upsertMessage’s deduplication behavior.src/components/chat/message-composer.tsx-126-128 (1)
126-128: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winExpose the urgent toggle state to assistive technology.
The
Pressableacts as a toggle, but it only reports a static label. AddaccessibilityRole="switch"andaccessibilityState={{ checked: urgent }}so screen reader users know the current state.As per coding guidelines "Follow WCAG guidelines for mobile accessibility".
🤖 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 126 - 128, Add switch semantics to the urgent-toggle Pressable in the message composer by setting accessibilityRole to "switch" and accessibilityState.checked to the current urgent value. Keep the existing label, toggle handler, and visual state unchanged.Source: Coding guidelines
src/components/chat/message-composer.tsx-77-79 (1)
77-79: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winShow a toast when image selection or location capture fails.
Both catch blocks only log. The user sees no feedback and can assume the app is unresponsive. The denied-permission paths already show a toast, so keep the feedback consistent.
♻️ Proposed change
} catch (error) { logger.error({ message: 'chat: image pick failed', context: { error } }); + useToastStore.getState().showToast('error', t('chat.image_pick_failed')); }} catch (error) { logger.error({ message: 'chat: location share failed', context: { error } }); + useToastStore.getState().showToast('error', t('chat.location_share_failed')); }Add the two keys to
src/translations/en.jsonand to every other language file.As per coding guidelines "Handle errors gracefully and provide user feedback via toast notifications from
useToastStore".Also applies to: 92-94
🤖 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 77 - 79, Update the image-selection and location-capture catch blocks in the message composer to show a user-facing toast through useToastStore in addition to logging the error, matching the existing denied-permission feedback pattern. Add the corresponding translation keys to en.json and every other language file, and use those keys for both failure messages.Source: Coding guidelines
src/translations/en.json-332-332 (1)
332-332: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse i18next plural suffixes for the count-based strings.
thread_replies,are_typing,ack_pending_count, andcreate_group_withinterpolatecountbut define a single form. Withcount: 1the UI shows "1 replies" and "1 people are typing...". Define_oneand_otherkeys so i18next selects the correct form.♻️ Proposed change
- "thread_replies": "{{count}} replies", + "thread_replies_one": "{{count}} reply", + "thread_replies_other": "{{count}} replies",- "are_typing": "{{count}} people are typing...", + "are_typing_one": "{{count}} person is typing...", + "are_typing_other": "{{count}} people are typing...",Apply the same pattern to
ack_pending_countandcreate_group_with, then keep theAckBannerbranch at Line 29 ofsrc/components/chat/ack-banner.tsxor remove it in favor of the plural rule.Also applies to: 337-337, 349-349, 377-377
🤖 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 332, Update the count-based translation keys thread_replies, are_typing, ack_pending_count, and create_group_with in en.json to use i18next _one and _other variants, preserving the singular and plural wording for count values. Ensure callers, including the AckBanner branch, rely on the pluralized translation behavior without duplicating or overriding the singular case.src/app/chat/[channelId].tsx-216-216 (1)
216-216: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle the promises returned by these handlers.
acknowledgeMessageat Line 216 returns a promise that nothing awaits or catches. A rejection becomes an unhandled rejection. TheonCopyprop at Line 265 is declared as(message) => voidinMessageActionsSheet, but anasyncfunction is passed, so its rejection is also unhandled. Wrap both bodies so errors are caught and reported throughuseToastStore.Also applies to: 265-268
🤖 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 at line 216, Update the AckBanner onAcknowledge handler and the async onCopy handler passed to MessageActionsSheet so each handles promise rejection instead of returning an unobserved promise; catch failures and report them through useToastStore while preserving their existing successful behavior.src/components/chat/typing-indicator.tsx-15-15 (1)
15-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRun Prettier to clear the CI warning.
The
testcheck reports a formatting difference on this block. The print width is 220, so the sequence fits on one line. The same warning appears insrc/components/chat/message-composer.tsx,src/app/chat/[channelId].tsx, andsrc/app/chat/thread/[messageId].tsx.🤖 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/typing-indicator.tsx` at line 15, Run Prettier on the Animated.sequence blocks in typing-indicator.tsx, message-composer.tsx, chat/[channelId].tsx, and chat/thread/[messageId].tsx so each sequence is formatted to fit the configured 220-character print width and CI’s test check passes.Source: Linters/SAST tools
src/components/chat/message-actions-sheet.tsx-101-101 (1)
101-101: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winHardcoded hex icon colors across the chat components. Every new chat component passes raw hex values to
lucide-react-nativeicons. These values do not change with the color scheme, so icons keep light-mode contrast in dark mode. Define one shared color map that reads the semantic Tailwind tokens throughuseColorScheme(), then use it at each site.
src/components/chat/message-actions-sheet.tsx#L101-L101: replace#6b7280and#dc2626on all six action icons with the shared tokens.src/components/chat/ack-banner.tsx#L26-L26: replace#dc2626onAlertTrianglewith the error token used by the adjacenttext-error-700text.src/components/chat/message-composer.tsx#L101-L101: replace#6b7280,#dc2626, and#ffffffon the composer icons with the shared tokens.src/components/chat/typing-indicator.tsx#L9-L9: replace the'#9ca3af'default parameter with a token-derived default.src/app/chat/[channelId].tsx#L212-L212: replace#22c55e,#9ca3af, and#2563ebat Line 212 and Line 240 with the shared tokens.As per coding guidelines "Use semantic color tokens from Tailwind config, not hardcoded hex values" and "Ensure sufficient color contrast in both light and dark mode".
🤖 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` at line 101, Define a shared color map derived from semantic Tailwind tokens via useColorScheme(), then replace all hardcoded icon colors: in src/components/chat/message-actions-sheet.tsx:101-101 update all six action icons; in src/components/chat/ack-banner.tsx:26-26 use the error token matching adjacent text-error-700; in src/components/chat/message-composer.tsx:101-101 replace gray, red, and white values; in src/components/chat/typing-indicator.tsx:9-9 use a token-derived default; and in src/app/chat/[channelId].tsx:212-212 update the green, gray, and blue values at both referenced icon locations.Source: Coding guidelines
src/app/chat/[channelId].tsx-145-168 (1)
145-168: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winKeep the upload failure path on a failed server-confirmed message.
upsertMessagepreserves client-only fields when merging, so_localAttachmentUriremains on the confirmed message. However, theif (!sent)case still leaves the user with an attachment bubble while nothing reports the upload failure. Add anelsebranch that callsuseToastStore.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 145 - 168, The handleSendImage callback must report an upload failure when no server-confirmed message is found. Add an else branch to the existing if (sent) check that calls useToastStore.getState().showToast('error', t('chat.attachment_failed')), preserving the current upload and catch behavior.src/app/chat/[channelId].tsx-82-88 (1)
82-88: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCancel the presence request when the effect re-runs.
The effect calls
getPresenceon everymemberschange and never cancels the previous request. A slow earlier response can resolve last and overwrite newer presence data. The.thenalso runs after unmount.🛠️ Proposed fix
useEffect(() => { const ids = (members ?? []).map((m) => m.UserId).filter((id): id is string => !!id && id !== currentUserId); if (ids.length === 0) return; + let active = true; getPresence(ids) - .then((result) => setPresenceIds(new Set(result.OnlineUserIds ?? []))) + .then((result) => { + if (active) setPresenceIds(new Set(result.OnlineUserIds ?? [])); + }) .catch(() => undefined); + return () => { + active = false; + }; }, [members, currentUserId]);🤖 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 82 - 88, Update the presence-loading useEffect around getPresence so each effect run creates a cancellation or active-state guard, ignores results from superseded requests and unmounted components, and cleans up that guard when dependencies change or the component unmounts. Preserve the existing member filtering and setPresenceIds behavior for the latest active request.src/app/chat/[channelId].tsx-91-95 (1)
91-95: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKey the read receipt on the newest message, not the list length.
The effect depends on
inverted.length. If a message arrives while another is removed, or if the newest message changes without a length change, the effect does not run and the channel stays unread.♻️ Proposed change
- useEffect(() => { - if (channelId && inverted.length > 0) { - void useChatStore.getState().markChannelRead(channelId); - } - }, [channelId, inverted.length]); + const newestMessageId = inverted[0]?.ChatMessageId; + useEffect(() => { + if (channelId && newestMessageId) { + void useChatStore.getState().markChannelRead(channelId); + } + }, [channelId, newestMessageId]);🤖 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 91 - 95, Update the read-receipt effect in the chat channel component to depend on the newest message identity or timestamp rather than inverted.length, so it reruns when the latest message changes even if the list size is unchanged. Preserve the existing channelId and non-empty-list guards and markChannelRead behavior.src/translations/en.json-317-389 (1)
317-389: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the missing translation keys to all language files and sort the dictionary order.
Several source files do not expose a local
<TranslationContext.Provider ...>TranslationContext.ProviderTranslationContext.ProviderTranslationContext.Provider-only runtime, so thechatandchatbotkeys insrc/translations/en.jsonhave no corresponding translations in allsrc/translations/*.jsonfiles. Translation guidelines require identical keys across supported languages; add the missing keys and keep each language file alphabetically sorted.🤖 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` around lines 317 - 389, Add the complete chat and chatbot translation key sets from the English dictionary to every supported file under src/translations, providing localized values or the established fallback convention. Ensure all translation dictionaries retain identical key structures and sort their entries alphabetically according to the project’s existing ordering rules.Source: Coding guidelines
src/app/chat/thread/[messageId].tsx-30-37 (1)
30-37: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle the deep-link case where the root message is not cached, and cancel stale thread fetches.
rootreads only frommessagesByChannel. A push notification or a cold start can open this screen with an empty channel cache. The original message then never appears. Fetch the root message when the cache misses.The
getThreadcall also has no cancellation. IfmessageIdchanges, an earlier response can resolve last and replace the newer replies.getThreadaccepts anAbortSignal(src/api/chat/chat.tsLine 111).🛠️ Proposed fix for the fetch race
useEffect(() => { if (!messageId) return; - getThread(messageId, undefined, 50) - .then((response) => setFetchedReplies(response.Data ?? [])) + const controller = new AbortController(); + getThread(messageId, undefined, 50, controller.signal) + .then((response) => { + if (!controller.signal.aborted) setFetchedReplies(response.Data ?? []); + }) .catch((error) => logger.error({ message: 'chat: failed to load thread', context: { error, messageId } })); + return () => controller.abort(); }, [messageId]);🤖 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-loading flow around root and getThread to fetch the root message when channelMessages has no matching ChatMessageId, while preserving the cached-message path. Create an AbortController per messageId effect, pass its signal to getThread, ignore AbortError cancellations, and abort the controller in the effect cleanup so stale responses cannot update fetched replies after messageId changes.src/components/chat/message-actions-sheet.tsx-141-151 (1)
141-151: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHide the pin action for deleted messages.
The other moderator action at Line 160 checks
!isDeleted, but the pin action does not. A moderator can pin a deleted message.♻️ Proposed change
- {isModerator ? ( + {isModerator && !isDeleted ? (🤖 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 action in the message actions sheet to render only when the message is not deleted, matching the existing !isDeleted guard used by the nearby moderator action; preserve the current pin/unpin behavior for active messages.src/components/chat/chat-utils.ts-1-2 (1)
1-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRun ESLint autofix for import order.
The
testjob fails onsimple-import-sort/importsfor this file. Run the autofix before merge.🤖 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 - 2, Run ESLint autofix for import ordering in chat-utils.ts, updating the imports around getAvatarUrl and ChatChannelType to satisfy simple-import-sort/imports without changing their usage.Source: Linters/SAST tools
src/app/(app)/chat.tsx-142-144 (1)
142-144: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd an accessibility label to the FAB.
The FAB contains only a
Plusicon, so screen readers announce no purpose. AddaccessibilityLabel={t('chat.new_conversation')}and register the key in the translation files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/chat.tsx around lines 142 - 144, Add the translated accessibility label to the icon-only FAB in the chat component using the existing `t` function and the `chat.new_conversation` key, then register that key with an appropriate value in every supported translation file.Source: Coding guidelines
src/components/chat/new-conversation-sheet.tsx-81-85 (1)
81-85: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReport the missing
ChatChannelIdcase.If the API returns success without
ChatChannelId, both handlers do nothing. The sheet stays open and the button becomes active again with no message. Show the failure toast in that branch.🛡️ Proposed fix
if (response.Data?.ChatChannelId) { onCreated(response.Data.ChatChannelId); onClose(); - } + } else { + logger.error({ message: 'chat: create DM returned no channel id' }); + useToastStore.getState().showToast('error', t('chat.create_conversation_failed')); + }Apply the same branch to
createGroup.Also applies to: 100-104
🤖 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 81 - 85, Update both the direct-message and group-creation handlers around createDirectMessage and createGroup so a successful response without Data.ChatChannelId shows the existing failure toast. Preserve the current onCreated and onClose behavior when ChatChannelId is present.src/app/(app)/chat.tsx-1-5 (1)
1-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the import order to unblock CI.
The
React Native CI/CD / testjob fails withsimple-import-sort/importsfor this file. The Prettier check also reports theScrollViewprops on Line 122. Run the ESLint and Prettier autofix.🤖 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 1 - 5, Run ESLint autofix to reorder the imports in the chat screen according to simple-import-sort/imports, then run Prettier autofix to format the ScrollView props and the rest of the file; preserve the existing behavior and imports.Source: Pipeline failures
src/app/(app)/chatbot.tsx-19-21 (1)
19-21: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the import order to unblock CI.
useAuthStoreis imported afteruseChatStore, which breakssimple-import-sort/imports. Thetestjob fails on this file. Run the ESLint autofix. The Prettier check also reports thePressableprops on Line 79.🤖 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)/chatbot.tsx around lines 19 - 21, Reorder the imports in the chatbot module to satisfy simple-import-sort/imports, placing useAuthStore before useChatStore as required by the sorter. Also apply the formatter’s autofix to the Pressable props near the existing usage so the Prettier check passes.Source: Linters/SAST tools
src/components/chat/message-bubble.tsx-94-94 (1)
94-94: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe chat feature hardcodes colors instead of using semantic tokens. All three files pass raw hex values to Lucide icons or use raw palette classes. These values do not respond to the color scheme, so contrast degrades in dark mode. Add the required tokens to the Tailwind theme, then read them through a shared helper or
useColorScheme().
src/components/chat/message-bubble.tsx#L94-L94: replace the hex icon colors on Lines 94, 140, 165, 172, 175, and 179 with token-derived values.src/app/(app)/chatbot.tsx#L69-L87: replacebg-purple-50,bg-purple-600,bg-purple-100,text-purple-700, and the#ffffffand#7c3aedicon colors with an assistant semantic token.src/components/sidebar/sidebar-content.tsx#L66-L77: replace the#2563eband#7c3aedicon colors with the same tokens.As per coding guidelines: "Use semantic color tokens from Tailwind config (
primary,secondary,background,typography, etc.), not hardcoded hex values".🤖 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` at line 94, Replace the hardcoded icon colors and raw purple palette classes with semantic, color-scheme-aware tokens: update src/components/chat/message-bubble.tsx lines 94-94 (also the specified icon uses on lines 140, 165, 172, 175, and 179), src/app/(app)/chatbot.tsx lines 69-87, and src/components/sidebar/sidebar-content.tsx lines 66-77. Add the required tokens to the Tailwind theme and consume them through a shared helper or useColorScheme(), reusing the same assistant-related tokens across chatbot.tsx and sidebar-content.tsx; preserve the existing visual roles while removing all listed hex values and raw purple classes.Source: Coding guidelines
src/lib/utils.ts-106-109 (1)
106-109: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEncode the
userIdand declare the return type.The
userIdvalue is interpolated into a query string without encoding. Any&,#, or space in the id corrupts the URL. AddencodeURIComponentand an explicitstringreturn type.🛡️ Proposed fix
/** Absolute URL for a person's avatar image, served by the Resgrid API. */ -export function getAvatarUrl(userId: string) { - return getBaseApiUrl() + '/Avatars/Get?id=' + userId; +export function getAvatarUrl(userId: string): string { + return `${getBaseApiUrl()}/Avatars/Get?id=${encodeURIComponent(userId)}`; }🤖 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/utils.ts` around lines 106 - 109, Update getAvatarUrl to encode userId with encodeURIComponent before placing it in the query string, and declare its return type explicitly as string.src/app/(app)/chatbot.tsx-31-46 (1)
31-46: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard the chatbot cleanup against the channel navigation state.
The chatbot cleanup always sets
activeChannelIdtonull, while other chat screens store their own channel ID in the same global slot. During focus-transition overlap, this cleanup can overwrite a foreign active channel, causing incoming messages for that channel to count as unread. Track the chatbot’s own active channel state and only reset it back tonullon blur.🤖 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)/chatbot.tsx around lines 31 - 46, The chatbot cleanup in the first useFocusEffect must only clear the global active channel when the chatbot still owns it. Track or read the chatbot’s channel ID via chatbotChannelId and conditionally call setActiveChannel(null) only when the current activeChannelId matches that chatbot channel, preserving any channel selected by another chat screen during focus transitions.
🤖 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/api/chat/chat.ts`:
- Line 25: Update the chat API methods in the module to use createApiEndpoint
for mutations and createCachedApiEndpoint for cacheable channel and message
reads instead of direct api calls. After each successful create, update, or
delete operation, call cacheManager.remove() for every affected chat endpoint so
cached lists are invalidated.
- Around line 188-198: Remove the manually specified Content-Type header from
the api.post options in uploadAttachment, leaving the FormData body and other
request options unchanged so React Native supplies the multipart boundary
automatically.
In `@src/app/chat/`[channelId].tsx:
- Around line 1-32: Restore simple-import-sort ordering in all affected imports:
in src/app/chat/[channelId].tsx (lines 1-32), move expo-image into the
external-package group and order keyboard-avoiding-view and useAuthStore
correctly within the alias imports; in
src/components/chat/message-actions-sheet.tsx (lines 1-9), apply the ESLint
autofix for the required group and import order; in
src/app/chat/thread/[messageId].tsx (lines 1-18), place useAuthStore before
useChatStore.
In `@src/app/chat/thread/`[messageId].tsx:
- Around line 62-64: The thread view’s image and GIF controls are rendered
despite inert handlers. Add optional capability flags to MessageComposer and
render each control only when its corresponding handler exists, then configure
the thread’s MessageComposer usage to omit the unsupported image and GIF
controls; update handleSendGif and the related image handler only as needed to
preserve supported text sending.
In `@src/components/chat/chat-utils.ts`:
- Around line 40-44: Update getChannelDisplayName so it no longer hardcodes the
user-visible fallback labels; accept localized Direct Message and Channel labels
as parameters (or move fallback selection to the calling component), ensuring
the displayed text is produced through the caller’s t() translation flow while
preserving the existing channel-name precedence.
- Around line 99-102: Update hasLink to use a non-global URL pattern for
detection, avoiding the shared state caused by URL_REGEX’s g flag. Preserve the
existing false result for empty bodies and ensure linkifySegments continues
using the global pattern without inheriting a mutated lastIndex.
In `@src/components/chat/gif-picker-sheet.tsx`:
- Around line 32-54: Update the GIF search lifecycle around runSearch,
handleChange, and the isOpen effect to pass an AbortSignal to searchGifs and
abort any previous request before starting a new one, clear the debounce timer
during cleanup on unmount or close, and reset query to an empty string when
reopening before loading fresh results. Ensure aborted or stale requests cannot
update GIF state.
In `@src/components/chat/message-bubble.tsx`:
- Line 92: Update both Pressable handlers for map and link segments in the
message bubble to handle rejected Linking.openURL promises with catch logic,
logging through logger.error and providing user feedback. Apply the same
error-handling pattern consistently to the handlers near the map URL and link
segment.
In `@src/components/chat/message-composer.tsx`:
- Around line 43-54: Update handleChange to debounce typing notifications: send
onTyping(true) only when transitioning into typing, reset an idle timer on each
non-empty change, and invoke stopTyping after the idle timeout. Ensure the timer
is cleaned up on unmount and dependencies remain correct, while preserving
immediate stopping for empty input.
In `@src/components/chat/new-conversation-sheet.tsx`:
- Around line 50-60: Update the recipient-loading effect in the isOpen useEffect
to cancel or ignore stale requests when the sheet closes or a newer load starts,
preventing late responses from updating state. In the catch handler, retain
logging and notify the user through useToastStore with the
chat.load_people_failed translation key, adding that key to every translation
file.
In `@src/services/push-notification.ts`:
- Around line 41-52: Update handleChatDeepLink to restrict channelId to the
expected safe channel-ID format instead of accepting arbitrary characters,
reject invalid payloads before routing, and URL-encode the validated identifier
when constructing the chat route. Preserve the existing success and error
behavior for valid and routing-failure cases.
In `@src/stores/chat/store.ts`:
- Around line 376-381: Add bounded retry handling across sendOutboxItem and
drainOutbox: remove failed items for non-retryable 4xx responses except 408/429,
enforce a maximum persisted outbox size, and evict entries older than the
configured CreatedAt age threshold before draining or persisting. Add
exponential backoff between drain attempts, preserving retries for transient
failures and preventing reconnects from issuing a sequential burst of writes.
Update partialize or the outbox update path so these limits apply across
restarts.
- Around line 415-453: Update addReaction and removeReaction to capture each
message’s previous Reactions array before the optimistic mutation, then restore
that array in the corresponding catch block when the API request fails. Preserve
the existing optimistic updates and error logging, and only roll back the
affected message in messagesByChannel.
- Around line 749-767: The chat store reset currently leaves the persisted
outbox intact, allowing stale messages to drain after logout. Update the logout
reset flow in resetAllStores to invoke useChatStore.getState().reset(), or
ensure the existing chat reset implementation clears the outbox alongside other
chat state before handleChatConnected can drain it.
In `@src/stores/signalr/signalr-store.ts`:
- Around line 379-442: Update connectChatHub to register a listener for
SignalRService.HUB_DISCONNECTED_EVENT:${Env.CHAT_HUB_NAME}, storing it in
chatHubHandlers alongside the existing lifecycle handler and setting
isChatHubConnected to false when invoked. Keep reconnect or resynchronization
behavior out of this change.
---
Minor comments:
In `@src/app/`(app)/chat.tsx:
- Around line 142-144: Add the translated accessibility label to the icon-only
FAB in the chat component using the existing `t` function and the
`chat.new_conversation` key, then register that key with an appropriate value in
every supported translation file.
- Around line 1-5: Run ESLint autofix to reorder the imports in the chat screen
according to simple-import-sort/imports, then run Prettier autofix to format the
ScrollView props and the rest of the file; preserve the existing behavior and
imports.
In `@src/app/`(app)/chatbot.tsx:
- Around line 19-21: Reorder the imports in the chatbot module to satisfy
simple-import-sort/imports, placing useAuthStore before useChatStore as required
by the sorter. Also apply the formatter’s autofix to the Pressable props near
the existing usage so the Prettier check passes.
- Around line 31-46: The chatbot cleanup in the first useFocusEffect must only
clear the global active channel when the chatbot still owns it. Track or read
the chatbot’s channel ID via chatbotChannelId and conditionally call
setActiveChannel(null) only when the current activeChannelId matches that
chatbot channel, preserving any channel selected by another chat screen during
focus transitions.
In `@src/app/chat/`[channelId].tsx:
- Line 216: Update the AckBanner onAcknowledge handler and the async onCopy
handler passed to MessageActionsSheet so each handles promise rejection instead
of returning an unobserved promise; catch failures and report them through
useToastStore while preserving their existing successful behavior.
- Around line 145-168: The handleSendImage callback must report an upload
failure when no server-confirmed message is found. Add an else branch to the
existing if (sent) check that calls useToastStore.getState().showToast('error',
t('chat.attachment_failed')), preserving the current upload and catch behavior.
- Around line 82-88: Update the presence-loading useEffect around getPresence so
each effect run creates a cancellation or active-state guard, ignores results
from superseded requests and unmounted components, and cleans up that guard when
dependencies change or the component unmounts. Preserve the existing member
filtering and setPresenceIds behavior for the latest active request.
- Around line 91-95: Update the read-receipt effect in the chat channel
component to depend on the newest message identity or timestamp rather than
inverted.length, so it reruns when the latest message changes even if the list
size is unchanged. Preserve the existing channelId and non-empty-list guards and
markChannelRead behavior.
In `@src/app/chat/thread/`[messageId].tsx:
- Around line 30-37: Update the thread-loading flow around root and getThread to
fetch the root message when channelMessages has no matching ChatMessageId, while
preserving the cached-message path. Create an AbortController per messageId
effect, pass its signal to getThread, ignore AbortError cancellations, and abort
the controller in the effect cleanup so stale responses cannot update fetched
replies after messageId changes.
In `@src/components/chat/chat-utils.ts`:
- Around line 1-2: Run ESLint autofix for import ordering in chat-utils.ts,
updating the imports around getAvatarUrl and ChatChannelType to satisfy
simple-import-sort/imports without changing their usage.
In `@src/components/chat/message-actions-sheet.tsx`:
- Line 101: Define a shared color map derived from semantic Tailwind tokens via
useColorScheme(), then replace all hardcoded icon colors: in
src/components/chat/message-actions-sheet.tsx:101-101 update all six action
icons; in src/components/chat/ack-banner.tsx:26-26 use the error token matching
adjacent text-error-700; in src/components/chat/message-composer.tsx:101-101
replace gray, red, and white values; in
src/components/chat/typing-indicator.tsx:9-9 use a token-derived default; and in
src/app/chat/[channelId].tsx:212-212 update the green, gray, and blue values at
both referenced icon locations.
- Around line 141-151: Update the moderator pin action in the message actions
sheet to render only when the message is not deleted, matching the existing
!isDeleted guard used by the nearby moderator action; preserve the current
pin/unpin behavior for active messages.
In `@src/components/chat/message-bubble.tsx`:
- Line 94: Replace the hardcoded icon colors and raw purple palette classes with
semantic, color-scheme-aware tokens: update
src/components/chat/message-bubble.tsx lines 94-94 (also the specified icon uses
on lines 140, 165, 172, 175, and 179), src/app/(app)/chatbot.tsx lines 69-87,
and src/components/sidebar/sidebar-content.tsx lines 66-77. Add the required
tokens to the Tailwind theme and consume them through a shared helper or
useColorScheme(), reusing the same assistant-related tokens across chatbot.tsx
and sidebar-content.tsx; preserve the existing visual roles while removing all
listed hex values and raw purple classes.
In `@src/components/chat/message-composer.tsx`:
- Around line 126-128: Add switch semantics to the urgent-toggle Pressable in
the message composer by setting accessibilityRole to "switch" and
accessibilityState.checked to the current urgent value. Keep the existing label,
toggle handler, and visual state unchanged.
- Around line 77-79: Update the image-selection and location-capture catch
blocks in the message composer to show a user-facing toast through useToastStore
in addition to logging the error, matching the existing denied-permission
feedback pattern. Add the corresponding translation keys to en.json and every
other language file, and use those keys for both failure messages.
In `@src/components/chat/new-conversation-sheet.tsx`:
- Around line 81-85: Update both the direct-message and group-creation handlers
around createDirectMessage and createGroup so a successful response without
Data.ChatChannelId shows the existing failure toast. Preserve the current
onCreated and onClose behavior when ChatChannelId is present.
In `@src/components/chat/typing-indicator.tsx`:
- Line 15: Run Prettier on the Animated.sequence blocks in typing-indicator.tsx,
message-composer.tsx, chat/[channelId].tsx, and chat/thread/[messageId].tsx so
each sequence is formatted to fit the configured 220-character print width and
CI’s test check passes.
In `@src/lib/utils.ts`:
- Around line 106-109: Update getAvatarUrl to encode userId with
encodeURIComponent before placing it in the query string, and declare its return
type explicitly as string.
In `@src/stores/chat/store.ts`:
- Around line 1-23: In src/stores/chat/store.ts lines 1-23, run import sorting
and change Env to use the `@env` alias; in src/models/v4/chat/index.ts lines 1-6,
alphabetize all six export statements. Run the repository lint task with --fix
to apply both changes.
- Around line 564-577: The chatbot send flow around the typing state update must
prevent chatbotTyping from remaining true when no reply arrives. Add a bounded
timeout after setting chatbotTyping in the relevant store action, clear the flag
when it expires, and retain the timeout handle so reply handling in
handleChatbotMessageReceived can cancel it before clearing the indicator.
- Around line 172-185: Update the findIndex predicate in upsertMessage to a
single line so it conforms to the configured Prettier print width, preserving
its existing matching logic.
- Around line 473-491: Update the catch block in markChannelRead to delete the
channel’s entry from lastMarkedSeq when chatApi.markRead fails, before or
alongside the existing debug log, so a later call can retry the same sequence.
- Around line 619-627: Update the state update around upsertMessage in
chatMessageReceived to determine whether msg is already present by ChatMessageId
or ClientMessageId before insertion. Increment UnreadCount only for a new
message when the channel is neither active nor owned by the sender; preserve the
existing count for duplicate deliveries and keep upsertMessage’s deduplication
behavior.
In `@src/stores/signalr/signalr-store.ts`:
- Around line 451-463: Update the catch block in disconnectChatHub to set
isChatHubConnected to false when disconnectFromHub fails, alongside the existing
error state update, so a later connectChatHub call is not blocked by stale
connection state.
In `@src/translations/en.json`:
- Line 332: Update the count-based translation keys thread_replies, are_typing,
ack_pending_count, and create_group_with in en.json to use i18next _one and
_other variants, preserving the singular and plural wording for count values.
Ensure callers, including the AckBanner branch, rely on the pluralized
translation behavior without duplicating or overriding the singular case.
- Around line 317-389: Add the complete chat and chatbot translation key sets
from the English dictionary to every supported file under src/translations,
providing localized values or the established fallback convention. Ensure all
translation dictionaries retain identical key structures and sort their entries
alphabetically according to the project’s existing ordering rules.
---
Nitpick comments:
In `@src/api/chat/chatbot.ts`:
- Line 3: Update the chatbot API module’s client import to use the
`@/api/common/client` alias, then wrap its three API callers with the appropriate
createApiEndpoint or createCachedApiEndpoint helper and invoke the generated
endpoint methods instead of calling the client directly.
In `@src/app/`(app)/chat.tsx:
- Around line 122-140: Replace the channel sections inside the ScrollView with
one virtualized sectioned FlashList from `@shopify/flash-list`, preserving the
existing empty state, section titles, ordering, and openChannel behavior.
Configure the list with removeClippedSubviews, maxToRenderPerBatch, and
windowSize, and retain the bottom spacing through the list footer rather than a
separate trailing Box.
- Around line 27-42: Move the Leading component definition out of the ChannelRow
render scope to preserve its identity across renders, defining it at module
scope with channel and isDm passed as props. Update the current <Leading />
usage to render the resulting leading content through the existing leading
value, while preserving the avatar, incident, chatbot, and default icon
behavior.
- Line 96: Memoize the result of groupChannels in the chat component using
useMemo with channels as its sole dependency, and add useMemo to the React
imports. Preserve the existing grouped value and grouping behavior while
avoiding recomputation on unrelated renders such as fabOpen or newMode changes.
In `@src/app/`(app)/chatbot.tsx:
- Around line 57-62: Stabilize the chatbot list callbacks by moving the no-op
handlers used by renderItem outside the component and hoisting keyExtractor so
they are not recreated during renders. Update the relevant FlatList
configuration to enable removeClippedSubviews and set maxToRenderPerBatch and
windowSize according to the performance guidelines, while preserving existing
item rendering and key behavior.
In `@src/app/chat/`[channelId].tsx:
- Around line 106-109: Update the typing state flow around typingNames so
expired entries are re-evaluated even when no store update occurs. Add an
interval-driven refresh while typing is non-empty, clean up the interval when
typing becomes empty or the component unmounts, and preserve the existing expiry
filtering and fallback display-name behavior.
- Around line 224-232: Optimize both FlatList instances in
src/app/chat/[channelId].tsx lines 224-232 and
src/app/chat/thread/[messageId].tsx lines 113-119 by adding
removeClippedSubviews, maxToRenderPerBatch, and windowSize. In the channel list,
hoist the inline onEndReached handler into a useCallback while preserving its
existing channelId and loadOlderMessages behavior.
In `@src/app/chat/thread/`[messageId].tsx:
- Around line 81-97: Extract the inline onToggleReaction callback from
renderItem into a useCallback with channelId as a dependency, preserving the
existing addReaction/removeReaction behavior and early return when channelId is
unavailable. Pass the stable handler reference to MessageBubble from renderItem,
and update renderItem’s dependencies accordingly.
In `@src/components/chat/ack-banner.tsx`:
- Line 17: Update the AckBanner component declaration to use
React.FC<AckBannerProps> and close the assigned component expression with `};`.
Apply the same React.FC<Props> typing convention to the other new chat
components referenced by the review.
In `@src/components/chat/chat-utils.ts`:
- Around line 122-134: Update formatShortTime to use date-fns format instead of
the native toLocaleTimeString and toLocaleDateString calls, deriving the
date-fns locale from the active i18next locale. Preserve the existing
invalid-date, future-date, same-day time, and date-label behavior while ensuring
both outputs follow the app language.
In `@src/components/chat/gif-picker-sheet.tsx`:
- Around line 82-98: Replace the GIF grid’s ScrollView and mapped Pressable
layout with a FlashList from `@shopify/flash-list`, using gifs as its data source
and preserving the two-column layout, image styling, selection callback, and
close behavior. Configure the list for efficient rendering while retaining the
existing spacing and maximum-height presentation.
In `@src/components/chat/message-bubble.tsx`:
- Around line 18-30: Update MessageBubble to use React.FC<MessageBubbleProps>
typing and wrap the component export with React.memo, closing the memoized
component at the end of its body while preserving its existing props and
rendering behavior.
In `@src/components/chat/message-composer.tsx`:
- Line 98: Replace the direct Platform.OS check in the message composer’s Box
style with the shared isIOS utility from src/lib/platform.ts, importing it as
needed and preserving the existing iOS padding behavior.
- Line 89: Update the location retrieval around Location.getCurrentPositionAsync
to specify an explicit LocationAccuracy and enforce a timeout before sending.
When the request times out, fall back to Location.getLastKnownPositionAsync({}),
preserving the existing position handling for successful results.
In `@src/components/chat/new-conversation-sheet.tsx`:
- Around line 146-176: Replace the recipient ScrollView and filtered.map
rendering with a Shopify FlashList configured for the filtered recipients, and
extract the Pressable row rendering into a stable renderer outside the component
render body. Preserve the existing selection, direct-message, disabled, avatar,
and group-check behavior while supplying stable keys and appropriate estimated
item sizing.
In `@src/components/chat/typing-indicator.tsx`:
- Around line 12-24: Update the animation construction in the useEffect hook so
each dot’s index-based Animated.delay occurs once outside Animated.loop, while
the opacity timing sequence remains looped. Preserve the existing staggered
start and cleanup behavior in animations.forEach.
In `@src/models/v4/chat/chatInputs.ts`:
- Around line 25-27: Replace the bare number fields in
SetNotificationPreferenceInput, ChatMentionInput, SendChatMessageInput, and
FlagMessageInput with their corresponding existing enums:
ChatNotificationPreference, ChatMentionType, ChatMessageType,
ChatMessagePriority, and ChatFlagReason.
In `@src/models/v4/chat/outbox.ts`:
- Around line 21-24: Update sendMessage to populate
ChatOutboxItem.SenderDisplayName when constructing the outbox item, using the
current sender’s display name so optimistic messages preserve it; keep the
existing SenderUserId assignment unchanged.
In `@src/services/push-notification.ts`:
- Around line 278-286: Add tests in push-notification.test.ts covering the
handleChatDeepLink branch in the setTimeout callback: verify t: and g: event
codes route via router.push, unmatched codes fall through to showModalForData,
and a router.push failure is handled while still showing the modal.
In `@src/services/signalr.service.ts`:
- Line 595: Update SignalRService.invoke to use a generic return type
representing the hub response, and return the result from connection.invoke
instead of discarding it. Preserve compatibility for existing void-style callers
by keeping the generic usable when no response value is needed.
In `@src/stores/chat/store.ts`:
- Around line 383-410: Update the catch blocks in editMessage, deleteMessage,
moderatorDeleteMessage, acknowledgeMessage, togglePin, and flagMessage to show
an error toast via useToastStore.getState().showToast using the existing
translation helper t, while preserving the current logger.error calls. Ensure
each failed user action provides appropriate localized feedback, especially
acknowledgement failures.
- Line 52: Bound the in-memory cache managed through messagesByChannel to
prevent unbounded growth across visited channels. Update loadOlderMessages and
upsertMessage to retain only a defined per-channel window, preserving the active
channel’s loaded pages, and evict cached entries for channels that are no longer
open. Ensure eviction and trimming maintain the newest messages and existing
ordering behavior.
🪄 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: 61447cff-0674-44a0-84bf-34acb1a6ccf7
📒 Files selected for processing (31)
env.jssrc/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/sidebar-content.tsxsrc/hooks/use-signalr-lifecycle.tssrc/lib/utils.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
| } from '@/models/v4/chat'; | ||
| import useAuthStore from '@/stores/auth/store'; | ||
|
|
||
| import { api } from '../common/client'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the createApiEndpoint/createCachedApiEndpoint pattern and cacheManager usage elsewhere.
set -euo pipefail
echo "--- common client / endpoint factory ---"
fd -t f . src/api/common
echo "--- other API modules using createApiEndpoint/createCachedApiEndpoint ---"
rg -n -C3 'createApiEndpoint|createCachedApiEndpoint' src/api
echo "--- cacheManager.remove usage in other API modules ---"
rg -n -C3 'cacheManager\.remove' src/apiRepository: Resgrid/Unit
Length of output: 39843
🏁 Script executed:
#!/bin/bash
# Inspect src/api/chat/chat.ts structure, endpoint definitions, and raw api call/mutation/cache usage.
set -euo pipefail
echo "--- file size ---"
wc -l src/api/chat/chat.ts
echo "--- outline ---"
ast-grep outline src/api/chat/chat.ts --view compact || true
echo "--- imports and endpoint/client lines ---"
sed -n '1,80p' src/api/chat/chat.ts
echo "--- direct api calls and cacheManager references ---"
rg -n -C3 '(api\.(get|post|put|delete)<|cacheManager\.remove|uploadAttachment|const .*Api|\.delete\(|\.put\(|\.post\()' src/api/chat/chat.ts
echo "--- mutation definitions ---"
sed -n '180,280p' src/api/chat/chat.tsRepository: Resgrid/Unit
Length of output: 15524
🌐 Web query:
axios multipart/form-data React Native set Content-Type header FormData boundary
💡 Result:
When using FormData with Axios in React Native, you should not manually set the Content-Type header [1][2]. The React Native runtime environment, specifically its networking layer, is responsible for generating the multipart boundary string required for successful multipart/form-data requests [3][4]. If you manually set the Content-Type header to multipart/form-data, you overwrite the internal boundary generation process, which often leads to errors—particularly on Android, where the native networking layer strictly requires the boundary to be present in the header [5][3][4]. Best Practices: 1. Do not set headers manually: Simply pass your FormData object directly to the data property of your Axios request [1][2]. 2. Ensure you are using a recent version of Axios: Updates (notably those around May 2026) have improved how Axios handles FormData in React Native by ensuring default headers like application/x-www-form-urlencoded are properly cleared when FormData is detected, allowing the React Native runtime to set the correct Content-Type with the boundary [6][5]. 3. Troubleshooting: If you encounter issues where the request is sent with an incorrect header (e.g., application/x-www-form-urlencoded) despite using FormData, ensure your environment's FormData is being correctly identified by Axios [5]. As a last-resort workaround, you can explicitly unset the header in your request config to allow the runtime to take over: javascript // Only use this if you are encountering issues with automatic header resolution axios.post(url, formData, { headers: { 'Content-Type': false // This prevents axios from applying a default type } }); Setting the header to false instructs Axios to omit the Content-Type header entirely, which forces the React Native NetworkingModule to handle the request correctly [5].
Citations:
- 1: https://github.com/axios/axios/blob/e435384f/docs/pages/advanced/multipart-form-data-format.md
- 2: fix: remove forced multipart/form-data placeholder header in postForm/putForm/patchForm axios/axios#10980
- 3: Drop forced bare
multipart/form-dataplaceholder header inpostForm/putForm/patchForm(breaks RN Android) axios/axios#10886 - 4: fix: remove forced multipart header to fix Android postForm/patchForm… axios/axios#7243
- 5: FormData requests crash on RN Android due to incorrect default Content-Type axios/axios#10895
- 6: fix: clear RN FormData content type axios/axios#10898
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- common api client snippet ---"
sed -n '1,180p' src/api/common/client.tsx
echo "--- cached api client snippet ---"
sed -n '1,120p' src/api/common/cached-client.ts
echo "--- cache remover implementation ---"
fd -t f cache-manager src | xargs -r -I{} sh -c 'echo ";; {}"; sed -n "1,220p" "{}"'Repository: Resgrid/Unit
Length of output: 12790
Use the endpoint factory for all chat APIs and invalidate chat caches after mutations.
createApiEndpoint/createCachedApiEndpoint provide the shared path wrapper used by other src/api/** modules, while direct api.get/api.post/api.put/api.delete calls bypass that pattern. Cacheable reads such as channel/message lists should use createCachedApiEndpoint; create/update/delete chat operations should call cacheManager.remove() for the affected endpoints after a successful mutation.
🤖 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` at line 25, Update the chat API methods in the module
to use createApiEndpoint for mutations and createCachedApiEndpoint for cacheable
channel and message reads instead of direct api calls. After each successful
create, update, or delete operation, call cacheManager.remove() for every
affected chat endpoint so cached lists are invalidated.
Source: Coding guidelines
| const handleSendGif = useCallback(() => { | ||
| // GIFs in threads are sent as text-less messages via the composer's gif flow; kept minimal here. | ||
| }, []); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The image and GIF controls are inert in the thread view.
onSendImage and onOpenGif resolve to no-ops, but MessageComposer still renders both buttons. The user can grant the photo permission, select an image, and receive no result and no error. Add optional capability flags to MessageComposer so the thread view can hide the unsupported controls, or implement both handlers.
♻️ Proposed change for the composer props
interface MessageComposerProps {
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;
onTyping: (isTyping: boolean) => void;Then render each control only when its handler exists.
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, The thread view’s
image and GIF controls are rendered despite inert handlers. Add optional
capability flags to MessageComposer and render each control only when its
corresponding handler exists, then configure the thread’s MessageComposer usage
to omit the unsupported image and GIF controls; update handleSendGif and the
related image handler only as needed to preserve supported text sending.
| export function handleChatDeepLink(eventCode: string): boolean { | ||
| const match = /^([tg]):(.+)$/.exec(eventCode); | ||
| if (!match) return false; | ||
| const channelId = match[2]; | ||
| try { | ||
| router.push(`/chat/${channelId}`); | ||
| return true; | ||
| } catch (error) { | ||
| logger.error({ message: 'Failed to deep-link to chat channel', context: { error, eventCode } }); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate channelId before you build the route.
(.+) accepts any characters, including / and ... The value comes from a push payload and goes straight into a route path. A payload such as t:../settings navigates the user to an unrelated screen. Restrict the captured value to the expected channel id format and encode it.
🔒️ Proposed fix
- const match = /^([tg]):(.+)$/.exec(eventCode);
+ const match = /^([tg]):([A-Za-z0-9-]{1,64})$/.exec(eventCode);
if (!match) return false;
const channelId = match[2];
try {
- router.push(`/chat/${channelId}`);
+ router.push(`/chat/${encodeURIComponent(channelId)}`);
return true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function handleChatDeepLink(eventCode: string): boolean { | |
| const match = /^([tg]):(.+)$/.exec(eventCode); | |
| if (!match) return false; | |
| const channelId = match[2]; | |
| try { | |
| router.push(`/chat/${channelId}`); | |
| return true; | |
| } catch (error) { | |
| logger.error({ message: 'Failed to deep-link to chat channel', context: { error, eventCode } }); | |
| return false; | |
| } | |
| } | |
| export function handleChatDeepLink(eventCode: string): boolean { | |
| const match = /^([tg]):([A-Za-z0-9-]{1,64})$/.exec(eventCode); | |
| if (!match) return false; | |
| const channelId = match[2]; | |
| try { | |
| router.push(`/chat/${encodeURIComponent(channelId)}`); | |
| return true; | |
| } catch (error) { | |
| logger.error({ message: 'Failed to deep-link to chat channel', context: { error, eventCode } }); | |
| return false; | |
| } | |
| } |
🧰 Tools
🪛 OpenGrep (1.26.0)
[ERROR] 42-42: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 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 41 - 52, Update
handleChatDeepLink to restrict channelId to the expected safe channel-ID format
instead of accepting arbitrary characters, reject invalid payloads before
routing, and URL-encode the validated identifier when constructing the chat
route. Preserve the existing success and error behavior for valid and
routing-failure cases.
| drainOutbox: async () => { | ||
| const items = [...get().outbox]; | ||
| for (const item of items) { | ||
| await sendOutboxItem(item, set, get); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
The persisted outbox has no size cap, no age limit, and no backoff.
sendOutboxItem removes an item only on success. On failure the item stays in outbox, which partialize persists to MMKV. Three consequences:
- A permanently rejected message (for example, HTTP 403 on a channel the unit left, or 400 on an invalid body) is retried on every
handleChatConnectedand never removed. The queue grows without bound across app restarts. drainOutboxawaits each item in sequence with no delay. After a long offline period the store issues a burst of writes on reconnect.- A non-retryable status code is treated the same as a network error.
Consider: drop items on 4xx responses (other than 408/429), cap the outbox length, evict items older than a threshold using CreatedAt, and add exponential backoff between drain attempts.
As per coding guidelines: "Services implement retry logic with exponential backoff where appropriate".
Also applies to: 785-820
🤖 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 376 - 381, Add bounded retry handling
across sendOutboxItem and drainOutbox: remove failed items for non-retryable 4xx
responses except 408/429, enforce a maximum persisted outbox size, and evict
entries older than the configured CreatedAt age threshold before draining or
persisting. Add exponential backoff between drain attempts, preserving retries
for transient failures and preventing reconnects from issuing a sequential burst
of writes. Update partialize or the outbox update path so these limits apply
across restarts.
Source: Coding guidelines
| connectChatHub: async () => { | ||
| try { | ||
| if (get().isChatHubConnected) { | ||
| return; | ||
| } | ||
|
|
||
| const eventingUrl = useCoreStore.getState().config?.EventingUrl; | ||
| if (!eventingUrl) { | ||
| logger.warn({ message: 'EventingUrl not available for chat hub, skipping connection' }); | ||
| return; | ||
| } | ||
|
|
||
| // Ensure any previous handlers are cleaned up before registering new ones. | ||
| unregisterChatHubHandlers(); | ||
|
|
||
| await signalRService.connectToHubWithEventingUrl({ | ||
| name: Env.CHAT_HUB_NAME, | ||
| eventingUrl, | ||
| hubName: Env.CHAT_HUB_NAME, | ||
| methods: CHAT_HUB_METHODS, | ||
| }); | ||
|
|
||
| const chat = useChatStore.getState(); | ||
| const handlerMap: Record<string, (raw: unknown) => void> = { | ||
| chatMessageReceived: chat.handleMessageReceived, | ||
| chatMessageEdited: chat.handleMessageEdited, | ||
| chatMessageDeleted: chat.handleMessageDeleted, | ||
| chatReactionUpdated: chat.handleReactionUpdated, | ||
| chatReceiptUpdated: chat.handleReceiptUpdated, | ||
| chatChannelUpdated: chat.handleChannelUpdated, | ||
| chatChannelProvisioned: chat.handleChannelProvisioned, | ||
| chatModerationApplied: chat.handleModerationApplied, | ||
| chatMessageAckRequired: chat.handleAckRequired, | ||
| chatThreadUpdated: chat.handleThreadUpdated, | ||
| chatbotMessageReceived: chat.handleChatbotMessageReceived, | ||
| chatbotTyping: chat.handleChatbotTyping, | ||
| chatTyping: chat.handleTyping, | ||
| chatPresenceChanged: chat.handlePresenceChanged, | ||
| }; | ||
|
|
||
| Object.entries(handlerMap).forEach(([event, handler]) => { | ||
| const wrapped = (data: unknown) => handler(data); | ||
| chatHubHandlers[event] = wrapped; | ||
| signalRService.on(event, wrapped); | ||
| }); | ||
|
|
||
| const onChatConnected = () => { | ||
| logger.info({ message: 'Connected to chat SignalR hub' }); | ||
| set({ isChatHubConnected: true, error: null }); | ||
| useChatStore.getState().handleChatConnected(); | ||
| }; | ||
| chatHubHandlers.onChatConnected = onChatConnected; | ||
| signalRService.on('onChatConnected', onChatConnected); | ||
|
|
||
| // Announce chat presence to the hub, then begin the periodic heartbeat. | ||
| await signalRService.invoke(Env.CHAT_HUB_NAME, 'Connect'); | ||
| set({ isChatHubConnected: true }); | ||
|
|
||
| stopChatHeartbeat(); | ||
| chatHeartbeatTimer = setInterval(() => { | ||
| signalRService.invoke(Env.CHAT_HUB_NAME, 'Heartbeat').catch(() => { | ||
| // Heartbeat is best-effort; ignore transient failures. | ||
| }); | ||
| }, CHAT_HEARTBEAT_INTERVAL_MS); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm CHAT_HUB_NAME is declared in the env schema and check how sibling hubs bind lifecycle events.
rg -n 'CHAT_HUB_NAME' env.js src/lib/env.js src/lib/env.ts 2>/dev/null | head -20
fd -t f 'env.js' -x rg -n 'HUB_NAME' {} | head -30
rg -n -C2 'HUB_RECONNECTED_EVENT|HUB_DISCONNECTED_EVENT' src/stores/signalr/signalr-store.ts | head -40Repository: Resgrid/Unit
Length of output: 1719
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== signalr-store outline =="
ast-grep outline src/stores/signalr/signalr-store.ts --match connectChatHub --view expanded || true
echo
echo "== relevant signalr-store sections =="
sed -n '1,90p' src/stores/signalr/signalr-store.ts
sed -n '140,230p' src/stores/signalr/signalr-store.ts
sed -n '220,290p' src/stores/signalr/signalr-store.ts
sed -n '330,470p' src/stores/signalr/signalr-store.ts
echo
echo "== signalRService event constants and on/invoke definitions =="
rg -n -C3 'HUB_DISCONNECTED_EVENT|HUB_RECONNECTING_EVENT|HUB_RECONNECTED_EVENT|on\(|invoke\(' src/services src/stores src/lib | head -220Repository: Resgrid/Unit
Length of output: 30971
Register the chat hub lifecycle listeners so connection drops do not leave isChatHubConnected stale.
connectUpdateHub registers a hub-scoped HUB_DISCONNECTED_EVENT listener, but connectChatHub does not. If the chat connection drops while the app is foregrounded, the guard at line 381 prevents later connectChatHub() calls from running, while the heartbeat errors are swallowed.
Subscribe to SignalRService.HUB_DISCONNECTED_EVENT:${Env.CHAT_HUB_NAME} and clear isChatHubConnected. Treat reconnect/resync handling as a separate follow-up once the lifecycle listener flow is in place.
🤖 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 379 - 442, Update
connectChatHub to register a listener for
SignalRService.HUB_DISCONNECTED_EVENT:${Env.CHAT_HUB_NAME}, storing it in
chatHubHandlers alongside the existing lifecycle handler and setting
isChatHubConnected to false when invoked. Keep reconnect or resynchronization
behavior out of this change.
| <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.
Performance degradation occurs due to inline arrow functions inside JSX props, which create new functions on every render. Move function definitions outside the render method or wrap them in useCallback.
Also found in:
src/app/(app)/chat.tsx:115-115src/app/(app)/chat.tsx:120-120src/app/(app)/chat.tsx:124-124src/app/(app)/chat.tsx:142-142src/app/(app)/chat.tsx:147-147src/app/(app)/chat.tsx:154-154src/app/(app)/chat.tsx:163-163src/app/(app)/chat.tsx:172-172src/app/(app)/chat.tsx:186-186src/app/(app)/chat.tsx:187-187src/app/(app)/chatbot.tsx:59-59src/app/(app)/chatbot.tsx:81-81src/app/chat/[channelId].tsx:196-196src/app/chat/[channelId].tsx:216-216src/app/chat/[channelId].tsx:229-229src/app/chat/[channelId].tsx:249-249src/app/chat/[channelId].tsx:250-250src/app/chat/[channelId].tsx:255-255src/app/chat/[channelId].tsx:260-260src/app/chat/[channelId].tsx:263-263src/app/chat/[channelId].tsx:265-265src/app/chat/[channelId].tsx:269-269src/app/chat/[channelId].tsx:273-273src/app/chat/[channelId].tsx:274-274src/app/chat/[channelId].tsx:275-275src/app/chat/[channelId].tsx:276-276src/app/chat/[channelId].tsx:280-280src/app/chat/[channelId].tsx:293-293src/app/chat/[channelId].tsx:307-307src/app/chat/thread/[messageId].tsx:88-88src/app/chat/thread/[messageId].tsx:89-89src/app/chat/thread/[messageId].tsx:107-107src/app/chat/thread/[messageId].tsx:123-123src/app/chat/thread/[messageId].tsx:126-126src/components/chat/ack-banner.tsx:32-32src/components/chat/gif-picker-sheet.tsx:89-89src/components/chat/message-actions-sheet.tsx:67-67src/components/chat/message-actions-sheet.tsx:84-84src/components/chat/message-actions-sheet.tsx:96-96src/components/chat/message-actions-sheet.tsx:107-107src/components/chat/message-actions-sheet.tsx:119-119src/components/chat/message-actions-sheet.tsx:131-131src/components/chat/message-actions-sheet.tsx:143-143src/components/chat/message-actions-sheet.tsx:154-154src/components/chat/message-actions-sheet.tsx:162-162src/components/chat/message-bubble.tsx:73-73src/components/chat/message-bubble.tsx:92-92src/components/chat/message-bubble.tsx:109-109src/components/chat/message-bubble.tsx:136-136src/components/chat/message-bubble.tsx:151-151src/components/chat/message-bubble.tsx:163-163src/components/chat/message-bubble.tsx:177-177src/components/chat/message-composer.tsx:100-100src/components/chat/message-composer.tsx:126-126src/components/chat/message-composer.tsx:142-142src/components/chat/message-composer.tsx:153-153src/components/chat/new-conversation-sheet.tsx:154-154
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/(app)/chat.tsx:
Line 74:
Performance degradation occurs due to inline arrow functions inside JSX props, which create new functions on every render. Move function definitions outside the render method or wrap them in `useCallback`.
**Also found in:**
- `src/app/(app)/chat.tsx:115-115`
- `src/app/(app)/chat.tsx:120-120`
- `src/app/(app)/chat.tsx:124-124`
- `src/app/(app)/chat.tsx:142-142`
- `src/app/(app)/chat.tsx:147-147`
- `src/app/(app)/chat.tsx:154-154`
- `src/app/(app)/chat.tsx:163-163`
- `src/app/(app)/chat.tsx:172-172`
- `src/app/(app)/chat.tsx:186-186`
- `src/app/(app)/chat.tsx:187-187`
- `src/app/(app)/chatbot.tsx:59-59`
- `src/app/(app)/chatbot.tsx:81-81`
- `src/app/chat/[channelId].tsx:196-196`
- `src/app/chat/[channelId].tsx:216-216`
- `src/app/chat/[channelId].tsx:229-229`
- `src/app/chat/[channelId].tsx:249-249`
- `src/app/chat/[channelId].tsx:250-250`
- `src/app/chat/[channelId].tsx:255-255`
- `src/app/chat/[channelId].tsx:260-260`
- `src/app/chat/[channelId].tsx:263-263`
- `src/app/chat/[channelId].tsx:265-265`
- `src/app/chat/[channelId].tsx:269-269`
- `src/app/chat/[channelId].tsx:273-273`
- `src/app/chat/[channelId].tsx:274-274`
- `src/app/chat/[channelId].tsx:275-275`
- `src/app/chat/[channelId].tsx:276-276`
- `src/app/chat/[channelId].tsx:280-280`
- `src/app/chat/[channelId].tsx:293-293`
- `src/app/chat/[channelId].tsx:307-307`
- `src/app/chat/thread/[messageId].tsx:88-88`
- `src/app/chat/thread/[messageId].tsx:89-89`
- `src/app/chat/thread/[messageId].tsx:107-107`
- `src/app/chat/thread/[messageId].tsx:123-123`
- `src/app/chat/thread/[messageId].tsx:126-126`
- `src/components/chat/ack-banner.tsx:32-32`
- `src/components/chat/gif-picker-sheet.tsx:89-89`
- `src/components/chat/message-actions-sheet.tsx:67-67`
- `src/components/chat/message-actions-sheet.tsx:84-84`
- `src/components/chat/message-actions-sheet.tsx:96-96`
- `src/components/chat/message-actions-sheet.tsx:107-107`
- `src/components/chat/message-actions-sheet.tsx:119-119`
- `src/components/chat/message-actions-sheet.tsx:131-131`
- `src/components/chat/message-actions-sheet.tsx:143-143`
- `src/components/chat/message-actions-sheet.tsx:154-154`
- `src/components/chat/message-actions-sheet.tsx:162-162`
- `src/components/chat/message-bubble.tsx:73-73`
- `src/components/chat/message-bubble.tsx:92-92`
- `src/components/chat/message-bubble.tsx:109-109`
- `src/components/chat/message-bubble.tsx:136-136`
- `src/components/chat/message-bubble.tsx:151-151`
- `src/components/chat/message-bubble.tsx:163-163`
- `src/components/chat/message-bubble.tsx:177-177`
- `src/components/chat/message-composer.tsx:100-100`
- `src/components/chat/message-composer.tsx:126-126`
- `src/components/chat/message-composer.tsx:142-142`
- `src/components/chat/message-composer.tsx:153-153`
- `src/components/chat/new-conversation-sheet.tsx:154-154`
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const trimmed = text.trim(); | ||
| if (!trimmed) return; | ||
| setText(''); | ||
| void useChatStore.getState().sendChatbotMessage(trimmed); |
There was a problem hiding this comment.
Unhandled promise rejection occurs when async store action sendChatbotMessage() is invoked with void and no rejection handler. Add a .catch() handler or wrap the call in try/catch to log or surface network failures.
Also found in:
src/app/chat/thread/[messageId].tsx:57-57src/app/(app)/chatbot.tsx:34-34src/app/(app)/chatbot.tsx:81-81src/app/chat/thread/[messageId].tsx:69-76src/app/chat/thread/[messageId].tsx:91-91src/app/(app)/chat.tsx:92-92src/app/chat/thread/[messageId].tsx:92-92src/app/chat/[channelId].tsx:115-115src/app/(app)/chat.tsx:91-91src/app/chat/[channelId].tsx:196-196src/app/(app)/chat.tsx:124-124src/api/chat/chatbot.ts:9-9src/api/chat/chatbot.ts:18-18src/app/chat/[channelId].tsx:149-149src/app/chat/[channelId].tsx:72-72src/app/chat/[channelId].tsx:276-276src/app/chat/[channelId].tsx:266-266src/app/chat/[channelId].tsx:229-229src/components/chat/message-bubble.tsx:92-92src/app/chat/[channelId].tsx:274-274src/components/chat/message-bubble.tsx:109-109src/app/chat/[channelId].tsx:273-273src/app/chat/[channelId].tsx:250-250src/app/chat/[channelId].tsx:173-173src/app/chat/[channelId].tsx:275-275src/app/chat/[channelId].tsx:174-174src/app/chat/[channelId].tsx:295-295src/app/chat/[channelId].tsx:93-93src/app/chat/[channelId].tsx:73-73src/api/chat/chat.ts:35-35src/app/chat/[channelId].tsx:74-74src/api/chat/chatbot.ts:27-27src/api/chat/chat.ts:193-193src/api/chat/chat.ts:43-43src/api/chat/chat.ts:48-48src/api/chat/chat.ts:63-63src/api/chat/chat.ts:82-82src/api/chat/chat.ts:130-130src/api/chat/chat.ts:144-144src/api/chat/chat.ts:169-169src/api/chat/chat.ts:53-53src/api/chat/chat.ts:77-77src/api/chat/chat.ts:120-120src/api/chat/chat.ts:139-139src/api/chat/chat.ts:149-149src/api/chat/chat.ts:164-164src/api/chat/chat.ts:247-247src/api/chat/chat.ts:253-253src/api/chat/chat.ts:58-58src/api/chat/chat.ts:72-72src/api/chat/chat.ts:87-87src/api/chat/chat.ts:96-96src/api/chat/chat.ts:104-104src/api/chat/chat.ts:112-112src/api/chat/chat.ts:125-125src/api/chat/chat.ts:154-154src/api/chat/chat.ts:159-159src/api/chat/chat.ts:174-174src/api/chat/chat.ts:223-223src/api/chat/chat.ts:231-231src/api/chat/chat.ts:239-239
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/app/(app)/chatbot.tsx:
Line 54:
Unhandled promise rejection occurs when async store action `sendChatbotMessage()` is invoked with `void` and no rejection handler. Add a `.catch()` handler or wrap the call in try/catch to log or surface network failures.
**Also found in:**
- `src/app/chat/thread/[messageId].tsx:57-57`
- `src/app/(app)/chatbot.tsx:34-34`
- `src/app/(app)/chatbot.tsx:81-81`
- `src/app/chat/thread/[messageId].tsx:69-76`
- `src/app/chat/thread/[messageId].tsx:91-91`
- `src/app/(app)/chat.tsx:92-92`
- `src/app/chat/thread/[messageId].tsx:92-92`
- `src/app/chat/[channelId].tsx:115-115`
- `src/app/(app)/chat.tsx:91-91`
- `src/app/chat/[channelId].tsx:196-196`
- `src/app/(app)/chat.tsx:124-124`
- `src/api/chat/chatbot.ts:9-9`
- `src/api/chat/chatbot.ts:18-18`
- `src/app/chat/[channelId].tsx:149-149`
- `src/app/chat/[channelId].tsx:72-72`
- `src/app/chat/[channelId].tsx:276-276`
- `src/app/chat/[channelId].tsx:266-266`
- `src/app/chat/[channelId].tsx:229-229`
- `src/components/chat/message-bubble.tsx:92-92`
- `src/app/chat/[channelId].tsx:274-274`
- `src/components/chat/message-bubble.tsx:109-109`
- `src/app/chat/[channelId].tsx:273-273`
- `src/app/chat/[channelId].tsx:250-250`
- `src/app/chat/[channelId].tsx:173-173`
- `src/app/chat/[channelId].tsx:275-275`
- `src/app/chat/[channelId].tsx:174-174`
- `src/app/chat/[channelId].tsx:295-295`
- `src/app/chat/[channelId].tsx:93-93`
- `src/app/chat/[channelId].tsx:73-73`
- `src/api/chat/chat.ts:35-35`
- `src/app/chat/[channelId].tsx:74-74`
- `src/api/chat/chatbot.ts:27-27`
- `src/api/chat/chat.ts:193-193`
- `src/api/chat/chat.ts:43-43`
- `src/api/chat/chat.ts:48-48`
- `src/api/chat/chat.ts:63-63`
- `src/api/chat/chat.ts:82-82`
- `src/api/chat/chat.ts:130-130`
- `src/api/chat/chat.ts:144-144`
- `src/api/chat/chat.ts:169-169`
- `src/api/chat/chat.ts:53-53`
- `src/api/chat/chat.ts:77-77`
- `src/api/chat/chat.ts:120-120`
- `src/api/chat/chat.ts:139-139`
- `src/api/chat/chat.ts:149-149`
- `src/api/chat/chat.ts:164-164`
- `src/api/chat/chat.ts:247-247`
- `src/api/chat/chat.ts:253-253`
- `src/api/chat/chat.ts:58-58`
- `src/api/chat/chat.ts:72-72`
- `src/api/chat/chat.ts:87-87`
- `src/api/chat/chat.ts:96-96`
- `src/api/chat/chat.ts:104-104`
- `src/api/chat/chat.ts:112-112`
- `src/api/chat/chat.ts:125-125`
- `src/api/chat/chat.ts:154-154`
- `src/api/chat/chat.ts:159-159`
- `src/api/chat/chat.ts:174-174`
- `src/api/chat/chat.ts:223-223`
- `src/api/chat/chat.ts:231-231`
- `src/api/chat/chat.ts:239-239`
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.
Authentication failure occurs in the full-screen image preview because source={{ uri: imageUri }} omits the required bearer auth header. Store the full image source object with headers instead of the bare URI string, using getChatAttachmentImageSource(attachmentId).
Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 313 to 316:
Authentication failure occurs in the full-screen image preview because `source={{ uri: imageUri }}` omits the required bearer auth header. Store the full image source object with headers instead of the bare URI string, using `getChatAttachmentImageSource(attachmentId)`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| import { useToastStore } from '@/stores/toast/store'; | ||
| import useAuthStore from '@/stores/auth/store'; | ||
|
|
||
| export default function ChannelConversationScreen() { |
There was a problem hiding this comment.
Default export reduces refactor-safety and greppability. Export the component as a named function (export function ChannelConversationScreen()), or keep the default if router-mandated but ensure it is also exported as a named symbol.
Also found in:
src/app/chat/thread/[messageId].tsx:20-20src/stores/chat/store.ts:23-23src/app/(app)/chatbot.tsx:23-23src/app/(app)/chat.tsx:80-80
Kody rule violation: Avoid default exports
Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 34:
Default export reduces refactor-safety and greppability. Export the component as a named function (`export function ChannelConversationScreen()`), or keep the default if router-mandated but ensure it is also exported as a named symbol.
**Also found in:**
- `src/app/chat/thread/[messageId].tsx:20-20`
- `src/stores/chat/store.ts:23-23`
- `src/app/(app)/chatbot.tsx:23-23`
- `src/app/(app)/chat.tsx:80-80`
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return ( | ||
| <HStack className="items-center justify-between border-b border-error-300 bg-error-50 px-4 py-2" space="sm"> | ||
| <HStack className="flex-1 items-center" space="sm"> | ||
| <AlertTriangle size={18} color="#dc2626" /> |
There was a problem hiding this comment.
Hardcoded hex color #dc2626 duplicates the error-600 design token and risks theme drift in dark/high-contrast modes. Use the design system's theme hook like useToken('colors.error.600') or a shared color constant instead.
Kody rule violation: Prefer the company design system over hand-rolled UI styles
Prompt for LLM
File src/components/chat/ack-banner.tsx:
Line 26:
Hardcoded hex color `#dc2626` duplicates the `error-600` design token and risks theme drift in dark/high-contrast modes. Use the design system's theme hook like `useToken('colors.error.600')` or a shared color constant instead.
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.
Unsafe type cast occurs when the JSON.parse result is returned as T without structural validation. Validate the parsed JSON using a schema like Zod or structural checks before returning the result.
Kody rule violation: Always validate JSON parsing
Prompt for LLM
File src/components/chat/chat-utils.ts:
Line 58:
Unsafe type cast occurs when the `JSON.parse` result is returned as `T` without structural validation. Validate the parsed JSON using a schema like Zod or structural checks before returning the result.
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.
Memory leak caused by an uncleared debounce timeout on component unmount. Add a cleanup effect to clear debounceRef.current when the component unmounts.
Kody rule violation: Clear timers on teardown/unmount
Prompt for LLM
File src/components/chat/gif-picker-sheet.tsx:
Line 53:
Memory leak caused by an uncleared debounce timeout on component unmount. Add a cleanup effect to clear `debounceRef.current` when the component unmounts.
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.
React list reordering issue caused by using array indexes as keys. Use unique identifiers instead of the index variable for the key prop.
Also found in:
src/components/chat/message-bubble.tsx:113-113src/components/chat/typing-indicator.tsx:29-29
Kody rule violation: Avoid array indexes as keys in React lists
Prompt for LLM
File src/components/chat/message-bubble.tsx:
Line 109:
React list reordering issue caused by using array indexes as keys. Use unique identifiers instead of the `index` variable for the `key` prop.
**Also found in:**
- `src/components/chat/message-bubble.tsx:113-113`
- `src/components/chat/typing-indicator.tsx:29-29`
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const loc = parseLocationMetadata(message.MetadataJson); | ||
| if (loc) { | ||
| return ( | ||
| <Pressable onPress={() => Linking.openURL(`https://maps.google.com/?q=${loc.Latitude},${loc.Longitude}`)}> |
There was a problem hiding this comment.
External system call Linking.openURL lacks a try/catch block to handle system failures like missing handlers or invalid URLs. Wrap the call in a try/catch block to log failures or display a user-facing alert.
Also found in:
src/api/chat/chatbot.ts:9-9src/components/chat/message-bubble.tsx:109-109src/api/chat/chatbot.ts:18-18src/api/chat/chatbot.ts:27-27src/api/chat/chat.ts:35-35src/api/chat/chat.ts:253-253src/api/chat/chat.ts:193-193src/api/chat/chat.ts:48-48src/api/chat/chat.ts:43-43src/api/chat/chat.ts:82-82src/api/chat/chat.ts:144-144src/api/chat/chat.ts:53-53src/api/chat/chat.ts:223-223src/api/chat/chat.ts:63-63src/api/chat/chat.ts:130-130src/api/chat/chat.ts:169-169src/api/chat/chat.ts:77-77src/api/chat/chat.ts:120-120src/api/chat/chat.ts:139-139src/api/chat/chat.ts:149-149src/api/chat/chat.ts:164-164src/api/chat/chat.ts:247-247src/api/chat/chat.ts:58-58src/api/chat/chat.ts:72-72src/api/chat/chat.ts:87-87src/api/chat/chat.ts:96-96src/api/chat/chat.ts:104-104src/api/chat/chat.ts:112-112src/api/chat/chat.ts:125-125src/api/chat/chat.ts:154-154src/api/chat/chat.ts:159-159src/api/chat/chat.ts:174-174src/api/chat/chat.ts:239-239src/api/chat/chat.ts:231-231
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/components/chat/message-bubble.tsx:
Line 92:
External system call `Linking.openURL` lacks a try/catch block to handle system failures like missing handlers or invalid URLs. Wrap the call in a try/catch block to log failures or display a user-facing alert.
**Also found in:**
- `src/api/chat/chatbot.ts:9-9`
- `src/components/chat/message-bubble.tsx:109-109`
- `src/api/chat/chatbot.ts:18-18`
- `src/api/chat/chatbot.ts:27-27`
- `src/api/chat/chat.ts:35-35`
- `src/api/chat/chat.ts:253-253`
- `src/api/chat/chat.ts:193-193`
- `src/api/chat/chat.ts:48-48`
- `src/api/chat/chat.ts:43-43`
- `src/api/chat/chat.ts:82-82`
- `src/api/chat/chat.ts:144-144`
- `src/api/chat/chat.ts:53-53`
- `src/api/chat/chat.ts:223-223`
- `src/api/chat/chat.ts:63-63`
- `src/api/chat/chat.ts:130-130`
- `src/api/chat/chat.ts:169-169`
- `src/api/chat/chat.ts:77-77`
- `src/api/chat/chat.ts:120-120`
- `src/api/chat/chat.ts:139-139`
- `src/api/chat/chat.ts:149-149`
- `src/api/chat/chat.ts:164-164`
- `src/api/chat/chat.ts:247-247`
- `src/api/chat/chat.ts:58-58`
- `src/api/chat/chat.ts:72-72`
- `src/api/chat/chat.ts:87-87`
- `src/api/chat/chat.ts:96-96`
- `src/api/chat/chat.ts:104-104`
- `src/api/chat/chat.ts:112-112`
- `src/api/chat/chat.ts:125-125`
- `src/api/chat/chat.ts:154-154`
- `src/api/chat/chat.ts:159-159`
- `src/api/chat/chat.ts:174-174`
- `src/api/chat/chat.ts:239-239`
- `src/api/chat/chat.ts:231-231`
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.
Undefined access error occurs when indexing the first element of Attachments without verifying the array is non-empty. Check message.Attachments?.length before indexing or use a safe accessor pattern.
Kody rule violation: Check query results before accessing indices
Prompt for LLM
File src/components/chat/message-bubble.tsx:
Line 68:
Undefined access error occurs when indexing the first element of `Attachments` without verifying the array is non-empty. Check `message.Attachments?.length` before indexing or use a safe accessor pattern.
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 throws a TypeError when API returns undefined for r.Name. Add optional chaining with a fallback using (r.Name ?? '').toLowerCase() to match the existing null-guard on line 37.
Also found in:
src/components/chat/message-bubble.tsx:89-89src/components/chat/message-bubble.tsx:81-81src/components/chat/message-bubble.tsx:174-174src/components/chat/message-bubble.tsx:35-35src/components/chat/message-bubble.tsx:68-68
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 throws a TypeError when API returns undefined for `r.Name`. Add optional chaining with a fallback using `(r.Name ?? '').toLowerCase()` to match the existing null-guard on line 37.
**Also found in:**
- `src/components/chat/message-bubble.tsx:89-89`
- `src/components/chat/message-bubble.tsx:81-81`
- `src/components/chat/message-bubble.tsx:174-174`
- `src/components/chat/message-bubble.tsx:35-35`
- `src/components/chat/message-bubble.tsx:68-68`
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| function isPersonRecipient(recipient: RecipientsResultData): boolean { | ||
| const type = (recipient.Type ?? '').toLowerCase(); | ||
| return type === 'personnel' || type === 'person' || type === 'user' || type === 'p' || type === ''; |
There was a problem hiding this comment.
Magic strings 'personnel', 'person', 'user', and 'p' create error-prone synchronization with backend values. Define a const map or enum like const RECIPIENT_TYPES = { PERSONNEL: 'personnel', PERSON: 'person', USER: 'user', P: 'p' } as const in a shared constants module.
Also found in:
src/hooks/use-signalr-lifecycle.ts:67-67src/stores/signalr/signalr-store.ts:434-434src/stores/signalr/signalr-store.ts:439-439src/hooks/use-signalr-lifecycle.ts:124-124src/stores/chat/store.ts:485-485src/components/chat/message-bubble.tsx:55-55src/components/chat/message-bubble.tsx:56-56src/stores/chat/store.ts:593-593src/stores/chat/store.ts:606-606src/components/chat/message-composer.tsx:101-101src/components/chat/message-composer.tsx:127-127src/components/chat/message-composer.tsx:137-137src/components/chat/message-composer.tsx:118-118src/components/chat/message-composer.tsx:131-131src/components/chat/message-composer.tsx:121-121src/components/chat/message-composer.tsx:124-124
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/components/chat/new-conversation-sheet.tsx:
Line 38:
Magic strings `'personnel'`, `'person'`, `'user'`, and `'p'` create error-prone synchronization with backend values. Define a const map or enum like `const RECIPIENT_TYPES = { PERSONNEL: 'personnel', PERSON: 'person', USER: 'user', P: 'p' } as const` in a shared constants module.
**Also found in:**
- `src/hooks/use-signalr-lifecycle.ts:67-67`
- `src/stores/signalr/signalr-store.ts:434-434`
- `src/stores/signalr/signalr-store.ts:439-439`
- `src/hooks/use-signalr-lifecycle.ts:124-124`
- `src/stores/chat/store.ts:485-485`
- `src/components/chat/message-bubble.tsx:55-55`
- `src/components/chat/message-bubble.tsx:56-56`
- `src/stores/chat/store.ts:593-593`
- `src/stores/chat/store.ts:606-606`
- `src/components/chat/message-composer.tsx:101-101`
- `src/components/chat/message-composer.tsx:127-127`
- `src/components/chat/message-composer.tsx:137-137`
- `src/components/chat/message-composer.tsx:118-118`
- `src/components/chat/message-composer.tsx:131-131`
- `src/components/chat/message-composer.tsx:121-121`
- `src/components/chat/message-composer.tsx:124-124`
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }; | ||
|
|
||
| const handleNavigateToAssistant = () => { | ||
| onClose?.(); |
There was a problem hiding this comment.
Duplicated navigation sequence onClose?.(); router.push(route) exists across handleNavigateToChat, handleNavigateToAssistant, and handleNavigateToSettings. Extract a reusable navigate helper function that accepts a route string.
Also found in:
src/components/sidebar/sidebar-content.tsx:45-45src/components/sidebar/sidebar-content.tsx:39-39src/components/sidebar/sidebar-content.tsx:40-40src/hooks/use-signalr-lifecycle.ts:67-67src/hooks/use-signalr-lifecycle.ts:124-124src/components/chat/typing-indicator.tsx:10-10
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File src/components/sidebar/sidebar-content.tsx:
Line 44:
Duplicated navigation sequence `onClose?.(); router.push(route)` exists across `handleNavigateToChat`, `handleNavigateToAssistant`, and `handleNavigateToSettings`. Extract a reusable `navigate` helper function that accepts a route string.
**Also found in:**
- `src/components/sidebar/sidebar-content.tsx:45-45`
- `src/components/sidebar/sidebar-content.tsx:39-39`
- `src/components/sidebar/sidebar-content.tsx:40-40`
- `src/hooks/use-signalr-lifecycle.ts:67-67`
- `src/hooks/use-signalr-lifecycle.ts:124-124`
- `src/components/chat/typing-indicator.tsx:10-10`
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| /** Absolute URL for a person's avatar image, served by the Resgrid API. */ | ||
| export function getAvatarUrl(userId: string) { | ||
| return getBaseApiUrl() + '/Avatars/Get?id=' + userId; |
There was a problem hiding this comment.
String concatenation using + reduces readability and increases error risk. Build the avatar URL using a template literal instead.
Kody rule violation: Use Template Literals Instead of String Concatenation
Prompt for LLM
File src/lib/utils.ts:
Line 108:
String concatenation using `+` reduces readability and increases error risk. Build the avatar URL using a template literal instead.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
|
|
||
| /** Client-only lifecycle status for optimistic messages (never sent by the server). */ | ||
| export type ChatMessageLocalStatus = 'pending' | 'failed' | 'sent'; |
There was a problem hiding this comment.
Type definition ChatMessageLocalStatus relies on a bare string-literal union instead of a const tuple, risking runtime and type-level desync. Declare the values as a const tuple and derive the type using typeof.
Kody rule violation: Derive TypeScript types from validation schemas
Prompt for LLM
File src/models/v4/chat/chatEnums.ts:
Line 69:
Type definition `ChatMessageLocalStatus` relies on a bare string-literal union instead of a const tuple, risking runtime and type-level desync. Declare the values as a `const` tuple and derive the type using `typeof`.
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.
Naming convention inconsistency arises when the camelCase property success deviates from the established PascalCase pattern in ChatbotChannelResponse and ChatbotSendResponse. Rename the property to Success to match sibling fields.
Kody rule violation: Use proper naming conventions
Prompt for LLM
File src/models/v4/chat/chatbotModels.ts:
Line 20:
Naming convention inconsistency arises when the camelCase property `success` deviates from the established PascalCase pattern in `ChatbotChannelResponse` and `ChatbotSendResponse`. Rename the property to `Success` to match sibling fields.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if ((lastMarkedSeq.get(channelId) ?? 0) >= seq) return; | ||
| lastMarkedSeq.set(channelId, seq); | ||
|
|
||
| set((s) => ({ | ||
| channels: s.channels.map((c) => (c.ChatChannelId === channelId ? { ...c, UnreadCount: 0, MyLastReadSeq: seq } : c)), | ||
| })); | ||
|
|
||
| // Read pointer is recorded against the active unit identity in the Unit app. | ||
| const asUnitId = activeUnitIdNumber(); | ||
| void safeInvoke('MarkRead', channelId, seq, ...(asUnitId != null ? [asUnitId] : [])); | ||
| try { | ||
| await chatApi.markRead(channelId, { Seq: seq, AsUnitId: asUnitId }); | ||
| } catch (error) { | ||
| logger.debug({ message: 'chat: markRead failed', context: { error, channelId } }); | ||
| } |
There was a problem hiding this comment.
Failed mark-read operation permanently blocks retrying the sequence because lastMarkedSeq is set before the chatApi.markRead call and never cleared on failure. Move lastMarkedSeq.set into a .then() on the API call, or clear it in the catch block.
if ((lastMarkedSeq.get(channelId) ?? 0) >= seq) return;
set((s) => ({
channels: s.channels.map((c) => (c.ChatChannelId === channelId ? { ...c, UnreadCount: 0, MyLastReadSeq: seq } : c)),
}));
const asUnitId = activeUnitIdNumber();
void safeInvoke('MarkRead', channelId, seq, ...(asUnitId != null ? [asUnitId] : []));
try {
await chatApi.markRead(channelId, { Seq: seq, AsUnitId: asUnitId });
lastMarkedSeq.set(channelId, seq);
} catch (error) {
logger.debug({ message: 'chat: markRead failed', context: { error, channelId } });
}Prompt for LLM
File src/stores/chat/store.ts:
Line 476 to 490:
Failed mark-read operation permanently blocks retrying the sequence because `lastMarkedSeq` is set before the `chatApi.markRead` call and never cleared on failure. Move `lastMarkedSeq.set` into a `.then()` on the API call, or clear it in the catch block.
Suggested Code:
if ((lastMarkedSeq.get(channelId) ?? 0) >= seq) return;
set((s) => ({
channels: s.channels.map((c) => (c.ChatChannelId === channelId ? { ...c, UnreadCount: 0, MyLastReadSeq: seq } : c)),
}));
const asUnitId = activeUnitIdNumber();
void safeInvoke('MarkRead', channelId, seq, ...(asUnitId != null ? [asUnitId] : []));
try {
await chatApi.markRead(channelId, { Seq: seq, AsUnitId: asUnitId });
lastMarkedSeq.set(channelId, seq);
} catch (error) {
logger.debug({ message: 'chat: markRead failed', context: { error, channelId } });
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } catch (error) { | ||
| logger.error({ message: 'chat: chatbot send failed', context: { error } }); | ||
| markOutboxFailed(set, channelId, clientMessageId); | ||
| set({ chatbotTyping: false }); | ||
| } |
There was a problem hiding this comment.
Stranded optimistic message permanently sticks as failed because markOutboxFailed marks a bubble that was never added to the outbox array, and onRetry is undefined in chatbot.tsx:58-59. Add chatbot messages to the outbox on failure so retryOutboxItem works, or clear the failed optimistic message on error.
Prompt for LLM
File src/stores/chat/store.ts:
Line 572 to 576:
Stranded optimistic message permanently sticks as failed because `markOutboxFailed` marks a bubble that was never added to the `outbox` array, and `onRetry` is undefined in chatbot.tsx:58-59. Add chatbot messages to the outbox on failure so `retryOutboxItem` works, or clear the failed optimistic message on error.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ChatMessageId: `local-${clientMessageId}`, | ||
| ChatChannelId: args.channelId, | ||
| MessageSeq: PENDING_SEQ_BASE + outboxItem.CreatedAt, | ||
| SenderParticipantType: 0, |
There was a problem hiding this comment.
Magic number assigned to SenderParticipantType breaks consistency with sibling enum-backed fields like ChatMessageType. Introduce a ChatParticipantType enum and assign ChatParticipantType.User instead of 0.
Also found in:
src/components/chat/message-bubble.tsx:83-83src/app/chat/thread/[messageId].tsx:34-34src/components/chat/message-bubble.tsx:74-74src/components/chat/ack-banner.tsx:26-26src/stores/chat/store.ts:424-424src/components/chat/typing-indicator.tsx:10-10src/components/chat/typing-indicator.tsx:18-18src/components/chat/gif-picker-sheet.tsx:35-35src/components/chat/message-actions-sheet.tsx:101-101src/components/chat/message-bubble.tsx:136-136src/stores/chat/store.ts:263-263src/stores/chat/store.ts:284-284src/stores/chat/store.ts:256-256src/stores/chat/store.ts:547-547src/app/(app)/chat.tsx:60-60src/components/chat/message-actions-sheet.tsx:148-148src/components/sidebar/sidebar-content.tsx:69-69src/components/sidebar/sidebar-content.tsx:73-73src/components/chat/typing-indicator.tsx:29-29src/components/chat/gif-picker-sheet.tsx:53-53src/components/chat/message-actions-sheet.tsx:112-112src/components/chat/message-actions-sheet.tsx:124-124src/components/chat/message-actions-sheet.tsx:136-136src/components/chat/message-actions-sheet.tsx:155-155src/components/chat/message-actions-sheet.tsx:167-167src/components/chat/typing-indicator.tsx:16-16src/components/chat/typing-indicator.tsx:17-17src/stores/chat/store.ts:291-291src/components/chat/gif-picker-sheet.tsx:57-57src/stores/chat/store.ts:304-304src/components/chat/message-composer.tsx:72-72src/components/chat/message-composer.tsx:98-98src/components/chat/message-composer.tsx:101-101src/components/chat/message-composer.tsx:131-131src/components/chat/message-composer.tsx:137-137src/components/chat/message-composer.tsx:118-118src/components/chat/message-composer.tsx:121-121src/components/chat/message-composer.tsx:124-124src/components/chat/message-composer.tsx:127-127
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/stores/chat/store.ts:
Line 344:
Magic number assigned to `SenderParticipantType` breaks consistency with sibling enum-backed fields like `ChatMessageType`. Introduce a `ChatParticipantType` enum and assign `ChatParticipantType.User` instead of `0`.
**Also found in:**
- `src/components/chat/message-bubble.tsx:83-83`
- `src/app/chat/thread/[messageId].tsx:34-34`
- `src/components/chat/message-bubble.tsx:74-74`
- `src/components/chat/ack-banner.tsx:26-26`
- `src/stores/chat/store.ts:424-424`
- `src/components/chat/typing-indicator.tsx:10-10`
- `src/components/chat/typing-indicator.tsx:18-18`
- `src/components/chat/gif-picker-sheet.tsx:35-35`
- `src/components/chat/message-actions-sheet.tsx:101-101`
- `src/components/chat/message-bubble.tsx:136-136`
- `src/stores/chat/store.ts:263-263`
- `src/stores/chat/store.ts:284-284`
- `src/stores/chat/store.ts:256-256`
- `src/stores/chat/store.ts:547-547`
- `src/app/(app)/chat.tsx:60-60`
- `src/components/chat/message-actions-sheet.tsx:148-148`
- `src/components/sidebar/sidebar-content.tsx:69-69`
- `src/components/sidebar/sidebar-content.tsx:73-73`
- `src/components/chat/typing-indicator.tsx:29-29`
- `src/components/chat/gif-picker-sheet.tsx:53-53`
- `src/components/chat/message-actions-sheet.tsx:112-112`
- `src/components/chat/message-actions-sheet.tsx:124-124`
- `src/components/chat/message-actions-sheet.tsx:136-136`
- `src/components/chat/message-actions-sheet.tsx:155-155`
- `src/components/chat/message-actions-sheet.tsx:167-167`
- `src/components/chat/typing-indicator.tsx:16-16`
- `src/components/chat/typing-indicator.tsx:17-17`
- `src/stores/chat/store.ts:291-291`
- `src/components/chat/gif-picker-sheet.tsx:57-57`
- `src/stores/chat/store.ts:304-304`
- `src/components/chat/message-composer.tsx:72-72`
- `src/components/chat/message-composer.tsx:98-98`
- `src/components/chat/message-composer.tsx:101-101`
- `src/components/chat/message-composer.tsx:131-131`
- `src/components/chat/message-composer.tsx:137-137`
- `src/components/chat/message-composer.tsx:118-118`
- `src/components/chat/message-composer.tsx:121-121`
- `src/components/chat/message-composer.tsx:124-124`
- `src/components/chat/message-composer.tsx:127-127`
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| signalRService.invoke(Env.CHAT_HUB_NAME, 'Heartbeat').catch(() => { | ||
| // Heartbeat is best-effort; ignore transient failures. | ||
| }); |
There was a problem hiding this comment.
Silent error swallowing occurs in the .catch() handler for the heartbeat invoke. Replace the empty arrow function with a logging call like .catch((err) => logger.warn({ message: 'Chat heartbeat failed', context: { error: err } })).
Also found in:
src/components/chat/chat-utils.ts:116-118src/components/chat/chat-utils.ts:59-61src/app/chat/[channelId].tsx:87-87
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 439 to 441:
Silent error swallowing occurs in the `.catch()` handler for the heartbeat invoke. Replace the empty arrow function with a logging call like `.catch((err) => logger.warn({ message: 'Chat heartbeat failed', context: { error: err } }))`.
**Also found in:**
- `src/components/chat/chat-utils.ts:116-118`
- `src/components/chat/chat-utils.ts:59-61`
- `src/app/chat/[channelId].tsx:87-87`
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| logger.info({ message: 'Chat hub disconnected and handlers cleaned up' }); | ||
| } catch (error) { | ||
| const err = error instanceof Error ? error : new Error('Unknown error occurred'); | ||
| logger.error({ message: 'Failed to disconnect from chat SignalR hub', context: { error: err } }); |
There was a problem hiding this comment.
Unstructured error log embeds the operation name in the message text instead of as a queryable field. Add structured fields like op: 'disconnectChatHub' and hub: Env.CHAT_HUB_NAME to the logger.error payload.
Also found in:
src/stores/signalr/signalr-store.ts:447-447
Kody rule violation: Include error context in structured logs
Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 460:
Unstructured error log embeds the operation name in the message text instead of as a queryable field. Add structured fields like `op: 'disconnectChatHub'` and `hub: Env.CHAT_HUB_NAME` to the `logger.error` payload.
**Also found in:**
- `src/stores/signalr/signalr-store.ts:447-447`
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| set({ error: err }); | ||
| } | ||
| }, | ||
| connectChatHub: async () => { |
There was a problem hiding this comment.
Missing JSDoc on async function connectChatHub omits required Promise return types, resolve values, and rejection conditions. Add JSDoc annotations including @returns {Promise<void>} and @throws {Error}.
Also found in:
src/stores/signalr/signalr-store.ts:451-451src/components/chat/chat-utils.ts:109-109src/api/chat/chatbot.ts:8-8src/api/chat/chatbot.ts:17-17src/api/chat/chatbot.ts:26-26
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 379:
Missing JSDoc on async function `connectChatHub` omits required Promise return types, resolve values, and rejection conditions. Add JSDoc annotations including `@returns {Promise<void>}` and `@throws {Error}`.
**Also found in:**
- `src/stores/signalr/signalr-store.ts:451-451`
- `src/components/chat/chat-utils.ts:109-109`
- `src/api/chat/chatbot.ts:8-8`
- `src/api/chat/chatbot.ts:17-17`
- `src/api/chat/chatbot.ts:26-26`
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.
Duplicated string literal 'onChatConnected' risks divergence from the CHAT_HUB_METHODS array on line 29. Extract named constants like const ON_CHAT_CONNECTED = 'onChatConnected' and reference the constant in both the array and the .on() call.
Also found in:
src/hooks/use-signalr-lifecycle.ts:67-67src/lib/utils.ts:108-108src/components/chat/message-actions-sheet.tsx:101-101src/hooks/use-signalr-lifecycle.ts:124-124src/stores/chat/store.ts:770-770src/components/sidebar/sidebar-content.tsx:45-45src/components/sidebar/sidebar-content.tsx:40-40src/stores/chat/store.ts:341-341src/components/chat/message-actions-sheet.tsx:112-112src/app/(app)/chat.tsx:115-115src/components/chat/message-actions-sheet.tsx:136-136src/stores/chat/store.ts:544-544src/components/chat/message-actions-sheet.tsx:124-124src/components/chat/message-actions-sheet.tsx:167-167src/app/(app)/chat.tsx:174-174src/components/chat/message-actions-sheet.tsx:148-148src/components/chat/message-actions-sheet.tsx:155-155
Kody rule violation: Centralize string constants
Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 431:
Duplicated string literal `'onChatConnected'` risks divergence from the `CHAT_HUB_METHODS` array on line 29. Extract named constants like `const ON_CHAT_CONNECTED = 'onChatConnected'` and reference the constant in both the array and the `.on()` call.
**Also found in:**
- `src/hooks/use-signalr-lifecycle.ts:67-67`
- `src/lib/utils.ts:108-108`
- `src/components/chat/message-actions-sheet.tsx:101-101`
- `src/hooks/use-signalr-lifecycle.ts:124-124`
- `src/stores/chat/store.ts:770-770`
- `src/components/sidebar/sidebar-content.tsx:45-45`
- `src/components/sidebar/sidebar-content.tsx:40-40`
- `src/stores/chat/store.ts:341-341`
- `src/components/chat/message-actions-sheet.tsx:112-112`
- `src/app/(app)/chat.tsx:115-115`
- `src/components/chat/message-actions-sheet.tsx:136-136`
- `src/stores/chat/store.ts:544-544`
- `src/components/chat/message-actions-sheet.tsx:124-124`
- `src/components/chat/message-actions-sheet.tsx:167-167`
- `src/app/(app)/chat.tsx:174-174`
- `src/components/chat/message-actions-sheet.tsx:148-148`
- `src/components/chat/message-actions-sheet.tsx:155-155`
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/chat/new-conversation-sheet.tsx (1)
93-127: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInvalidate pending create requests when the sheet closes.
A direct-message or group request can resolve after the user closes or reopens the sheet. Its stale completion then calls
onCreatedandonClose, which can navigate the user into an unintended channel. Track a submission generation or cancellation state, and apply completion effects only for the active sheet session.🤖 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 93 - 127, Update the new-conversation sheet’s submission flow around startDirectMessage and createGroup to track the active sheet session and invalidate pending requests when the sheet closes or reopens. Before calling onCreated or onClose, verify the request still belongs to the active session; stale completions must only clear their own submitting state and must not trigger navigation or close effects.src/stores/chat/store.ts (2)
316-334: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the delta-sync loop so it cannot repeat the same page forever.
The loop only exits when a page returns fewer than 200 rows. The next cursor comes from
highestRealSeq(...)after the store update. If a full page does not raise the highest real sequence,afterstays the same and the loop reissues the identical request without end. This happens when the server returns rows already present, returns rows at or belowafter, or ignores theafterparameter.upsertMessagededuplicates, so the store stops changing while the network requests continue.Add a cursor-advance check and a page cap.
🐛 Proposed fix to bound the loop
loadNewerMessages: async (channelId: string) => { try { let after = highestRealSeq(get().messagesByChannel[channelId]); - for (;;) { + for (let page = 0; page < MAX_DELTA_SYNC_PAGES; page += 1) { 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]); + const next = highestRealSeq(get().messagesByChannel[channelId]); + if (next <= after) { + logger.warn({ message: 'chat: delta sync cursor did not advance', context: { channelId, after } }); + return; + } + after = next; } + logger.warn({ message: 'chat: delta sync page cap reached', context: { channelId, after } }); } catch (error) { logger.error({ message: 'chat: delta sync failed', context: { error, channelId } }); } },Declare the cap next to the other module constants:
const MAX_DELTA_SYNC_PAGES = 50;🤖 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 316 - 334, Update loadNewerMessages to prevent unbounded pagination by using a MAX_DELTA_SYNC_PAGES cap declared with the module constants and stopping once the cap is reached. After each page, recompute the highest real sequence and return when it does not advance beyond the previous after cursor, while preserving the existing empty-page and short-page exits.
772-776: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winArm and clear the typing watchdog in
handleChatbotTyping.This handler sets
chatbotTypingdirectly and never toucheschatbotTypingTimer. If the hub emits a typing event after the send watchdog already fired, the indicator turns on with no timeout armed. If the bot never replies, the indicator stays visible until the user leaves the screen or the store resets. The state does not self-recover.🐛 Proposed fix
handleChatbotTyping: (raw: unknown) => { const evt = parseEventData<{ IsTyping?: boolean; isTyping?: boolean }>(raw); const isTyping = evt?.IsTyping ?? evt?.isTyping ?? false; + if (isTyping) startChatbotTypingTimeout(set); + else clearChatbotTypingTimeout(); set({ chatbotTyping: isTyping }); },🤖 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 772 - 776, Update handleChatbotTyping to clear any existing chatbotTypingTimer before applying the parsed typing state, and arm the watchdog whenever the resulting chatbotTyping value is true. Ensure the timer callback clears the indicator and timer state if no bot reply arrives, while preserving the existing false-state behavior.
🧹 Nitpick comments (1)
src/stores/chat/store.ts (1)
456-463: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGive the same user feedback for a failed moderator delete.
editMessage,deleteMessage,togglePin, andflagMessagenow show an error toast on failure.moderatorDeleteMessageonly logs. The moderator sees no result when the delete fails.♻️ Proposed change
} catch (error) { logger.error({ message: 'chat: moderator delete failed', context: { error, messageId } }); + useToastStore.getState().showToast('error', getTranslatedMessage('chat.delete_failed', 'Could not delete message')); }As per coding guidelines: "Handle errors gracefully and provide user feedback via toast notifications using
useToastStore".🤖 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 456 - 463, Update moderatorDeleteMessage to provide the same failure toast feedback as editMessage, deleteMessage, togglePin, and flagMessage, while retaining the existing logger.error call and message patch behavior. Reuse the established useToastStore error-notification pattern and message used by those neighboring actions.Source: Coding guidelines
🤖 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`:
- Line 1: Apply the configured import ordering by moving the TFunction type-only
import after the value-import group in src/components/chat/chat-utils.ts lines
1-1 and src/components/chat/__tests__/chat-utils.test.ts lines 1-5; preserve all
imported symbols and usage.
In `@src/stores/chat/store.ts`:
- Around line 850-858: Add an onRehydrateStorage callback to the chat store that
invokes drainOutbox after persisted state hydration and the merge callback have
restored the outbox. Preserve the existing merge behavior that rebuilds
optimistic messages, and ensure the drain runs against the hydrated store state
rather than before rehydration completes.
- Around line 964-989: Update markOutboxFailed so terminal failures, including
attempts reaching MAX_OUTBOX_ATTEMPTS, assign the message a terminal rejected
status instead of failed, while retryable failures retain the existing failed
status and retry behavior. Extend the local status union in chatModels.ts, hide
the retry affordance for rejected messages in the message bubble, and show the
specified error toast when sending is abandoned.
---
Outside diff comments:
In `@src/components/chat/new-conversation-sheet.tsx`:
- Around line 93-127: Update the new-conversation sheet’s submission flow around
startDirectMessage and createGroup to track the active sheet session and
invalidate pending requests when the sheet closes or reopens. Before calling
onCreated or onClose, verify the request still belongs to the active session;
stale completions must only clear their own submitting state and must not
trigger navigation or close effects.
In `@src/stores/chat/store.ts`:
- Around line 316-334: Update loadNewerMessages to prevent unbounded pagination
by using a MAX_DELTA_SYNC_PAGES cap declared with the module constants and
stopping once the cap is reached. After each page, recompute the highest real
sequence and return when it does not advance beyond the previous after cursor,
while preserving the existing empty-page and short-page exits.
- Around line 772-776: Update handleChatbotTyping to clear any existing
chatbotTypingTimer before applying the parsed typing state, and arm the watchdog
whenever the resulting chatbotTyping value is true. Ensure the timer callback
clears the indicator and timer state if no bot reply arrives, while preserving
the existing false-state behavior.
---
Nitpick comments:
In `@src/stores/chat/store.ts`:
- Around line 456-463: Update moderatorDeleteMessage to provide the same failure
toast feedback as editMessage, deleteMessage, togglePin, and flagMessage, while
retaining the existing logger.error call and message patch behavior. Reuse the
established useToastStore error-notification pattern and message used by those
neighboring actions.
🪄 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: d6fe7018-e81f-4ded-91d3-2361a74361ef
📒 Files selected for processing (22)
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/app-reset.service.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 (6)
- src/translations/en.json
- src/services/push-notification.ts
- src/components/chat/message-composer.tsx
- src/api/chat/chat.ts
- src/app/(app)/chat.tsx
- src/app/chat/[channelId].tsx
| @@ -0,0 +1,154 @@ | |||
| import { type TFunction } from 'i18next'; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Apply the configured import sort order.
The type-only imports precede value-import groups. This causes the reported autofix failure.
src/components/chat/chat-utils.ts#L1-L1: MoveTFunctioninto the type-only import group after value imports.src/components/chat/__tests__/chat-utils.test.ts#L1-L5: MoveTFunctioninto the type-only import group after value imports.
🧰 Tools
🪛 GitHub Check: test
[failure] 1-1:
Run autofix to sort these imports!
📍 Affects 2 files
src/components/chat/chat-utils.ts#L1-L1(this comment)src/components/chat/__tests__/chat-utils.test.ts#L1-L5
🤖 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` at line 1, Apply the configured import
ordering by moving the TFunction type-only import after the value-import group
in src/components/chat/chat-utils.ts lines 1-1 and
src/components/chat/__tests__/chat-utils.test.ts lines 1-5; preserve all
imported symbols and usage.
Sources: Coding guidelines, Linters/SAST tools
| merge: (persistedState, currentState) => { | ||
| const persisted = persistedState as { outbox?: ChatOutboxItem[] } | undefined; | ||
| const outbox = persisted?.outbox ?? []; | ||
| const messagesByChannel = { ...currentState.messagesByChannel }; | ||
| for (const item of outbox) { | ||
| messagesByChannel[item.ChannelId] = upsertMessage(messagesByChannel[item.ChannelId] ?? [], buildOutboxOptimisticMessage(item)); | ||
| } | ||
| return { ...currentState, outbox, messagesByChannel }; | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Trace when the chat outbox is drained relative to store rehydration and hub connection.
set -euo pipefail
echo "== drain / rehydrate entry points in the chat store"
rg -n -C4 'drainOutbox|onRehydrateStorage|handleChatConnected|hasHydrated|persist\(' src/stores/chat/store.ts
echo
echo "== who calls handleChatConnected and when the chat hub connects"
rg -n -C6 'handleChatConnected|chatHub|ChatHub|connectChat' src --glob '!**/__tests__/**'
echo
echo "== SignalR lifecycle wiring"
fd -t f 'use-signalr-lifecycle' src --exec rg -n -C4 'chat|connect|start' {}Repository: Resgrid/Unit
Length of output: 28670
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== store persistence / onRehydrateStorage / drain scheduling in src/stores/chat/store.ts"
sed -n '130,920p' src/stores/chat/store.ts | nl -ba | sed -n '1,260p'
echo
sed -n '920,975p' src/stores/chat/store.ts | nl -ba -v920
echo
echo "== direct store imports / initialization of chat store"
rg -n -C3 'useChatStore|connectChatHub|start|init|App Start|hydration|hasHydrated|hydrate' src/app src/hooks src/lib src/providers --glob '!**/__tests__/**'
echo
echo "== package version hints for zustand-mmkv"
if [ -f package.json ]; then
jq -r '.dependencies, .devDependencies' package.json | rg 'zustand|react-native-mmkv|`@react-native-async/async-storage`' -C2 || true
fiRepository: Resgrid/Unit
Length of output: 279
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== static verifier: compare persisted-outbox branch timing"
python3 - <<'PY'
from pathlib import Path
p = Path('src/stores/chat/store.ts')
s = p.read_text()
checks = {
'merge_uses_persisted_outbox': 'persisted?.outbox ?? []' in s,
'merge_calls_buildOutboxOptimisticMessage': 'buildOutboxOptimisticMessage(item)' in s,
'handle_chat_connected_drain_outbox': 'handleChatConnected: () => {' in s and 'void get().drainOutbox();' in s,
'channel_connected_drain_outbox': 'channelConnected: async (channelId: string) => {' in s and '} as Promise<void>' in s and 'drainOutbox()' in s[s.find('channelConnected: async (channelId: string):'):],
'on_rehydrate_exists': 'onRehydrateStorage' in s,
'on_rehydrate_contains_drain': 'onRehydrateStorage' in s and 'drainOutbox' in s[s.find('onRehydrateStorage'):s.find('onRehydrateStorage')+700],
}
for k, v in checks.items():
print(f"{k}: {v}")
PYRepository: Resgrid/Unit
Length of output: 431
🌐 Web query:
zustand persist onRehydrateStorage is called on hydration after initialize
💡 Result:
In the Zustand persist middleware, the onRehydrateStorage function is designed to handle logic both before and after the hydration process occurs [1][2]. When you provide a function to onRehydrateStorage, it is called immediately when the hydration process begins [2]. This primary function can optionally return a second function, which is then executed automatically once the hydration process completes (or if an error occurs) [1][3]. The sequence is as follows: 1. Initialization: When the store is initialized, the persist middleware triggers the hydration process [1]. 2. Start: The main function provided to onRehydrateStorage is called [1][2]. 3. Hydration: The middleware retrieves and merges the persisted state [4][2]. 4. Finish: The function returned by your initial onRehydrateStorage call is executed, receiving the hydrated state or an error object as an argument [1][3]. If you use skipHydration: true, the hydration process (and thus the onRehydrateStorage lifecycle) will not trigger on initialization [1][4]. Instead, it will only occur when you manually invoke the rehydrate() method on the store [4][3]. This pattern is the standard way to track hydration status or perform side effects related to state restoration [1][3]. Top Results: [1], [3], [2]
Citations:
- 1: https://zustand.docs.pmnd.rs/reference/integrations/persisting-store-data
- 2: https://github.com/pmndrs/zustand/blob/main/src/middleware/persist.ts
- 3: https://github.com/pmndrs/zustand/blob/main/docs/reference/integrations/persisting-store-data.md
- 4: https://zustand.docs.pmnd.rs/reference/middlewares/persist
Drain restored chat-store outbox on hydration.
sipmerge restores persisted outbox entries and rebuilds optimistic messages, but the store has no onRehydrateStorage drain path. Only handleChatConnected and scheduled reconnect drains call drainOutbox, so restored items can remain pending until the next reconnect. Call drainOutbox from onRehydrateStorage after merging the 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 850 - 858, Add an onRehydrateStorage
callback to the chat store that invokes drainOutbox after persisted state
hydration and the merge callback have restored the outbox. Preserve the existing
merge behavior that rebuilds optimistic messages, and ensure the drain runs
against the hydrated store state rather than before rehydration completes.
| function markOutboxFailed(set: SetState, channelId: string, clientMessageId: string, error?: unknown): void { | ||
| const terminal = isNonRetryableSendError(error); | ||
| set((s) => { | ||
| const list = s.messagesByChannel[channelId] ?? []; | ||
| const idx = list.findIndex((m) => m.ClientMessageId === clientMessageId); | ||
| const next = list.slice(); | ||
| if (idx >= 0) next[idx] = { ...next[idx], _localStatus: 'failed' }; | ||
|
|
||
| let outbox = s.outbox; | ||
| const itemIdx = s.outbox.findIndex((o) => o.ClientMessageId === clientMessageId); | ||
| 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() }; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| ...(idx >= 0 ? { messagesByChannel: { ...s.messagesByChannel, [channelId]: next } } : {}), | ||
| ...(outbox !== s.outbox ? { outbox } : {}), | ||
| }; | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A terminally failed message leaves a bubble whose retry action does nothing.
When terminal is true, or when attempts reaches MAX_OUTBOX_ATTEMPTS, this function removes the entry from outbox but still sets the bubble to _localStatus: 'failed'. The bubble stays on screen with a retry affordance. retryOutboxItem then finds no outbox entry and no pendingChatbotMessages entry, so it returns without any action and without feedback. The user taps retry and observes nothing.
Distinguish a terminal failure from a retryable one so the UI can present it correctly, and tell the user the message was rejected.
🐛 Proposed fix
Add a terminal status to the local status union in src/models/v4/chat/chatModels.ts:
-export type ChatMessageLocalStatus = 'pending' | 'sent' | 'failed';
+export type ChatMessageLocalStatus = 'pending' | 'sent' | 'failed' | 'rejected';Then mark the bubble accordingly and notify the user:
function markOutboxFailed(set: SetState, channelId: string, clientMessageId: string, error?: unknown): void {
const terminal = isNonRetryableSendError(error);
set((s) => {
const list = s.messagesByChannel[channelId] ?? [];
const idx = list.findIndex((m) => m.ClientMessageId === clientMessageId);
const next = list.slice();
- if (idx >= 0) next[idx] = { ...next[idx], _localStatus: 'failed' };
let outbox = s.outbox;
const itemIdx = s.outbox.findIndex((o) => o.ClientMessageId === clientMessageId);
+ let dropped = terminal;
if (itemIdx >= 0) {
const attempts = (s.outbox[itemIdx].Attempts ?? 0) + 1;
if (terminal || attempts >= MAX_OUTBOX_ATTEMPTS) {
+ dropped = true;
outbox = s.outbox.filter((o) => o.ClientMessageId !== clientMessageId);
} else {
outbox = s.outbox.slice();
outbox[itemIdx] = { ...outbox[itemIdx], Attempts: attempts, LastAttemptAt: Date.now() };
}
}
+ if (idx >= 0) next[idx] = { ...next[idx], _localStatus: dropped ? 'rejected' : 'failed' };
return {
...(idx >= 0 ? { messagesByChannel: { ...s.messagesByChannel, [channelId]: next } } : {}),
...(outbox !== s.outbox ? { outbox } : {}),
};
});
}Hide the retry affordance for 'rejected' in the message bubble, and show a toast when the send is abandoned:
useToastStore.getState().showToast('error', getTranslatedMessage('chat.send_rejected', 'Message could not be sent'));🤖 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 964 - 989, Update markOutboxFailed so
terminal failures, including attempts reaching MAX_OUTBOX_ATTEMPTS, assign the
message a terminal rejected status instead of failed, while retryable failures
retain the existing failed status and retry behavior. Extend the local status
union in chatModels.ts, hide the retry affordance for rejected messages in the
message bubble, and show the specified error toast when sending is abandoned.
| 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.
Memory leak in handleSendImage: the useChatStore.subscribe callback only self-unsubscribes upon successful server reconciliation, failing to clean up on terminal send failures or component unmounts. Track the subscription in a ref or unsubscribe when the message transitions to _localStatus: 'failed'.
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 158 to 169:
Memory leak in `handleSendImage`: the `useChatStore.subscribe` callback only self-unsubscribes upon successful server reconciliation, failing to clean up on terminal send failures or component unmounts. Track the subscription in a ref or unsubscribe when the message transitions to `_localStatus: 'failed'`.
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 name = uri.split('/').pop() || `photo-${Date.now()}.jpg`; | ||
| const type = getImageMimeType(uri, mimeType); | ||
|
|
||
| const unsubscribe = useChatStore.subscribe((state) => { |
There was a problem hiding this comment.
Nondeterministic listener cleanup in useChatStore.subscribe: the listener leaks if the send fails or the component unmounts because it only unsubscribes upon message reconciliation. Wrap the callback body in try/catch and store the unsubscribe function in a ref tied to the component lifecycle.
Kody rule violation: Provide error handlers to subscription/listener APIs
Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 158:
Nondeterministic listener cleanup in `useChatStore.subscribe`: the listener leaks if the send fails or the component unmounts because it only unsubscribes upon message reconciliation. Wrap the callback body in try/catch and store the unsubscribe function in a ref tied to the component lifecycle.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| void (async () => { | ||
| try { | ||
| await uploadAttachment(channelId, sent.ChatMessageId, { uri, name, type }); | ||
| } catch { |
There was a problem hiding this comment.
Swallowed exception: the catch block discards the error object without logging it. Change to } catch (err) { and add a structured log using logger.error before showing the generic toast.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 165:
Swallowed exception: the catch block discards the error object without logging it. Change to `} catch (err) {` and add a structured log using `logger.error` before showing the generic toast.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const name = uri.split('/').pop() || `photo-${Date.now()}.jpg`; | ||
| const type = getImageMimeType(uri, mimeType); | ||
|
|
||
| const unsubscribe = useChatStore.subscribe((state) => { |
There was a problem hiding this comment.
Memory leak in useChatStore.subscribe: the listener remains active indefinitely on component unmount or send failure because it only unsubscribes when the target message is reconciled. Capture the unsubscribe function and invoke it within a useEffect cleanup.
Kody rule violation: Proper memory management in event listeners
Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 158:
Memory leak in `useChatStore.subscribe`: the listener remains active indefinitely on component unmount or send failure because it only unsubscribes when the target message is reconciled. Capture the unsubscribe function and invoke it within a `useEffect` cleanup.
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.
Performance regression: the inline arrow function in the onPress JSX prop creates a new function instance on every render. 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/components/chat/message-composer.tsx:
Line 142:
Performance regression: the inline arrow function in the `onPress` JSX prop creates a new function instance on every render. 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.
| /** | ||
| * 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.
Incomplete JSDoc: the async function routerPushWithRetry lacks formal @returns and @throws tags. Add @returns {Promise<void>} and @throws {Error} to document the resolve value and rejection conditions.
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File src/lib/navigation.ts:
Line 11 to 15:
Incomplete JSDoc: the async function `routerPushWithRetry` lacks formal `@returns` and `@throws` tags. Add `@returns {Promise<void>}` and `@throws {Error}` to document the resolve value and rejection conditions.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!match) return false; | ||
| const channelId = match[2]; | ||
| if (/[/\\?#]/.test(channelId)) return false; | ||
| void routerPushWithRetry({ pathname: '/chat/[channelId]', params: { channelId } }, { maxAttempts: 20, retryDelayMs: 250 }).catch((error) => { |
There was a problem hiding this comment.
Magic numbers in the retry configuration: raw literals for attempts and delay lack context and reusability. Extract these values into named constants like DEEP_LINK_MAX_ATTEMPTS and DEEP_LINK_RETRY_DELAY_MS.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/services/push-notification.ts:
Line 46:
Magic numbers in the retry configuration: raw literals for attempts and delay lack context and reusability. Extract these values into named constants like `DEEP_LINK_MAX_ATTEMPTS` and `DEEP_LINK_RETRY_DELAY_MS`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!match) return false; | ||
| const channelId = match[2]; | ||
| if (/[/\\?#]/.test(channelId)) return false; | ||
| void routerPushWithRetry({ pathname: '/chat/[channelId]', params: { channelId } }, { maxAttempts: 20, retryDelayMs: 250 }).catch((error) => { |
There was a problem hiding this comment.
Hardcoded route path: the deep-link route '/chat/[channelId]' is inlined as a string literal. Define a centralized route constant like ROUTES.chatChannel in the navigation module to prevent drift and typos.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/services/push-notification.ts:
Line 46:
Hardcoded route path: the deep-link route `'/chat/[channelId]'` is inlined as a string literal. Define a centralized route constant like `ROUTES.chatChannel` in the navigation module to prevent drift and typos.
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 memory consumption in loadNewerMessages: the for(;;) loop makes sequential, uncapped API calls that load potentially thousands of messages into memory before returning control. Add a maximum page count or switch to lazy pagination to bound the work.
const MAX_DELTA_PAGES = 10;
for (let page = 0; page < MAX_DELTA_PAGES; page++) {
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]);
}Prompt for LLM
File src/stores/chat/store.ts:
Line 318 to 330:
Unbounded memory consumption in `loadNewerMessages`: the `for(;;)` loop makes sequential, uncapped API calls that load potentially thousands of messages into memory before returning control. Add a maximum page count or switch to lazy pagination to bound the work.
Suggested Code:
const MAX_DELTA_PAGES = 10;
for (let page = 0; page < MAX_DELTA_PAGES; page++) {
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]);
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| for (const item of [...get().outbox]) { | ||
| 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); |
There was a problem hiding this comment.
Broken retry mechanism: drainOutbox fails to schedule timers for items that fail mid-loop, leaving them stuck until a SignalR reconnect. Re-scan the current outbox (get().outbox) for items with 0 < Attempts < MAX_OUTBOX_ATTEMPTS to compute and schedule the backoff window.
for (const item of [...get().outbox]) {
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);
}
// Re-scan the CURRENT outbox (post-sendOutboxItem mutations) for items that
// still need retrying and schedule the nearest backoff window.
for (const item of get().outbox) {
const attempts = item.Attempts ?? 0;
if (attempts <= 0 || attempts >= MAX_OUTBOX_ATTEMPTS) continue;
const remaining = outboxRetryDelayMs(attempts) - (Date.now() - (item.LastAttemptAt ?? 0));
nextEligibleIn = Math.min(nextEligibleIn ?? Number.MAX_SAFE_INTEGER, Math.max(0, remaining));
}
if (nextEligibleIn !== null) scheduleOutboxDrain(nextEligibleIn);Prompt for LLM
File src/stores/chat/store.ts:
Line 417 to 428:
Broken retry mechanism: `drainOutbox` fails to schedule timers for items that fail mid-loop, leaving them stuck until a SignalR reconnect. Re-scan the current outbox (`get().outbox`) for items with `0 < Attempts < MAX_OUTBOX_ATTEMPTS` to compute and schedule the backoff window.
Suggested Code:
for (const item of [...get().outbox]) {
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);
}
// Re-scan the CURRENT outbox (post-sendOutboxItem mutations) for items that
// still need retrying and schedule the nearest backoff window.
for (const item of get().outbox) {
const attempts = item.Attempts ?? 0;
if (attempts <= 0 || attempts >= MAX_OUTBOX_ATTEMPTS) continue;
const remaining = outboxRetryDelayMs(attempts) - (Date.now() - (item.LastAttemptAt ?? 0));
nextEligibleIn = Math.min(nextEligibleIn ?? Number.MAX_SAFE_INTEGER, Math.max(0, 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.
| if (outboxDrainTimer) clearTimeout(outboxDrainTimer); | ||
| outboxDrainTimer = setTimeout(() => { | ||
| outboxDrainTimer = null; | ||
| void useChatStore.getState().drainOutbox(); |
There was a problem hiding this comment.
Unhandled promise rejection: invoking drainOutbox() with void discards the Promise without a .catch handler. Attach a handler to log unexpected throws, as the function only contains a try/finally block.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/stores/chat/store.ts:
Line 937:
Unhandled promise rejection: invoking `drainOutbox()` with `void` discards the Promise without a `.catch` handler. Attach a handler to log unexpected throws, as the function only contains a `try/finally` block.
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.
Missing correlation identifiers in the chatbot resend error log: the log omits the clientMessageId and pending.channelId. Add these available identifiers to the structured context to correlate the failure to the specific message.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File src/stores/chat/store.ts:
Line 404:
Missing correlation identifiers in the chatbot resend error log: the log omits the `clientMessageId` and `pending.channelId`. Add these available identifiers to the structured context to correlate the failure to the specific message.
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.
|
|
||
| <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 occurs because inline arrow functions in JSX props create new function instances on every render. Define the function outside the render method to resolve this.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/(app)/chat.tsx:
Line 123:
Performance degradation occurs because inline arrow functions in JSX props create new function instances on every render. Define the function outside the render method to resolve this.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| useEffect(() => { | ||
| const animations = dots.map((dot, index) => | ||
| Animated.loop( | ||
| Animated.sequence([Animated.delay(index * 150), Animated.timing(dot, { toValue: 1, duration: 400, useNativeDriver: true }), Animated.timing(dot, { toValue: 0.3, duration: 400, useNativeDriver: true })]) |
There was a problem hiding this comment.
Maintainability reduction caused by inlined magic numbers (150, 400, 1, and 0.3). Extract these values into named constants like STAGGER_DELAY and OPACITY_DIM above the component to make the intent self-documenting.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/components/chat/typing-indicator.tsx:
Line 15:
Maintainability reduction caused by inlined magic numbers (`150`, `400`, `1`, and `0.3`). Extract these values into named constants like `STAGGER_DELAY` and `OPACITY_DIM` above the component to make the intent self-documenting.
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.
| unsubscribeRef.current = useChatStore.subscribe((state) => { | ||
| const sent = (state.messagesByChannel[channelId] ?? []).find((m) => m._localAttachmentUri === uri); | ||
| if (!sent) return; | ||
| if (sent._localStatus === 'failed') { | ||
| unsubscribeRef.current?.(); | ||
| unsubscribeRef.current = null; | ||
| return; | ||
| } | ||
| if (sent.ChatMessageId.startsWith('local-')) return; | ||
| unsubscribeRef.current?.(); | ||
| unsubscribeRef.current = null; | ||
| void (async () => { | ||
| try { | ||
| await uploadAttachment(channelId, sent.ChatMessageId, { uri, name, type }); |
There was a problem hiding this comment.
Concurrent invocations of handleSendImage overwrite a shared unsubscribeRef, causing the first subscription to unsubscribe the wrong one, trigger repeated uploadAttachment calls, and silently skip the second image upload. Use a local unsubscribe variable for self-cleanup and register it with the ref only for the unmount effect.
const unsubscribe = useChatStore.subscribe((state) => {
const sent = (state.messagesByChannel[channelId] ?? []).find((m) => m._localAttachmentUri === uri);
if (!sent) return;
if (sent._localStatus === 'failed') {
unsubscribe();
if (unsubscribeRef.current === unsubscribe) unsubscribeRef.current = null;
return;
}
if (sent.ChatMessageId.startsWith('local-')) return;
unsubscribe();
if (unsubscribeRef.current === unsubscribe) unsubscribeRef.current = null;
void (async () => {
try {
await uploadAttachment(channelId, sent.ChatMessageId, { uri, name, type });
} catch {
useToastStore.getState().showToast('error', t('chat.attachment_failed'));
}
})();
});
unsubscribeRef.current = unsubscribe;Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 167 to 180:
Concurrent invocations of `handleSendImage` overwrite a shared `unsubscribeRef`, causing the first subscription to unsubscribe the wrong one, trigger repeated `uploadAttachment` calls, and silently skip the second image upload. Use a local `unsubscribe` variable for self-cleanup and register it with the ref only for the unmount effect.
Suggested Code:
const unsubscribe = useChatStore.subscribe((state) => {
const sent = (state.messagesByChannel[channelId] ?? []).find((m) => m._localAttachmentUri === uri);
if (!sent) return;
if (sent._localStatus === 'failed') {
unsubscribe();
if (unsubscribeRef.current === unsubscribe) unsubscribeRef.current = null;
return;
}
if (sent.ChatMessageId.startsWith('local-')) return;
unsubscribe();
if (unsubscribeRef.current === unsubscribe) unsubscribeRef.current = null;
void (async () => {
try {
await uploadAttachment(channelId, sent.ChatMessageId, { uri, name, type });
} catch {
useToastStore.getState().showToast('error', t('chat.attachment_failed'));
}
})();
});
unsubscribeRef.current = unsubscribe;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (sent._localStatus === 'failed') { | ||
| unsubscribeRef.current?.(); | ||
| unsubscribeRef.current = null; | ||
| return; | ||
| } |
There was a problem hiding this comment.
The _localStatus === 'failed' branch prematurely tears down the image-upload subscription, silently orphaning images because the subscription is destroyed before markOutboxFailed (store.ts:968-992) auto-retries and sendOutboxItem (store.ts:899) patches the message with a real ChatMessageId. Keep the subscription alive during transient failures to let uploadAttachment fire when a real ChatMessageId arrives.
if (sent._localStatus === 'failed') return; // transient failures stay in the outbox and are auto-retried; keep waiting for the real ChatMessageIdPrompt for LLM
File src/app/chat/[channelId].tsx:
Line 170 to 174:
The `_localStatus === 'failed'` branch prematurely tears down the image-upload subscription, silently orphaning images because the subscription is destroyed before `markOutboxFailed` (store.ts:968-992) auto-retries and `sendOutboxItem` (store.ts:899) patches the message with a real `ChatMessageId`. Keep the subscription alive during transient failures to let `uploadAttachment` fire when a real `ChatMessageId` arrives.
Suggested Code:
if (sent._localStatus === 'failed') return; // transient failures stay in the outbox and are auto-retried; keep waiting for the real ChatMessageId
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀
Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/app/(app)/chatbot.tsx (1)
92-98: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConfigure message-list virtualization.
Both message lists can grow without bounded render-window settings. Configure the required list properties and tune their values for message-row height.
src/app/(app)/chatbot.tsx#L92-L98: addremoveClippedSubviews,maxToRenderPerBatch, andwindowSize.src/app/chat/[channelId].tsx#L261-L270: addmaxToRenderPerBatchandwindowSize.As per coding guidelines, “Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, windowSize.”
🤖 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)/chatbot.tsx around lines 92 - 98, Configure FlatList virtualization for both message lists: in src/app/(app)/chatbot.tsx lines 92-98, add removeClippedSubviews, maxToRenderPerBatch, and windowSize; in src/app/chat/[channelId].tsx lines 261-270, add maxToRenderPerBatch and windowSize. Tune the values for the message-row height while preserving the existing list rendering behavior.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@src/app/`(app)/chatbot.tsx:
- Around line 92-98: Configure FlatList virtualization for both message lists:
in src/app/(app)/chatbot.tsx lines 92-98, add removeClippedSubviews,
maxToRenderPerBatch, and windowSize; in src/app/chat/[channelId].tsx lines
261-270, add maxToRenderPerBatch and windowSize. Tune the values for the
message-row height while preserving the existing list rendering behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f8998352-26f1-401f-bf28-896c2e8e72f9
📒 Files selected for processing (14)
.gitignoresrc/app/(app)/chat.tsxsrc/app/(app)/chatbot.tsxsrc/app/chat/[channelId].tsxsrc/app/chat/thread/[messageId].tsxsrc/components/chat/chat-utils.tssrc/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/ui/html-renderer/index.web.tsxsrc/models/v4/chat/index.tssrc/stores/chat/store.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/components/chat/new-conversation-sheet.tsx
- src/app/chat/thread/[messageId].tsx
- src/models/v4/chat/index.ts
- src/components/chat/message-composer.tsx
- src/components/chat/typing-indicator.tsx
- src/components/chat/message-actions-sheet.tsx
- src/components/chat/chat-utils.ts
- src/stores/chat/store.ts
- src/components/chat/message-bubble.tsx
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
2 participants
This PR implements a comprehensive chat system and AI chatbot assistant for the Resgrid Unit mobile app. Key additions include:
Chat Functionality:
Chatbot/Assistant:
Reliability & Infrastructure:
t:andg:)Navigation:
Summary by CodeRabbit