-
Notifications
You must be signed in to change notification settings - Fork 0
RG-T117 Chatbot fixes #36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,13 @@ | |
| import { useTranslation } from 'react-i18next'; | ||
| import { FlatList, Platform } from 'react-native'; | ||
|
|
||
| import { copyToClipboard } from '@/components/chat/chat-utils'; | ||
| import { MessageActionsSheet } from '@/components/chat/message-actions-sheet'; | ||
| import { MessageBubble } from '@/components/chat/message-bubble'; | ||
| import { TypingDots } from '@/components/chat/typing-indicator'; | ||
| import { Actionsheet, ActionsheetBackdrop, ActionsheetContent, ActionsheetDragIndicator, ActionsheetDragIndicatorWrapper } from '@/components/ui/actionsheet'; | ||
| import { Box } from '@/components/ui/box'; | ||
| import { Button, ButtonText } from '@/components/ui/button'; | ||
| import { Center } from '@/components/ui/center'; | ||
| import { FocusAwareStatusBar } from '@/components/ui/focus-aware-status-bar'; | ||
| import { HStack } from '@/components/ui/hstack'; | ||
|
|
@@ -15,19 +19,26 @@ | |
| import { Pressable } from '@/components/ui/pressable'; | ||
| import { Spinner } from '@/components/ui/spinner'; | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 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 Replace the inline 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| import useAuthStore from '@/stores/auth/store'; | ||
| import { useChatStore } from '@/stores/chat/store'; | ||
| import { useChatSystemStatus } from '@/stores/feature-flags/store'; | ||
| import { securityStore } from '@/stores/security/store'; | ||
| import { useToastStore } from '@/stores/toast/store'; | ||
|
|
||
| export default function ChatbotScreen() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 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 Use the configured component declaration form at each site.
📍 Affects 3 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| const { t } = useTranslation(); | ||
| const currentUserId = useAuthStore((s) => s.userId); | ||
| const chatbotChannelId = useChatStore((s) => s.chatbotChannelId); | ||
| const chatbotTyping = useChatStore((s) => s.chatbotTyping); | ||
| const messages = useChatStore((s) => (chatbotChannelId ? s.messagesByChannel[chatbotChannelId] : undefined)); | ||
| const isModerator = !!securityStore((s) => s.rights)?.IsAdmin; | ||
| const [text, setText] = useState(''); | ||
| const [actionsMessage, setActionsMessage] = useState<ChatMessageResultData | null>(null); | ||
| const [editMessage, setEditMessage] = useState<ChatMessageResultData | null>(null); | ||
| const [editText, setEditText] = useState(''); | ||
| const chatStatus = useChatSystemStatus(); | ||
| const isChatEnabled = chatStatus === 'enabled'; | ||
|
|
||
|
|
@@ -61,7 +72,7 @@ | |
|
|
||
| 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} /> | ||
|
Check warning on line 75 in src/app/(app)/chatbot.tsx
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inline arrow functions and Kody rule violation: Avoid using .bind() or arrow functions in JSX props Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| ), | ||
| [currentUserId] | ||
| ); | ||
|
|
@@ -134,6 +145,57 @@ | |
| </Pressable> | ||
| </HStack> | ||
| </KeyboardAvoidingView> | ||
|
|
||
| {/* Restricted actions for assistant messages: copy, edit own, pin (moderator), flag. */} | ||
| <MessageActionsSheet | ||
| message={actionsMessage} | ||
| isOpen={actionsMessage !== null} | ||
| onClose={() => setActionsMessage(null)} | ||
| isOwn={!!actionsMessage?.SenderUserId && actionsMessage.SenderUserId === currentUserId} | ||
| isModerator={isModerator} | ||
| assistant | ||
| onReact={() => undefined} | ||
| onReply={() => undefined} | ||
| onCopy={async (m) => { | ||
| const ok = await copyToClipboard(m.Body ?? ''); | ||
| useToastStore.getState().showToast(ok ? 'success' : 'info', ok ? t('chat.copied') : t('chat.copy_unavailable')); | ||
|
Comment on lines
+160
to
+161
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled promise rejection from Kody rule violation: Handle async operations with proper error handling Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Comment on lines
+160
to
+161
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing error handling around external clipboard API call Kody rule violation: Add try-catch blocks for external calls Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| }} | ||
| onEdit={(m) => { | ||
| setEditMessage(m); | ||
| setEditText(m.Body ?? ''); | ||
| }} | ||
| onDelete={() => undefined} | ||
| onFlag={(m, reason) => useChatStore.getState().flagMessage(m.ChatMessageId, reason)} | ||
| onTogglePin={(m, pinned) => chatbotChannelId && useChatStore.getState().togglePin(m.ChatMessageId, chatbotChannelId, pinned)} | ||
| onModeratorDelete={() => undefined} | ||
| /> | ||
|
|
||
| {/* Edit own message */} | ||
| <Actionsheet isOpen={editMessage !== null} onClose={() => setEditMessage(null)}> | ||
| <ActionsheetBackdrop /> | ||
| <ActionsheetContent> | ||
| <ActionsheetDragIndicatorWrapper> | ||
| <ActionsheetDragIndicator /> | ||
| </ActionsheetDragIndicatorWrapper> | ||
| <VStack className="w-full p-2" space="md"> | ||
| <Text className="text-base font-semibold text-typography-900">{t('chat.edit_message')}</Text> | ||
| <Textarea> | ||
| <TextareaInput value={editText} onChangeText={setEditText} multiline /> | ||
| </Textarea> | ||
| <Button | ||
| className="bg-primary-600" | ||
| onPress={() => { | ||
| if (editMessage && chatbotChannelId && editText.trim()) { | ||
| void useChatStore.getState().editMessage(editMessage.ChatMessageId, chatbotChannelId, editText.trim()); | ||
| } | ||
| setEditMessage(null); | ||
| }} | ||
| > | ||
| <ButtonText>{t('chat.save')}</ButtonText> | ||
| </Button> | ||
| </VStack> | ||
| </ActionsheetContent> | ||
| </Actionsheet> | ||
| </Box> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,9 +61,15 @@ export default function ChannelConversationScreen() { | |
| const [editText, setEditText] = useState(''); | ||
| const [imageUri, setImageUri] = useState<string | null>(null); | ||
| const [presenceIds, setPresenceIds] = useState<Set<string>>(new Set()); | ||
| const [resolveAttempted, setResolveAttempted] = useState(false); | ||
| const unsubscribeRef = useRef<(() => void) | null>(null); | ||
|
|
||
| const isDm = channel?.ChannelType === ChatChannelType.DirectMessage; | ||
| const isChatbot = channel?.ChannelType === ChatChannelType.Chatbot; | ||
| // Deep links (push notifications, cold starts) can arrive before the channel | ||
| // list loads; the channel type is unknown until then. Treat a completed fetch | ||
| // with no match as resolved so unknown channels keep the generic screen. | ||
| const isResolved = !!channel || resolveAttempted; | ||
| const showSender = !isDm; | ||
| // IC delta: in command-type channels the user posts as the Incident Commander. | ||
| // The server validates the user actually holds command (CanSendAsIcAsync) and rejects otherwise. | ||
|
|
@@ -72,11 +78,21 @@ export default function ChannelConversationScreen() { | |
| // Newest-first for the inverted list. | ||
| const inverted = useMemo(() => (messages ? messages.slice().reverse() : []), [messages]); | ||
|
|
||
| // Mount: activate channel, join hub, load history and members. | ||
| // Resolve the channel identity for deep links before mounting the generic view. | ||
| useEffect(() => { | ||
| if (channel || resolveAttempted || !isChatEnabled) return; | ||
| void useChatStore | ||
| .getState() | ||
| .fetchChannels() | ||
| .finally(() => setResolveAttempted(true)); | ||
|
Comment on lines
+84
to
+87
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled promise rejection occurs because the promise returned by Kody rule violation: Handle async operations with proper error handling Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| }, [channel, resolveAttempted, isChatEnabled]); | ||
|
|
||
| // Mount: activate channel, join hub, load history and members. Assistant | ||
| // conversations are handled by the dedicated chatbot screen — never join or | ||
| // load them here, and wait for unresolved deep links to identify first. | ||
| useFocusEffect( | ||
| useCallback(() => { | ||
| if (!isChatEnabled) return; | ||
| if (!channelId) return; | ||
| if (!channelId || !isChatEnabled || !isResolved || isChatbot) return; | ||
| const store = useChatStore.getState(); | ||
| store.setActiveChannel(channelId); | ||
| void store.joinChannel(channelId); | ||
|
|
@@ -85,7 +101,7 @@ export default function ChannelConversationScreen() { | |
| return () => { | ||
| useChatStore.getState().setActiveChannel(null); | ||
| }; | ||
| }, [channelId, isChatEnabled]) | ||
| }, [channelId, isChatEnabled, isResolved, isChatbot]) | ||
| ); | ||
|
|
||
| // Fetch presence for the channel members (for the header online dot). | ||
|
|
@@ -107,10 +123,10 @@ export default function ChannelConversationScreen() { | |
| // Mark read whenever the newest message changes while viewing. | ||
| useEffect(() => { | ||
| if (!isChatEnabled) return; | ||
| if (channelId && inverted.length > 0) { | ||
| if (channelId && isResolved && !isChatbot && inverted.length > 0) { | ||
| void useChatStore.getState().markChannelRead(channelId); | ||
| } | ||
| }, [channelId, inverted.length, isChatEnabled]); | ||
| }, [channelId, inverted.length, isChatEnabled, isResolved, isChatbot]); | ||
|
|
||
| const otherOnline = useMemo(() => { | ||
| if (!isDm) return false; | ||
|
|
@@ -267,6 +283,23 @@ export default function ChannelConversationScreen() { | |
| return <Redirect href="/" />; | ||
| } | ||
|
|
||
| // Deep link to a channel that isn't loaded yet: wait for the channel list so | ||
| // assistant conversations never mount the full-featured view. | ||
| if (!isResolved) { | ||
| return ( | ||
| <Box className="size-full flex-1 items-center justify-center bg-background-0"> | ||
| <Stack.Screen options={{ title, headerShown: true, headerBackTitle: '' }} /> | ||
| <Spinner /> | ||
| </Box> | ||
| ); | ||
| } | ||
|
|
||
| // Assistant conversations always use the dedicated restricted screen (text only, | ||
| // no reactions/threads/deletes) — catch deep links and stale routes here. | ||
| if (isChatbot) { | ||
| return <Redirect href={'/chatbot' as Href} />; | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| return ( | ||
| <Box className="size-full flex-1 bg-background-0"> | ||
| <Stack.Screen | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import * as Clipboard from 'expo-clipboard'; | ||
| import { type TFunction } from 'i18next'; | ||
|
|
||
| import { getAvatarUrl } from '@/lib/utils'; | ||
|
|
@@ -105,9 +106,9 @@ export function hasLink(body?: string | null): boolean { | |
| } | ||
|
|
||
| /** | ||
| * Copies text to the clipboard. Works on web/Electron via the async Clipboard | ||
| * API; native returns false (no clipboard native module is installed) so callers | ||
| * can surface an appropriate message. | ||
| * Copies text to the clipboard. Uses the async Clipboard API on web/Electron | ||
| * and expo-clipboard on native; returns false only when both are unavailable | ||
| * or the write fails, so callers can surface an appropriate message. | ||
| */ | ||
| export async function copyToClipboard(text: string): Promise<boolean> { | ||
| try { | ||
|
|
@@ -117,9 +118,13 @@ export async function copyToClipboard(text: string): Promise<boolean> { | |
| return true; | ||
| } | ||
| } catch { | ||
| // ignore and fall through | ||
| // ignore and fall through to the native module | ||
| } | ||
| try { | ||
| return await Clipboard.setStringAsync(text); | ||
| } catch { | ||
| return false; | ||
|
Comment on lines
+125
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Silent exception swallowing occurs in the Kody rule violation: Avoid empty catch blocks Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| } | ||
|
Comment on lines
+123
to
127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Returning try {
await Clipboard.setStringAsync(text);
return true;
} catch {
return false;
}Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| return false; | ||
| } | ||
|
|
||
| const IMAGE_MIME_BY_EXTENSION: Record<string, string> = { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded route string literal
'/chatbot'(also insrc/app/chat/[channelId].tsx:273). Centralize route paths as constants/enums, e.g.const Routes = { Chatbot: '/chatbot', Chat: '/chat' } as const, and referenceRoutes.Chatbotinstead of the raw string.Kody rule violation: Centralize string constants
Prompt for LLM
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.