Conversation
|
Warning Review limit reached
Next review available in: 21 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR updates chatbot API response handling, routes chatbot channels to a dedicated screen, adds assistant message actions, controls urgent messaging, and filters self-generated acknowledgement events. ChangesChatbot flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ChatList
participant ChatbotScreen
participant MessageActionsSheet
participant ChatbotAPI
ChatList->>ChatbotScreen: Open chatbot channel
ChatbotScreen->>MessageActionsSheet: Open assistant message actions
MessageActionsSheet->>ChatbotScreen: Select edit action
ChatbotScreen->>ChatbotAPI: Submit trimmed message edit
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/app/(app)/chatbot.tsx (2)
75-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFormat these JSX prop lists across multiple lines.
The active static-analysis check reports formatting warnings at both sites.
src/app/(app)/chatbot.tsx#L75-L75: splitMessageBubbleprops across lines.src/app/chat/thread/[messageId].tsx#L139-L139: splitMessageComposerprops across lines.🤖 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 at line 75, Reformat the JSX prop lists for MessageBubble in src/app/(app)/chatbot.tsx at lines 75-75 and MessageComposer in src/app/chat/thread/[messageId].tsx at lines 139-139 across multiple lines to satisfy the static-analysis formatting check; preserve all existing props and behavior.Source: Linters/SAST tools
75-75: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winExtract stable callbacks from the list renderer and action sheet.
onToggleReaction={() => undefined}allocates a new callback for every rendered message. The new action-sheet and save handlers also create new callback identities on each render.Define stable no-op callbacks outside the renderer. Use
useCallbackfor handlers passed toMessageActionsSheetand the edit save action.As per coding guidelines, avoid anonymous functions in
renderItemand event handlers.Also applies to: 159-192
🤖 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 at line 75, Extract the inline no-op reaction callback used by the MessageBubble list renderer into a stable callback defined outside renderItem. Wrap the MessageActionsSheet handlers and edit save handler in useCallback, then pass those named stable callbacks instead of anonymous functions in renderItem and event handlers.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/app/`(app)/chatbot.tsx:
- Line 24: Update the ChatMessageResultData import in the chatbot module from an
inline type specifier to a type-only import declaration, keeping it grouped with
the other imports after value imports.
- Line 31: Update the component declarations to use the configured React.FC
form: declare ChatbotScreen as React.FC in src/app/(app)/chatbot.tsx (lines
31-31), MessageActionsSheet as React.FC<MessageActionsSheetProps> in
src/components/chat/message-actions-sheet.tsx (lines 31-31), and MessageComposer
as React.FC<MessageComposerProps> in src/components/chat/message-composer.tsx
(lines 32-32).
In `@src/app/chat/`[channelId].tsx:
- Around line 270-274: Update the channel-resolution flow in the chat screen so
it loads or resolves the channel type before rendering the generic conversation.
Keep the screen in a loading state while channel information is unknown, then
redirect resolved Chatbot channels to `/chatbot` before starting generic
conversation effects such as joining or loading messages; preserve the existing
path for non-chatbot channels.
In `@src/stores/chat/store.ts`:
- Around line 734-738: Update the acknowledgement event model used by
handleAckRequired to define and consume the real sender identifier field in the
shared schema, rather than intersecting ChatAckResultData locally. Ensure
self-originating acknowledgements are filtered using that schema before adding
to pendingAcks, and keep fetchPendingAcks consistent so those entries are not
reintroduced.
---
Nitpick comments:
In `@src/app/`(app)/chatbot.tsx:
- Line 75: Reformat the JSX prop lists for MessageBubble in
src/app/(app)/chatbot.tsx at lines 75-75 and MessageComposer in
src/app/chat/thread/[messageId].tsx at lines 139-139 across multiple lines to
satisfy the static-analysis formatting check; preserve all existing props and
behavior.
- Line 75: Extract the inline no-op reaction callback used by the MessageBubble
list renderer into a stable callback defined outside renderItem. Wrap the
MessageActionsSheet handlers and edit save handler in useCallback, then pass
those named stable callbacks instead of anonymous functions in renderItem and
event handlers.
🪄 Autofix
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: 9d384393-9531-49f8-b96c-e216014c79b4
📒 Files selected for processing (9)
src/api/chat/chatbot.tssrc/app/(app)/chat.tsxsrc/app/(app)/chatbot.tsxsrc/app/chat/[channelId].tsxsrc/app/chat/thread/[messageId].tsxsrc/components/chat/message-actions-sheet.tsxsrc/components/chat/message-composer.tsxsrc/models/v4/chat/chatbotModels.tssrc/stores/chat/store.ts
| import { Text } from '@/components/ui/text'; | ||
| import { Textarea, TextareaInput } from '@/components/ui/textarea'; | ||
| import { VStack } from '@/components/ui/vstack'; | ||
| import { type ChatMessageResultData } from '@/models/v4/chat'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Locate chatbot.tsx files:\n'
fd -a 'chatbot\.tsx$' . || true
file="$(fd 'chatbot\.tsx$' . | head -n 1 || true)"
if [ -n "$file" ]; then
printf '\nOutline for %s:\n' "$file"
ast-grep outline "$file" --view compact || true
printf '\nFirst 45 lines of %s:\n' "$file"
sed -n '1,45p' "$file" | cat -n
fi
printf '\nSearch for ChatMessageResultData imports/usages:\n'
rg -n "import\s*(type\s*)?\{\s*ChatMessageResultData\s*\}|\bChatMessageResultData\b" -g '*.ts' -g '*.tsx' . || trueRepository: Resgrid/IC
Length of output: 10371
Use a type-only import for ChatMessageResultData.
Replace the inline type specifier on line 24 with import type { ChatMessageResultData } from '@/models/v4/chat';. Keep this type-only import with the other imports and after value imports.
🤖 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 at line 24, Update the ChatMessageResultData
import in the chatbot module from an inline type specifier to a type-only import
declaration, keeping it grouped with the other imports after value imports.
Source: Coding guidelines
| import { securityStore } from '@/stores/security/store'; | ||
| import { useToastStore } from '@/stores/toast/store'; | ||
|
|
||
| export default function ChatbotScreen() { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -H 'tsconfig*.json' . -x sed -n '1,220p' {}
rg -n --glob '*.{ts,tsx}' 'import type \* as React|React\.FC' srcRepository: Resgrid/IC
Length of output: 20479
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
'src/app/(app)/chatbot.tsx' \
'src/components/chat/message-actions-sheet.tsx' \
'src/components/chat/message-composer.tsx'
do
echo "===== $f ====="
fd -a "$f" . | sed 's#^\./##'
wc -l "$f"
sed -n '1,80p' "$f" | cat -n
done
echo "===== React imports/usages in target files ====="
rg -n 'import (React|type React|.*React)|React\.FC' 'src/app/(app)/chatbot.tsx' 'src/components/chat/message-actions-sheet.tsx' 'src/components/chat/message-composer.tsx'Repository: Resgrid/IC
Length of output: 12718
Type these React components with React.FC.
Use the configured component declaration form at each site.
src/app/(app)/chatbot.tsx#L31-L31: declareChatbotScreenasReact.FC.src/components/chat/message-actions-sheet.tsx#L31-L31: declareMessageActionsSheetasReact.FC<MessageActionsSheetProps>.src/components/chat/message-composer.tsx#L32-L32: declareMessageComposerasReact.FC<MessageComposerProps>.
📍 Affects 3 files
src/app/(app)/chatbot.tsx#L31-L31(this comment)src/components/chat/message-actions-sheet.tsx#L31-L31src/components/chat/message-composer.tsx#L32-L32
🤖 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 at line 31, Update the component declarations to
use the configured React.FC form: declare ChatbotScreen as React.FC in
src/app/(app)/chatbot.tsx (lines 31-31), MessageActionsSheet as
React.FC<MessageActionsSheetProps> in
src/components/chat/message-actions-sheet.tsx (lines 31-31), and MessageComposer
as React.FC<MessageComposerProps> in src/components/chat/message-composer.tsx
(lines 32-32).
Source: Coding guidelines
| const ack = parseEventData<ChatAckResultData & { SenderUserId?: string | null }>(raw); | ||
| if (!ack || !ack.ChatMessageId) return; | ||
| // The sender never has to acknowledge their own urgent message. | ||
| if (ack.SenderUserId && ack.SenderUserId === currentUserId()) return; | ||
| set((s) => (s.pendingAcks.some((a) => a.ChatMessageId === ack.ChatMessageId) ? {} : { pendingAcks: [...s.pendingAcks, ack] })); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'SenderUserId|senderUserId|handleAckRequired|AckRequired|getMyPendingAcks|parseEventData' .Repository: Resgrid/IC
Length of output: 34088
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Relevant model definitions"
sed -n '115,145p' src/models/v4/chat/chatModels.ts
echo
echo "## Relevant store state/init/reset/fetch handling"
sed -n '80,108p' src/stores/chat/store.ts
sed -n '160,185p' src/stores/chat/store.ts
sed -n '513,519p' src/stores/chat/store.ts
echo
echo "## Search for ack event backend definitions/usages outside client"
rg -n 'AckRequired|ack required|AckResult|GetMyPendingAcks|ChatAckResultData|ChatAck' . --glob '!node_modules' --glob '!dist' --glob '!build'Repository: Resgrid/IC
Length of output: 4984
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Behavioral probe: current handler with self-owner payload variations"
node - <<'JS'
const currentUserId = () => 'u1';
function handleAckRequired(raw) {
const ack = raw;
if (!ack || !ack.ChatMessageId) return 'invalid';
if (ack.SenderUserId && ack.SenderUserId === currentUserId()) return 'self_filtered';
const prev = [];
return prev.some((a) => a.ChatMessageId === ack.ChatMessageId) ? 'no_change' : { pendingAcks: [...prev, ack], change: 'added' };
}
for (const payload of [
{ ChatMessageId: 'm1', SenderUserId: 'u1' },
{ ChatMessageId: 'm1', SenderUserId: 'u2' },
{ ChatMessageId: 'm1', senderUserId: 'u1' },
{ ChatMessageId: 'm1' },
]) {
console.log(JSON.stringify(payload), '=>', JSON.stringify(handleAckRequired(payload)));
}
JSRepository: Resgrid/IC
Length of output: 614
Define and use a real acknowledgement event schema.
ChatAckResultData does not include SenderUserId; handleAckRequired currently relies on a field outside the shared model. If the SignalR event omits that field or uses an alternate name, self-originating acknowledgements can be added to pendingAcks and later reloaded from fetchPendingAcks because it replaces pendingAcks with the API payload.
🤖 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 734 - 738, Update the acknowledgement
event model used by handleAckRequired to define and consume the real sender
identifier field in the shared schema, rather than intersecting
ChatAckResultData locally. Ensure self-originating acknowledgements are filtered
using that schema before adding to pendingAcks, and keep fetchPendingAcks
consistent so those entries are not reintroduced.
This comment has been minimized.
This comment has been minimized.
| // (text only, no reactions/threads/deletes) instead of the generic conversation. | ||
| const channel = useChatStore.getState().channels.find((c) => c.ChatChannelId === channelId); | ||
| if (channel?.ChannelType === ChatChannelType.Chatbot) { | ||
| router.push('/chatbot' as Href); |
There was a problem hiding this comment.
Hardcoded route string literal '/chatbot' (also in src/app/chat/[channelId].tsx:273). Centralize route paths as constants/enums, e.g. const Routes = { Chatbot: '/chatbot', Chat: '/chat' } as const, and reference Routes.Chatbot instead of the raw string.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/app/(app)/chat.tsx:
Line 110:
Hardcoded route string literal `'/chatbot'` (also in `src/app/chat/[channelId].tsx:273`). Centralize route paths as constants/enums, e.g. `const Routes = { Chatbot: '/chatbot', Chat: '/chat' } as const`, and reference `Routes.Chatbot` instead of the raw string.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const renderItem = useCallback( | ||
| ({ item }: { item: ChatMessageResultData }) => ( | ||
| <MessageBubble message={item} isOwn={!!item.SenderUserId && item.SenderUserId === currentUserId} showSender={false} currentUserId={currentUserId} onLongPress={() => undefined} onToggleReaction={() => undefined} /> | ||
| <MessageBubble message={item} isOwn={!!item.SenderUserId && item.SenderUserId === currentUserId} showSender={false} currentUserId={currentUserId} onLongPress={setActionsMessage} onToggleReaction={() => undefined} /> |
There was a problem hiding this comment.
Inline arrow functions and .bind() calls in JSX props create new function references on every render, impacting performance. Extract these handlers to stable useCallback definitions outside the render method.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/(app)/chatbot.tsx:
Line 75:
Inline arrow functions and `.bind()` calls in JSX props create new function references on every render, impacting performance. Extract these handlers to stable `useCallback` definitions outside the render method.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const ok = await copyToClipboard(m.Body ?? ''); | ||
| useToastStore.getState().showToast(ok ? 'success' : 'info', ok ? t('chat.copied') : t('chat.copy_unavailable')); |
There was a problem hiding this comment.
Unhandled promise rejection from await copyToClipboard(...) in the onCopy async handler (chatbot.tsx:189). Wrap the call in try/catch and surface a user-facing error toast on failure.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/app/(app)/chatbot.tsx:
Line 160 to 161:
Unhandled promise rejection from `await copyToClipboard(...)` in the `onCopy` async handler (chatbot.tsx:189). Wrap the call in try/catch and surface a user-facing error toast on failure.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const ok = await copyToClipboard(m.Body ?? ''); | ||
| useToastStore.getState().showToast(ok ? 'success' : 'info', ok ? t('chat.copied') : t('chat.copy_unavailable')); |
There was a problem hiding this comment.
Missing error handling around external clipboard API call copyToClipboard. Wrap in try/catch with context, log/handle the failure with the relevant message id, and surface a user-facing error toast.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/app/(app)/chatbot.tsx:
Line 160 to 161:
Missing error handling around external clipboard API call `copyToClipboard`. Wrap in try/catch with context, log/handle the failure with the relevant message id, and surface a user-facing error toast.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| close(); | ||
| }} | ||
| > | ||
| <MessageSquare size={18} color="#6b7280" /> |
There was a problem hiding this comment.
Inline magic number 18 for icon size. Extract it into a named constant such as const ICON_SIZE_SM = 18 or reference an existing design-token constant for consistency and maintainability.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/components/chat/message-actions-sheet.tsx:
Line 104:
Inline magic number `18` for icon size. Extract it into a named constant such as `const ICON_SIZE_SM = 18` or reference an existing design-token constant for consistency and maintainability.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const ack = parseEventData<ChatAckResultData & { SenderUserId?: string | null }>(raw); | ||
| if (!ack || !ack.ChatMessageId) return; | ||
| // The sender never has to acknowledge their own urgent message. | ||
| if (ack.SenderUserId && ack.SenderUserId === currentUserId()) return; |
There was a problem hiding this comment.
Property name mismatch causes the self-ack guard to never trigger. The ChatAckResultData interface (chatModels.ts:127-135) defines only UserId, not SenderUserId, so ack.SenderUserId is always undefined and the sender is still prompted to acknowledge their own urgent message. Confirm the backend payload field name and read the correct property.
// Confirm the backend field that identifies the message sender on the ack event,
// then read that property (e.g. ack.SenderUserId if the backend genuinely emits it).
// If the backend only emits UserId, the guard must compare against that field instead.Prompt for LLM
File src/stores/chat/store.ts:
Line 734 to 737:
Property name mismatch causes the self-ack guard to never trigger. The `ChatAckResultData` interface (chatModels.ts:127-135) defines only `UserId`, not `SenderUserId`, so `ack.SenderUserId` is always undefined and the sender is still prompted to acknowledge their own urgent message. Confirm the backend payload field name and read the correct property.
Suggested Code:
// Confirm the backend field that identifies the message sender on the ack event,
// then read that property (e.g. ack.SenderUserId if the backend genuinely emits it).
// If the backend only emits UserId, the guard must compare against that field instead.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| void useChatStore | ||
| .getState() | ||
| .fetchChannels() | ||
| .finally(() => setResolveAttempted(true)); |
There was a problem hiding this comment.
Unhandled promise rejection occurs because the promise returned by fetchChannels() chains .finally() without a .catch() handler. Add a .catch() handler before .finally() to log the error, satisfying Rule [1] and preventing app crashes or resolveAttempted remaining false indefinitely.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 84 to 87:
Unhandled promise rejection occurs because the promise returned by `fetchChannels()` chains `.finally()` without a `.catch()` handler. Add a `.catch()` handler before `.finally()` to log the error, satisfying Rule [1] and preventing app crashes or `resolveAttempted` remaining `false` indefinitely.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| try { | ||
| return await Clipboard.setStringAsync(text); | ||
| } catch { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Returning await Clipboard.setStringAsync(text) resolves with undefined instead of true because setStringAsync returns Promise<void>. This causes both callers ([channelId].tsx:373 and chatbot.tsx:160) to falsely report 'copy unavailable'; explicitly return true after awaiting the call to resolve the issue.
try {
await Clipboard.setStringAsync(text);
return true;
} catch {
return false;
}Prompt for LLM
File src/components/chat/chat-utils.ts:
Line 123 to 127:
Returning `await Clipboard.setStringAsync(text)` resolves with `undefined` instead of `true` because `setStringAsync` returns `Promise<void>`. This causes both callers (`[channelId].tsx:373` and `chatbot.tsx:160`) to falsely report 'copy unavailable'; explicitly return `true` after awaiting the call to resolve the issue.
Suggested Code:
try {
await Clipboard.setStringAsync(text);
return true;
} catch {
return false;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } catch { | ||
| return false; |
There was a problem hiding this comment.
Silent exception swallowing occurs in the catch block for the expo-clipboard call, violating Rule [28] by returning false without logging. Capture the exception variable and log it with context before returning to ensure explicit error handling.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/components/chat/chat-utils.ts:
Line 125 to 126:
Silent exception swallowing occurs in the `catch` block for the `expo-clipboard` call, violating Rule [28] by returning `false` without logging. Capture the exception variable and log it with context before returning to ensure explicit error handling.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
PR Description: RG-T117 Chatbot Fixes
This PR addresses several issues in the chatbot (assistant) conversation feature and related chat functionality:
Chatbot API Response Parsing Fix
{ Data }response envelope. The models and API calls were updated to reflect that chatbot endpoints wrap payloads consistently with the rest of the Chat API (previously treated as plain objects).Chatbot Channel Routing
/chatbot) rather than the generic conversation view. This is enforced both in channel list navigation and as a redirect guard for deep links or stale routes pointing to a chatbot channel.Restricted Message Actions for Chatbot
assistantmode that shows only relevant actions: copy, edit own messages, pin (moderator), and flag. Reactions, reply-in-thread, delete, and moderator delete are hidden for assistant conversations. An inline edit sheet was also added.Urgent Message Fixes