-
Notifications
You must be signed in to change notification settings - Fork 0
Develop #37
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
Develop #37
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 |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { type ChatbotChannelResponse, type ChatbotSendResponse, type ChatbotSessionResponse } from '@/models/v4/chat'; | ||
| import { type ChatbotChannelResponse, type ChatbotSendResponse, type ChatbotSessionResponse, type IncidentAssistantAnswerResponse, type IncidentAssistantSuggestionsResponse } from '@/models/v4/chat'; | ||
|
|
||
| import { api } from '../common/client'; | ||
|
|
||
|
|
@@ -27,3 +27,19 @@ export const newChatbotSession = async () => { | |
| const response = await api.post<ChatbotSessionResponse>(`${CHATBOT}/NewChatSession`, {}); | ||
| return response.data; | ||
| }; | ||
|
|
||
| /** | ||
| * Asks the incident assistant a command-board question and gets the answer back in the same | ||
| * round-trip. `callId` scopes the question to the board the caller has open, so "PAR" resolves | ||
| * against that incident rather than guessing among the department's active commands. | ||
| */ | ||
| export const askIncidentAssistant = async (callId: number, question: string, signal?: AbortSignal) => { | ||
| const response = await api.post<IncidentAssistantAnswerResponse>(`${CHATBOT}/AskIncident`, { Question: question, CallId: callId }, { signal }); | ||
|
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: this external HTTP call to 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. |
||
| return response.data?.Data ?? null; | ||
| }; | ||
|
|
||
| /** Server-side suggested questions for an incident, from the ICS playbook it infers for the call. */ | ||
| export const getIncidentAssistantSuggestions = async (callId: number, signal?: AbortSignal) => { | ||
| const response = await api.get<IncidentAssistantSuggestionsResponse>(`${CHATBOT}/IncidentSuggestions`, { params: { callId }, signal }); | ||
| return response.data?.Data ?? null; | ||
|
Comment on lines
+36
to
+44
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 Use the approved API endpoint factories. These methods call the API client directly. Define them through As per coding guidelines, “Implement API modules with 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| /** | ||
| * Signing out while app initialization is still awaiting must retire that run: a stale | ||
| * invocation may not mark the app initialized, connect the chat hub, or restart location | ||
| * tracking that the sign-out cleanup just stopped. | ||
| * | ||
| * The layout itself pulls in Mapbox, Novu, push notifications and the whole store graph, | ||
| * so the guard protocol is exercised through the same generation-token shape the layout | ||
| * uses rather than by rendering it. | ||
| */ | ||
| import { act, renderHook } from '@testing-library/react-native'; | ||
| import React from 'react'; | ||
|
|
||
| interface Deferred { | ||
| promise: Promise<void>; | ||
| resolve: () => void; | ||
| } | ||
|
|
||
| function deferred(): Deferred { | ||
| let resolve: () => void = () => undefined; | ||
| const promise = new Promise<void>((res) => { | ||
| resolve = res; | ||
| }); | ||
| return { promise, resolve }; | ||
| } | ||
|
|
||
| /** Mirrors the layout's initializeApp guard: generation captured at start, checked after each await. */ | ||
| function useInitGuard(gate: Deferred, effects: { connectHub: jest.Mock; startLocation: jest.Mock; markInitialized: jest.Mock }) { | ||
| const initGeneration = React.useRef(0); | ||
| const isInitializing = React.useRef(false); | ||
|
|
||
| const initialize = React.useCallback(async () => { | ||
| if (isInitializing.current) return; | ||
| isInitializing.current = true; | ||
| const generation = (initGeneration.current += 1); | ||
| const isCurrentRun = () => initGeneration.current === generation; | ||
|
|
||
| try { | ||
| await gate.promise; | ||
| if (!isCurrentRun()) return; | ||
|
|
||
| effects.connectHub(); | ||
| if (!isCurrentRun()) return; | ||
|
|
||
| effects.markInitialized(); | ||
| if (!isCurrentRun()) return; | ||
|
|
||
| effects.startLocation(); | ||
| } finally { | ||
| if (isCurrentRun()) { | ||
| isInitializing.current = false; | ||
| } | ||
| } | ||
| }, [gate, effects]); | ||
|
|
||
| const signOut = React.useCallback(() => { | ||
| initGeneration.current += 1; | ||
| isInitializing.current = false; | ||
| }, []); | ||
|
|
||
| return { initialize, signOut, isInitializing }; | ||
| } | ||
|
|
||
| describe('app initialization session generation', () => { | ||
| const effects = { connectHub: jest.fn(), startLocation: jest.fn(), markInitialized: jest.fn() }; | ||
|
|
||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('abandons an in-flight run when the session ends mid-initialization', async () => { | ||
| const gate = deferred(); | ||
| const { result } = renderHook(() => useInitGuard(gate, effects)); | ||
|
|
||
| let pending: Promise<void> = Promise.resolve(); | ||
| act(() => { | ||
| pending = result.current.initialize(); | ||
| }); | ||
|
|
||
| // Sign-out lands while initialization is still awaiting its first step. | ||
| act(() => { | ||
| result.current.signOut(); | ||
| }); | ||
|
|
||
| await act(async () => { | ||
| gate.resolve(); | ||
| await pending; | ||
| }); | ||
|
|
||
| expect(effects.connectHub).not.toHaveBeenCalled(); | ||
| expect(effects.markInitialized).not.toHaveBeenCalled(); | ||
| expect(effects.startLocation).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('completes normally when the session survives', async () => { | ||
| const gate = deferred(); | ||
| const { result } = renderHook(() => useInitGuard(gate, effects)); | ||
|
|
||
| let pending: Promise<void> = Promise.resolve(); | ||
| act(() => { | ||
| pending = result.current.initialize(); | ||
| }); | ||
|
|
||
| await act(async () => { | ||
| gate.resolve(); | ||
| await pending; | ||
| }); | ||
|
|
||
| expect(effects.connectHub).toHaveBeenCalledTimes(1); | ||
| expect(effects.markInitialized).toHaveBeenCalledTimes(1); | ||
| expect(effects.startLocation).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('frees the in-progress guard so the next sign-in can initialize', async () => { | ||
| const first = deferred(); | ||
| const { result } = renderHook(() => useInitGuard(first, effects)); | ||
|
|
||
| let pending: Promise<void> = Promise.resolve(); | ||
| act(() => { | ||
| pending = result.current.initialize(); | ||
| }); | ||
| act(() => { | ||
| result.current.signOut(); | ||
| }); | ||
|
|
||
| // The new session starts before the retired run has settled. | ||
| let second: Promise<void> = Promise.resolve(); | ||
| act(() => { | ||
| second = result.current.initialize(); | ||
| }); | ||
|
|
||
| await act(async () => { | ||
| first.resolve(); | ||
| await Promise.all([pending, second]); | ||
|
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.
Kody rule violation: Use Promise.allSettled for batch operations with partial failures Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| }); | ||
|
|
||
| // Exactly one run reached the effects: the current one. | ||
| expect(effects.markInitialized).toHaveBeenCalledTimes(1); | ||
| expect(effects.startLocation).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
Comment on lines
+70
to
+139
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 | 🟡 Minor | ⚡ Quick win Unmount each hook after the test. Each As per coding guidelines, 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,6 +87,10 @@ export default function TabLayout() { | |
| // Refs to track initialization state | ||
| const hasInitialized = useRef(false); | ||
| const isInitializing = useRef(false); | ||
| // Bumped on every initialization start and whenever the session ends. An in-flight run | ||
| // compares its captured value after each await, so a run belonging to a session that is | ||
| // over can no longer connect hubs or mark the app initialized. | ||
| const initGeneration = useRef(0); | ||
| const hasHiddenSplash = useRef(false); | ||
| const parentRef = useRef(null); | ||
|
|
||
|
|
@@ -155,6 +159,8 @@ export default function TabLayout() { | |
| } | ||
|
|
||
| isInitializing.current = true; | ||
| const generation = (initGeneration.current += 1); | ||
| const isCurrentRun = () => initGeneration.current === generation; | ||
| logger.info({ | ||
| message: 'Starting app initialization', | ||
| context: { | ||
|
|
@@ -171,9 +177,13 @@ export default function TabLayout() { | |
| await securityStore.getState().getRights(); | ||
| await featureFlagsStore.getState().fetchFlags(); | ||
|
|
||
| if (!isCurrentRun()) return; | ||
|
|
||
| await useSignalRStore.getState().connectUpdateHub(); | ||
| await useSignalRStore.getState().connectGeolocationHub(); | ||
|
|
||
| if (!isCurrentRun()) return; | ||
|
|
||
| // Connect the realtime chat hub (best-effort; chat may be disabled per department) | ||
| if (featureFlagsStore.getState().isEnabled(FeatureFlagKeys.ChatSystem)) { | ||
| try { | ||
|
|
@@ -192,6 +202,8 @@ export default function TabLayout() { | |
| .syncFromServer() | ||
| .catch(() => {}); | ||
|
|
||
| if (!isCurrentRun()) return; | ||
|
|
||
| hasInitialized.current = true; | ||
|
|
||
| // Initialize Bluetooth and Audio services (native-only) | ||
|
|
@@ -208,12 +220,20 @@ export default function TabLayout() { | |
| message: 'Failed to initialize app', | ||
| context: { error }, | ||
| }); | ||
| // A run whose session already ended must not burn the retry budget or clobber | ||
| // state a newer run has since established. | ||
| if (!isCurrentRun()) return; | ||
|
|
||
| // Reset initialization state on error so it can be retried | ||
| hasInitialized.current = false; | ||
| setInitRetryCount((c) => c + 1); | ||
| } finally { | ||
| isInitializing.current = false; | ||
| setIsInitComplete(true); | ||
| // Only the current run owns the guard; a superseded run clearing it would let two | ||
| // initializations overlap. | ||
| if (isCurrentRun()) { | ||
| isInitializing.current = false; | ||
| setIsInitComplete(true); | ||
| } | ||
| } | ||
| }, [status]); | ||
|
|
||
|
|
@@ -261,7 +281,15 @@ export default function TabLayout() { | |
| // Handle app initialization - simplified logic | ||
| const MAX_INIT_RETRIES = 3; | ||
| useEffect(() => { | ||
| if (status !== 'signedIn' && initRetryCount > 0) { | ||
| if (status === 'signedIn') return; | ||
|
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. Raw string literal Kody rule violation: Use enums instead of magic strings Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. 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. Shared string literal Kody rule violation: Centralize string constants Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
|
|
||
| // Leaving the signed-in state retires any initialization still in flight, and frees | ||
| // the guard it no longer owns so the next sign-in is not skipped as "already | ||
| // initializing". | ||
| initGeneration.current += 1; | ||
| isInitializing.current = false; | ||
|
|
||
| if (initRetryCount > 0) { | ||
| setInitRetryCount(0); | ||
| } | ||
| }, [status, initRetryCount]); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,9 +8,7 @@ | |
| 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'; | ||
|
|
@@ -19,7 +17,6 @@ | |
| 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'; | ||
| import useAuthStore from '@/stores/auth/store'; | ||
|
|
@@ -37,8 +34,6 @@ | |
| 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'; | ||
|
|
||
|
|
@@ -72,7 +67,7 @@ | |
|
|
||
| const renderItem = useCallback( | ||
| ({ item }: { item: ChatMessageResultData }) => ( | ||
| <MessageBubble message={item} isOwn={!!item.SenderUserId && item.SenderUserId === currentUserId} showSender={false} currentUserId={currentUserId} onLongPress={setActionsMessage} onToggleReaction={() => undefined} /> | ||
|
Check warning on line 70 in src/app/(app)/chatbot.tsx
|
||
| ), | ||
| [currentUserId] | ||
| ); | ||
|
|
@@ -146,7 +141,7 @@ | |
| </HStack> | ||
| </KeyboardAvoidingView> | ||
|
|
||
| {/* Restricted actions for assistant messages: copy, edit own, pin (moderator), flag. */} | ||
| {/* Restricted actions for assistant messages: copy, pin (moderator), flag. */} | ||
| <MessageActionsSheet | ||
| message={actionsMessage} | ||
| isOpen={actionsMessage !== null} | ||
|
|
@@ -160,42 +155,12 @@ | |
| const ok = await copyToClipboard(m.Body ?? ''); | ||
| useToastStore.getState().showToast(ok ? 'success' : 'info', ok ? t('chat.copied') : t('chat.copy_unavailable')); | ||
| }} | ||
| onEdit={(m) => { | ||
| setEditMessage(m); | ||
| setEditText(m.Body ?? ''); | ||
| }} | ||
| onEdit={() => undefined} | ||
|
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 function in JSX prop creates a new function on every render, impacting performance. Move function definitions outside the render method or extract to a stable reference. 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. |
||
| 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> | ||
| ); | ||
| } | ||
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.
Missing JSDoc return documentation:
askIncidentAssistantomits a formal@returns {Promise<Type>}tag and rejection conditions. Rule [22] requires async functions to document the resolve value, rejection conditions, and await usage with@returns {Promise<...>}.Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.