Conversation
📝 WalkthroughWalkthroughThe change adds an offline incident assistant with ICS playbooks, local intent matching, server fallback, persisted conversations, command-board UI, localized text, and tests. It also updates SignalR event handling, chat metadata, message composition, session initialization, and Sentry debug configuration. ChangesIncident assistant
Chat transport and message handling
Application lifecycle and observability
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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
🧹 Nitpick comments (5)
src/api/chat/chatbot.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPlace type-only imports last.
Both
src/api/chat/chatbot.tsandsrc/stores/command/assistant-store.tsimport only types before value imports. Move the type-only imports to finalimport typedeclarations to follow the configured import order.🤖 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/chatbot.ts` at line 1, Move the type-only import in src/api/chat/chatbot.ts:1 to a final import type declaration after all value imports. Apply the same import-order fix in src/stores/command/assistant-store.ts:1-9 by placing its type-only imports last, without changing the imported symbols or runtime imports.Source: Coding guidelines
src/services/incident-assistant/__tests__/answerers.test.ts (1)
6-6: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for the remaining exported answerers.
The test imports do not include
answerTimers,answerNotes, oranswerBriefing. Add focused tests for empty data, populated data, and an expiredNextDueOnvalue. This validates the new offline response paths.As per coding guidelines, generate tests for new services and logic.
🤖 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/incident-assistant/__tests__/answerers.test.ts` at line 6, Add focused tests in the existing answerers test suite for the exported answerers answerTimers, answerNotes, and answerBriefing, including empty data, populated data, and expired NextDueOn cases. Import these symbols from answerers and verify each offline response path matches the established test patterns used by the other answerers.Source: Coding guidelines
src/services/incident-assistant/answerers.ts (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the required type-import and path-alias conventions.
Use
import typefor type-only imports. Place type imports after value imports. Replace relative imports for modules undersrcwith@/services/incident-assistant/...aliases.
src/services/incident-assistant/answerers.ts#L12-L12: change theTFunctionimport to a type-only import.src/services/incident-assistant/answerers.ts#L35-L36: splitIncidentPlaybookinto a type-only import and replace both relative paths with aliases.src/services/incident-assistant/__tests__/answerers.test.ts#L1-L6: changeTFunctionandIncidentAnswerContextto type-only imports and replace../answererswith its alias.src/services/incident-assistant/__tests__/role-vocabulary.test.ts#L3-L3: replace../role-vocabularywith its alias.As per coding guidelines, use
import typefor type-only imports and configured path aliases instead of relative 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/services/incident-assistant/answerers.ts` at line 12, Apply the import conventions across all listed sites: in src/services/incident-assistant/answerers.ts:12-12, make TFunction a type-only import; in src/services/incident-assistant/answerers.ts:35-36, split IncidentPlaybook into a type-only import and replace both relative imports with `@/services/incident-assistant/`... aliases; in src/services/incident-assistant/__tests__/answerers.test.ts:1-6, make TFunction and IncidentAnswerContext type-only imports and alias ../answerers; and in src/services/incident-assistant/__tests__/role-vocabulary.test.ts:3-3, replace ../role-vocabulary with its configured alias, keeping type imports after value imports.Source: Coding guidelines
src/stores/command/__tests__/assistant-store.test.ts (1)
154-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the test name with what the test asserts.
The name states "keeps a separate conversation per incident and clears only the one asked for". The body only exercises call id
'42'. It never creates a second conversation, so it cannot prove isolation. Either rename the test to describe clearing only, or add a second call id and assert thatclear('42')leaves it intact.🤖 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/command/__tests__/assistant-store.test.ts` around lines 154 - 163, Align the test named “keeps a separate conversation per incident and clears only the one asked for” with its assertions: either rename it to describe clearing a single conversation, or add a second incident conversation and assert that clear('42') preserves the other conversation.src/components/command/assistant-sheet.tsx (1)
99-108: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueExtract the suggestion chip into a memoized component.
Line 103 creates a new
onPressclosure for every chip on every render. The chip list is small, so the cost is low today.If you extract a
SuggestionChipcomponent that receivessuggestionandonSubmit, you remove the per-item closure and can wrap it inReact.memo().Consider also deriving the
testIDon line 104 from a stable slug rather than the raw question text, which contains spaces and punctuation.As per coding guidelines: "avoid anonymous functions in
renderItemand event handlers".🤖 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/command/assistant-sheet.tsx` around lines 99 - 108, Extract the mapped suggestion chip into a memoized SuggestionChip component that receives suggestion and onSubmit, and move the submit handling into that component without creating an inline per-item onPress closure in the parent mapping. Preserve the existing label and submission behavior, and derive the chip testID from a stable slug of the suggestion question instead of using raw text.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/api/chat/chatbot.ts`:
- Around line 36-44: Refactor askIncidentAssistant and
getIncidentAssistantSuggestions to use the approved createApiEndpoint or
createCachedApiEndpoint factories instead of calling api.post/api.get directly.
Preserve their typed response generics, request payloads, query parameters,
return values, and optional AbortSignal propagation.
In `@src/components/chat/message-bubble.tsx`:
- Around line 126-128: Update the avatar rendering condition in the message
bubble so an empty Avatar is not rendered when SenderUserId is absent. For
non-own messages with showSender enabled, render the existing w-8 spacer when
SenderUserId is missing, while preserving the AvatarImage path when it is
present.
In `@src/components/command/assistant-sheet.tsx`:
- Around line 126-132: Update IncidentAssistantSheet at
src/components/command/assistant-sheet.tsx#L126-L132, `#L73`, and `#L158-L166`: add
a dark-mode error bubble background variant, replace both Sparkles icon
hardcoded hex colors with semantic token classes using the existing Icon
pattern, and derive the Send icon color from its disabled condition so it
remains readable over the disabled background.
- Around line 126-132: Update the error-state class in the message bubble
rendered by the entry mapping to include an appropriate dark-mode background
variant alongside bg-error-50, preserving the existing normal-state classes and
text styling.
- Around line 47-50: Import and use useCommandStore in the assistant sheet
component to subscribe to the relevant board slice, then include that subscribed
board value in the dependency lists for the suggestions and typeName useMemo
calls. Preserve the existing isOpen and callId behavior while ensuring board
updates trigger recomputation of both values.
In `@src/services/incident-assistant/__tests__/answerers.test.ts`:
- Line 20: Update the time-dependent tests around minutesAgo, answerStatus,
answerTimeline, and answerResources to use Jest fake timers with a fixed system
time before each test. Restore real timers in afterEach, ensuring all
elapsed-time assertions use the deterministic clock.
In `@src/services/incident-assistant/answerers.ts`:
- Around line 518-535: Update the timer row selection in the timers mapping to
treat a valid NextDueOn timestamp at or before the current time as due, even
when timer.Status is Running; retain the existing status-based handling for
timers without a valid due timestamp. Add a fixed-clock test covering an expired
cached Running timer and assert that it uses the due-row translation.
In `@src/services/incident-assistant/ics-playbooks.ts`:
- Line 473: Update the playbook selection logic around inferPlaybook so it ranks
matching keyword candidates by keyword length and returns the most specific
match rather than the first PLAYBOOKS entry; preserve exact displayName matching
and null when nothing matches. Add regression tests covering overlapping
keywords such as “tanker rollover” and “flood rescue”.
- Around line 79-455: Update the playbook rendering flow around answerChecklist
to source displayName and checklist text from vetted react-i18next locale
resources instead of the English literals in GENERAL and PLAYBOOKS. Wrap every
rendered user-visible value with t(), using interpolation for dynamic values,
and ensure all playbook types and checklist entries have corresponding
translation keys; if doctrine must remain English, add an approved exception and
explicitly identify the source language in the UI.
In `@src/services/incident-assistant/intent-matcher.ts`:
- Around line 176-212: Update fuzzyMatch so the par check matches par only as a
standalone word while retaining accountab as a substring prefix; also apply a
word-boundary check to order so words like border and recorder do not trigger
needs, while preserving the remaining fuzzy intent checks.
In `@src/stores/chat/__tests__/hub-invoke-args.test.ts`:
- Around line 156-172: Update the “chat typing events” suite to use
jest.useFakeTimers(), reset the chat store state and restore real timers in
afterEach, and retain the existing beforeEach setup for typingByChannel. Ensure
handleTyping expiry timers are controlled and cleaned up between tests.
In `@src/stores/command/__tests__/assistant-store.test.ts`:
- Around line 142-152: Freeze the clock for the offline assistant test around
useIncidentAssistantStore.getState().ask, ensuring the module-level
minutesAgo(15) value and the store’s elapsed-time calculation use a consistent
timestamp. Use Jest fake timers for the test and restore real timers afterward,
preserving the existing assertions.
In `@src/stores/command/assistant-store.ts`:
- Around line 33-34: Replace the single askingCallId state with a per-call
pending map or set, updating the assistant store’s request-start and completion
logic to add and remove individual call IDs. Ensure completion at the existing
request-finalization path clears only the completed call, preserving other
calls’ pending state and UI submission blocking.
In `@src/translations/ar.json`:
- Around line 954-1093: Translate every English value in the incident_assistant
namespace into Arabic in src/translations/ar.json lines 954-1093, German in
src/translations/de.json lines 954-1093, and Spanish in src/translations/es.json
lines 954-1093. Preserve all keys and interpolation tokens exactly as defined in
src/translations/en.json.
In `@src/translations/fr.json`:
- Around line 954-1092: Translate every value in the incident_assistant
namespace into the target locale while preserving all keys and interpolation
tokens. Apply the translations in src/translations/fr.json lines 954-1092,
src/translations/it.json lines 954-1092, src/translations/pl.json lines
954-1092, src/translations/sv.json lines 954-1092, and src/translations/uk.json
lines 954-1092; keep the namespace structure and key sets identical across all
locale files.
---
Nitpick comments:
In `@src/api/chat/chatbot.ts`:
- Line 1: Move the type-only import in src/api/chat/chatbot.ts:1 to a final
import type declaration after all value imports. Apply the same import-order fix
in src/stores/command/assistant-store.ts:1-9 by placing its type-only imports
last, without changing the imported symbols or runtime imports.
In `@src/components/command/assistant-sheet.tsx`:
- Around line 99-108: Extract the mapped suggestion chip into a memoized
SuggestionChip component that receives suggestion and onSubmit, and move the
submit handling into that component without creating an inline per-item onPress
closure in the parent mapping. Preserve the existing label and submission
behavior, and derive the chip testID from a stable slug of the suggestion
question instead of using raw text.
In `@src/services/incident-assistant/__tests__/answerers.test.ts`:
- Line 6: Add focused tests in the existing answerers test suite for the
exported answerers answerTimers, answerNotes, and answerBriefing, including
empty data, populated data, and expired NextDueOn cases. Import these symbols
from answerers and verify each offline response path matches the established
test patterns used by the other answerers.
In `@src/services/incident-assistant/answerers.ts`:
- Line 12: Apply the import conventions across all listed sites: in
src/services/incident-assistant/answerers.ts:12-12, make TFunction a type-only
import; in src/services/incident-assistant/answerers.ts:35-36, split
IncidentPlaybook into a type-only import and replace both relative imports with
`@/services/incident-assistant/`... aliases; in
src/services/incident-assistant/__tests__/answerers.test.ts:1-6, make TFunction
and IncidentAnswerContext type-only imports and alias ../answerers; and in
src/services/incident-assistant/__tests__/role-vocabulary.test.ts:3-3, replace
../role-vocabulary with its configured alias, keeping type imports after value
imports.
In `@src/stores/command/__tests__/assistant-store.test.ts`:
- Around line 154-163: Align the test named “keeps a separate conversation per
incident and clears only the one asked for” with its assertions: either rename
it to describe clearing a single conversation, or add a second incident
conversation and assert that clear('42') preserves the other conversation.
🪄 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: fad6e575-1857-479b-9fc8-3a3dff4b3ee9
📒 Files selected for processing (35)
src/api/chat/chatbot.tssrc/app/(app)/chatbot.tsxsrc/app/(app)/command.tsxsrc/app/_layout.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/new-conversation-sheet.tsxsrc/components/command/assistant-sheet.tsxsrc/models/v4/chat/chatbotModels.tssrc/services/incident-assistant/__tests__/answerers.test.tssrc/services/incident-assistant/__tests__/intent-matcher.test.tssrc/services/incident-assistant/__tests__/role-vocabulary.test.tssrc/services/incident-assistant/answerers.tssrc/services/incident-assistant/ics-playbooks.tssrc/services/incident-assistant/index.tssrc/services/incident-assistant/intent-matcher.tssrc/services/incident-assistant/role-vocabulary.tssrc/services/signalr.service.tssrc/stores/chat/__tests__/hub-invoke-args.test.tssrc/stores/chat/store.tssrc/stores/command/__tests__/assistant-store.test.tssrc/stores/command/assistant-store.tssrc/stores/signalr/signalr-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
| export const askIncidentAssistant = async (callId: number, question: string, signal?: AbortSignal) => { | ||
| const response = await api.post<IncidentAssistantAnswerResponse>(`${CHATBOT}/AskIncident`, { Question: question, CallId: callId }, { signal }); | ||
| 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; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the approved API endpoint factories.
These methods call the API client directly. Define them through createApiEndpoint or createCachedApiEndpoint so they follow the required API module boundary. Preserve the typed response and AbortSignal behavior.
As per coding guidelines, “Implement API modules with createApiEndpoint or createCachedApiEndpoint, use typed generics on HTTP methods, and invalidate relevant caches after mutations.”
🤖 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/chatbot.ts` around lines 36 - 44, Refactor askIncidentAssistant
and getIncidentAssistantSuggestions to use the approved createApiEndpoint or
createCachedApiEndpoint factories instead of calling api.post/api.get directly.
Preserve their typed response generics, request payloads, query parameters,
return values, and optional AbortSignal propagation.
Source: Coding guidelines
| {/* No initials fallback: the avatar endpoint always answers with a silhouette | ||
| placeholder rather than a 404, so initials would never be visible anyway. */} | ||
| {!isOwn && showSender ? <Avatar size="sm">{message.SenderUserId ? <AvatarImage source={{ uri: getPersonAvatarUrl(message.SenderUserId) ?? '' }} /> : null}</Avatar> : !isOwn ? <Box className="w-8" /> : null} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Render a spacer when SenderUserId is absent.
Line 128 renders an empty Avatar when showSender is true and SenderUserId is absent. Messages with only SenderUnitId or SenderDisplayName show a blank avatar circle.
Render the existing width spacer for this case, or retain the initials fallback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/chat/message-bubble.tsx` around lines 126 - 128, Update the
avatar rendering condition in the message bubble so an empty Avatar is not
rendered when SenderUserId is absent. For non-own messages with showSender
enabled, render the existing w-8 spacer when SenderUserId is missing, while
preserving the AvatarImage path when it is present.
| // The playbook (and therefore the chips and type badge) is derived from the board and the call, so | ||
| // it recomputes whenever the sheet re-renders with new board data. | ||
| const suggestions = useMemo(() => (isOpen ? useIncidentAssistantStore.getState().suggestions(callId) : []), [isOpen, callId]); | ||
| const typeName = useMemo(() => (isOpen ? incidentTypeName(buildAnswerContext(callId)) : ''), [isOpen, callId]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The suggestions and the type badge do not refresh with new board data.
The comment states that these values recompute whenever the sheet re-renders with new board data. They do not. useMemo recomputes only when a dependency changes, and the dependency list is [isOpen, callId]. Both values also read the store through getState(), which creates no subscription, so a board update does not re-render this component at all.
Result: the sheet opens, the commander refreshes the board, the incident type changes, and the badge on line 77 keeps showing the old incident family while the chips keep the old prompts. The answers themselves stay correct because ask reads the store at call time.
Subscribe to the board slice and add it to the dependency lists.
🐛 Proposed fix
+ const board = useCommandStore((state) => state.boards[callId]?.board);
+
const messages = useIncidentAssistantStore((state) => state.messagesByCallId[callId]);
const askingCallId = useIncidentAssistantStore((state) => state.askingCallId);
@@
- // The playbook (and therefore the chips and type badge) is derived from the board and the call, so
- // it recomputes whenever the sheet re-renders with new board data.
- const suggestions = useMemo(() => (isOpen ? useIncidentAssistantStore.getState().suggestions(callId) : []), [isOpen, callId]);
- const typeName = useMemo(() => (isOpen ? incidentTypeName(buildAnswerContext(callId)) : ''), [isOpen, callId]);
+ // The playbook (and therefore the chips and type badge) is derived from the board and the call.
+ // Subscribing to the board slice keeps both current across board refreshes.
+ const suggestions = useMemo(() => (isOpen ? useIncidentAssistantStore.getState().suggestions(callId) : []), [isOpen, callId, board]);
+ const typeName = useMemo(() => (isOpen ? incidentTypeName(buildAnswerContext(callId)) : ''), [isOpen, callId, board]);Add the matching import:
import { useCommandStore } from '`@/stores/command/store`';📝 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.
| // The playbook (and therefore the chips and type badge) is derived from the board and the call, so | |
| // it recomputes whenever the sheet re-renders with new board data. | |
| const suggestions = useMemo(() => (isOpen ? useIncidentAssistantStore.getState().suggestions(callId) : []), [isOpen, callId]); | |
| const typeName = useMemo(() => (isOpen ? incidentTypeName(buildAnswerContext(callId)) : ''), [isOpen, callId]); | |
| const board = useCommandStore((state) => state.boards[callId]?.board); | |
| // The playbook (and therefore the chips and type badge) is derived from the board and the call. | |
| // Subscribing to the board slice keeps both current across board refreshes. | |
| const suggestions = useMemo(() => (isOpen ? useIncidentAssistantStore.getState().suggestions(callId) : []), [isOpen, callId, board]); | |
| const typeName = useMemo(() => (isOpen ? incidentTypeName(buildAnswerContext(callId)) : ''), [isOpen, callId, board]); |
🤖 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/command/assistant-sheet.tsx` around lines 47 - 50, Import and
use useCommandStore in the assistant sheet component to subscribe to the
relevant board slice, then include that subscribed board value in the dependency
lists for the suggestions and typeName useMemo calls. Preserve the existing
isOpen and callId behavior while ensuring board updates trigger recomputation of
both values.
| <VStack | ||
| key={entry.id} | ||
| space="xs" | ||
| className={`self-start rounded-2xl rounded-bl-sm px-3 py-2 ${entry.isError ? 'bg-error-50' : 'bg-gray-100 dark:bg-gray-800'}`} | ||
| testID={`incident-assistant-message-${entry.id}`} | ||
| > | ||
| <Text className="text-sm text-gray-900 dark:text-gray-100">{entry.text}</Text> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Color choices in IncidentAssistantSheet do not pair light and dark modes. Three sites set a color for one scheme only, either through a Tailwind class with no dark: variant or through a hardcoded hex passed to a lucide icon. A single pass that uses semantic tokens and scheme-aware icon colors fixes all three.
src/components/command/assistant-sheet.tsx#L126-L132: add a dark variant for the error bubble background, for examplebg-error-50 dark:bg-error-900, sodark:text-gray-100on line 132 stays readable.src/components/command/assistant-sheet.tsx#L73-L73: replace the hardcoded#ffffffon the headerSparklesicon, and the#9ca3afon the empty-stateSparklesicon on line 115, with semantic token colors; the file already usesIconwithclassNameon lines 135 and 171.src/components/command/assistant-sheet.tsx#L158-L166: derive theSendicon color from the same disabled condition that drives the background class, instead of always passing#ffffffoverbg-gray-300.
As per coding guidelines: "Support light and dark modes through the system color scheme and use semantic Tailwind color tokens instead of hardcoded hex colors."
📍 Affects 1 file
src/components/command/assistant-sheet.tsx#L126-L132(this comment)src/components/command/assistant-sheet.tsx#L73-L73src/components/command/assistant-sheet.tsx#L158-L166
🤖 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/command/assistant-sheet.tsx` around lines 126 - 132, Update
IncidentAssistantSheet at src/components/command/assistant-sheet.tsx#L126-L132,
`#L73`, and `#L158-L166`: add a dark-mode error bubble background variant, replace
both Sparkles icon hardcoded hex colors with semantic token classes using the
existing Icon pattern, and derive the Send icon color from its disabled
condition so it remains readable over the disabled background.
Source: Coding guidelines
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
The error bubble is unreadable in dark mode.
Line 129 applies bg-error-50 with no dark variant, while the normal bubble pairs bg-gray-100 with dark:bg-gray-800. Line 132 applies dark:text-gray-100. In dark mode the error bubble therefore renders near-white text on the light error-50 background.
Add a dark background variant for the error state.
🐛 Proposed fix
- className={`self-start rounded-2xl rounded-bl-sm px-3 py-2 ${entry.isError ? 'bg-error-50' : 'bg-gray-100 dark:bg-gray-800'}`}
+ className={`self-start rounded-2xl rounded-bl-sm px-3 py-2 ${entry.isError ? 'bg-error-50 dark:bg-error-900' : 'bg-gray-100 dark:bg-gray-800'}`}As per coding guidelines: "Support light and dark modes ... and sufficient contrast in light and dark modes."
📝 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.
| <VStack | |
| key={entry.id} | |
| space="xs" | |
| className={`self-start rounded-2xl rounded-bl-sm px-3 py-2 ${entry.isError ? 'bg-error-50' : 'bg-gray-100 dark:bg-gray-800'}`} | |
| testID={`incident-assistant-message-${entry.id}`} | |
| > | |
| <Text className="text-sm text-gray-900 dark:text-gray-100">{entry.text}</Text> | |
| <VStack | |
| key={entry.id} | |
| space="xs" | |
| className={`self-start rounded-2xl rounded-bl-sm px-3 py-2 ${entry.isError ? 'bg-error-50 dark:bg-error-900' : 'bg-gray-100 dark:bg-gray-800'}`} | |
| testID={`incident-assistant-message-${entry.id}`} | |
| > | |
| <Text className="text-sm text-gray-900 dark:text-gray-100">{entry.text}</Text> |
🤖 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/command/assistant-sheet.tsx` around lines 126 - 132, Update
the error-state class in the message bubble rendered by the entry mapping to
include an appropriate dark-mode background variant alongside bg-error-50,
preserving the existing normal-state classes and text styling.
Source: Coding guidelines
| return value.replace(/{{(\w+)}}/g, (_match, name: string) => String(options?.[name] ?? '')); | ||
| }) as unknown as TFunction; | ||
|
|
||
| const minutesAgo = (minutes: number) => new Date(Date.now() - minutes * 60_000).toISOString(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the test file and relevant implementation exports/usages.
printf 'Tracked files matching answerers:\n'
git ls-files | rg 'src/services/incident-assistant/.*(answerers\.ts|answerers\.test\.ts)$' || true
printf '\nAnswerer test outline/size:\n'
wc -l src/services/incident-assistant/__tests__/answerers.test.ts
ast-grep outline src/services/incident-assistant/__tests__/answerers.test.ts || true
printf '\nRelevant test lines:\n'
sed -n '1,60p' src/services/incident-assistant/__tests__/answerers.test.ts
printf '\n...\n'
sed -n '120,330p' src/services/incident-assistant/__tests__/answerers.test.ts
printf '\nImplementation exports/usages of Date.now and time windows:\n'
rg -n "Date\\.now|minutesAgo|jest\\.useFakeTimers|setSystemTime|answerTimers|answerNotes|answerBriefing" src/services/incident-assistant || trueRepository: Resgrid/IC
Length of output: 13826
Use a fixed Jest clock for time-dependent tests.
This test uses minutesAgo(Date.now()) and the answerers compute elapsed time with Date.now(), including answerStatus, answerTimeline, and answerResources. Set jest.useFakeTimers() and jest.setSystemTime(...) before each test, and restore real timers with jest.useRealTimers() in afterEach.
🤖 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/incident-assistant/__tests__/answerers.test.ts` at line 20,
Update the time-dependent tests around minutesAgo, answerStatus, answerTimeline,
and answerResources to use Jest fake timers with a fixed system time before each
test. Restore real timers in afterEach, ensuring all elapsed-time assertions use
the deterministic clock.
Source: Coding guidelines
| describe('chat typing events', () => { | ||
| beforeEach(() => { | ||
| useChatStore.setState({ typingByChannel: {} }); | ||
| }); | ||
|
|
||
| it('reads the hub payload ChannelId field', () => { | ||
| useChatStore.getState().handleTyping({ ChannelId: 'channel-1', UserId: 'user-2', DisplayName: 'Other', IsTyping: true }); | ||
|
|
||
| expect(useChatStore.getState().typingByChannel['channel-1']?.[0]?.displayName).toBe('Other'); | ||
| }); | ||
|
|
||
| it('reads a camelCase hub payload', () => { | ||
| useChatStore.getState().handleTyping({ channelId: 'channel-1', userId: 'user-2', displayName: 'Other', isTyping: true }); | ||
|
|
||
| expect(useChatStore.getState().typingByChannel['channel-1']?.[0]?.userId).toBe('user-2'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Control and clean up typing timers.
handleTyping(..., true) schedules a typing-expiry timer. These tests use real timers and do not clear the store timer state. This can leave open handles and cause test interference.
Use jest.useFakeTimers() for this suite. Reset the chat store and restore real timers in afterEach.
As per coding guidelines, “Use fake and real Jest timers for time-dependent tests.”
🤖 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/__tests__/hub-invoke-args.test.ts` around lines 156 - 172,
Update the “chat typing events” suite to use jest.useFakeTimers(), reset the
chat store state and restore real timers in afterEach, and retain the existing
beforeEach setup for typingByChannel. Ensure handleTyping expiry timers are
controlled and cleaned up between tests.
Source: Coding guidelines
| it('still answers board questions offline', async () => { | ||
| mockOnline = false; | ||
|
|
||
| await act(async () => { | ||
| await useIncidentAssistantStore.getState().ask('42', 'incident status', t); | ||
| }); | ||
|
|
||
| expect(mockAskIncidentAssistant).not.toHaveBeenCalled(); | ||
| expect(messages()[1]).toMatchObject({ source: 'device' }); | ||
| expect(messages()[1].text).toContain('Command running 15m'); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Freeze time for the elapsed-duration assertion.
minutesAgo(15) runs once when the module loads (line 35). The store computes the elapsed time when the test runs. If the suite crosses a minute boundary between module load and this test, the answer renders "Command running 16m" and the assertion on line 151 fails intermittently.
Use Jest fake timers to pin the clock for this test.
💚 Proposed fix
it('still answers board questions offline', async () => {
mockOnline = false;
+ jest.useFakeTimers().setSystemTime(new Date());
await act(async () => {
await useIncidentAssistantStore.getState().ask('42', 'incident status', t);
});
expect(mockAskIncidentAssistant).not.toHaveBeenCalled();
expect(messages()[1]).toMatchObject({ source: 'device' });
expect(messages()[1].text).toContain('Command running 15m');
+ jest.useRealTimers();
});A cleaner alternative is to compute EstablishedOn inside beforeEach instead of at module scope.
As per coding guidelines: "Use fake and real Jest timers for time-dependent tests".
🤖 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/command/__tests__/assistant-store.test.ts` around lines 142 - 152,
Freeze the clock for the offline assistant test around
useIncidentAssistantStore.getState().ask, ensuring the module-level
minutesAgo(15) value and the store’s elapsed-time calculation use a consistent
timestamp. Use Jest fake timers for the test and restore real timers afterward,
preserving the existing assertions.
Source: Coding guidelines
| /** Call id currently awaiting an answer, or null. */ | ||
| askingCallId: string | null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Track pending requests per call.
A request for call A can start, then a request for call B can replace askingCallId. When call A finishes, Line 156 clears the pending state while call B is still pending. The UI then enables another submission for call B.
Replace the single value with a per-call pending map or set. Clear only the completed call ID.
Also applies to: 113-113, 156-156
🤖 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/command/assistant-store.ts` around lines 33 - 34, Replace the
single askingCallId state with a per-call pending map or set, updating the
assistant store’s request-start and completion logic to add and remove
individual call IDs. Ensure completion at the existing request-finalization path
clears only the completed call, preserving other calls’ pending state and UI
submission blocking.
| "incident_assistant": { | ||
| "briefing_accountability": "ACCOUNTABILITY", | ||
| "briefing_action_plan": "Action plan: {{text}}", | ||
| "briefing_address": "Location: {{address}}", | ||
| "briefing_command": "COMMAND", | ||
| "briefing_commander": "Incident Commander: {{name}}", | ||
| "briefing_established": "Command established: {{time}} (running {{duration}})", | ||
| "briefing_header": "TRANSFER OF COMMAND BRIEFING — {{incident}}", | ||
| "briefing_icp": "Command post: {{location}}", | ||
| "briefing_important": "Important information: {{text}}", | ||
| "briefing_lane_row": "{{lane}} (lead: {{lead}}) — {{count}} resources: {{names}}", | ||
| "briefing_needs": "OUTSTANDING NEEDS", | ||
| "briefing_no_lanes": "- No lanes established", | ||
| "briefing_no_needs": "- None", | ||
| "briefing_no_objectives": "- No objectives recorded", | ||
| "briefing_no_par": "No personnel accountability is being tracked.", | ||
| "briefing_objectives": "OBJECTIVES", | ||
| "briefing_organization": "ORGANIZATION AND RESOURCES", | ||
| "briefing_rehab": "Rehab: {{location}}", | ||
| "briefing_situation": "SITUATION", | ||
| "briefing_staging": "Staging: {{location}}", | ||
| "briefing_type": "Incident type: {{type}}", | ||
| "check_action_plan": "Action plan or objectives recorded", | ||
| "check_command": "Command established with a named IC", | ||
| "check_icp": "Command post location set", | ||
| "check_par": "Accountability / check-in running", | ||
| "check_safety": "Safety Officer assigned", | ||
| "check_staging": "Staging designated", | ||
| "checklist_confirm": "Standard {{type}} items to confirm:", | ||
| "checklist_disclaimer": "This is general ICS guidance, not your department's policy — your SOGs and your judgement come first.", | ||
| "checklist_done": "Already done on the board: {{items}}.", | ||
| "checklist_header": "{{type}} checklist for {{incident}}.", | ||
| "checklist_outstanding": "Not showing on the board yet:", | ||
| "clear": "Clear conversation", | ||
| "command_post": "ICP: {{location}}.", | ||
| "commander": "IC: {{name}}.", | ||
| "elapsed": "Command running {{duration}}.", | ||
| "empty": "Ask about this incident — accountability, resources, objectives, needs, positions, or what you might be missing.", | ||
| "error": "Something went wrong answering that. Try again.", | ||
| "estimated_end": "Estimated end: {{time}}.", | ||
| "external_resources": "- External / mutual aid resources tracked: {{count}}", | ||
| "important": "Important: {{text}}", | ||
| "lane_empty": "Nothing is assigned to this lane.", | ||
| "lane_header": "{{lane}} ({{type}}): {{count}} resources.", | ||
| "lane_lead": "Lead: {{name}}.", | ||
| "lane_line": "- {{lane}}: {{count}} — {{names}}", | ||
| "lane_not_found": "I don't see a lane called \"{{lane}}\". Lanes on the board: {{lanes}}", | ||
| "lane_objective": "Primary objective: {{name}} ({{progress}}%).", | ||
| "missing_benchmarks": "Common {{type}} benchmarks not on the board yet: {{benchmarks}}", | ||
| "need_row": "- {{name}} [{{category}}] {{quantity}} — open {{age}}", | ||
| "needs_all_met": "Everything ordered has been filled.", | ||
| "needs_header": "{{incident}}: {{outstanding}} needs outstanding ({{met}} met, {{cancelled}} cancelled).", | ||
| "no_answer": "I couldn't answer that one.", | ||
| "no_board": "No command board is loaded for this incident yet.", | ||
| "no_lanes": "none yet", | ||
| "no_lanes_yet": "No lanes have been created on {{incident}} yet, so there's no span of control to check.", | ||
| "no_lead": "no lead", | ||
| "no_needs": "No needs have been recorded on {{incident}}.", | ||
| "no_notes": "No status notes have been recorded on {{incident}}.", | ||
| "no_objectives": "No tactical objectives have been set on {{incident}} yet.", | ||
| "no_resources": "Nothing is assigned to {{incident}} yet.", | ||
| "no_rit": "I don't see a RIT/RIC lane on {{incident}}. On a working incident that's worth assigning before crews go interior.", | ||
| "no_timers": "No timers are running on {{incident}}.", | ||
| "no_unassigned": "Everything tracked on {{incident}} is placed in a lane.", | ||
| "note_row": "- {{time}}: {{body}} ({{who}})", | ||
| "notes_header": "{{incident}} — {{count}} status note(s):", | ||
| "objective_complete": "complete", | ||
| "objective_in_progress": "in progress", | ||
| "objective_overdue": " [past target]", | ||
| "objective_pending": "pending", | ||
| "objective_row": "{{name}} — {{status}} ({{progress}}%){{overdue}}", | ||
| "objective_summary": "Objectives: {{complete}} of {{total}} complete.", | ||
| "objectives_all_complete": "Every objective on the board is complete.", | ||
| "objectives_header": "{{incident}}: {{complete}} of {{total}} objectives complete. Still open:", | ||
| "offline_badge": "Offline", | ||
| "offline_cannot_answer": "I can't answer that without a connection. Offline I can still give you PAR, resources, objectives, needs, ICS positions, timers, the incident log, a briefing, and the checklist for this incident type.", | ||
| "offline_hint": "No connection — answers come from the board cached on this device.", | ||
| "open_needs": "{{count}} needs still open.", | ||
| "par_all_good": "Everyone is accounted for.", | ||
| "par_counts": "{{green}} green, {{warning}} approaching, {{critical}} overdue.", | ||
| "par_critical_header": "Overdue — not accounted for:", | ||
| "par_due_row": "- {{name}}: due in {{minutes}} min", | ||
| "par_header": "PAR for {{incident}}: {{count}} personnel tracked.", | ||
| "par_none": "No personnel accountability is being tracked on {{incident}} — nobody has checked in and no check-in timer is running.", | ||
| "par_overdue_row": "- {{name}}: {{minutes}} min overdue", | ||
| "par_summary": "PAR: {{total}} tracked, {{critical}} overdue, {{warning}} approaching.", | ||
| "par_warning_header": "Approaching check-in:", | ||
| "placeholder": "Ask about this incident", | ||
| "resource_counts": "{{units}} units and {{personnel}} personnel across {{lanes}} lanes ({{unassigned}} unassigned).", | ||
| "resources_header": "{{incident}}: {{units}} units and {{personnel}} personnel working.", | ||
| "rit_found": "{{lane}} is standing by with {{count}} resource(s).", | ||
| "role_filled": "{{role}}: {{name}}", | ||
| "role_row": "{{role}}: {{name}}", | ||
| "role_unfilled": "No {{role}} is assigned on {{incident}}.", | ||
| "role_unknown": "I don't recognize the position \"{{role}}\". Ask for ICS roles to see everything assigned.", | ||
| "roles_header": "{{incident}}: {{count}} ICS position(s) assigned.", | ||
| "roles_unfilled": "Unfilled positions a {{type}} usually needs: {{roles}}", | ||
| "send": "Send", | ||
| "source_device": "Answered on this device", | ||
| "source_server": "Answered by Resgrid", | ||
| "span_all_good": "Span of control looks reasonable — every lane is within its limits and no lane is over {{ceiling}} resources.", | ||
| "span_header": "{{incident}}: {{lanes}} lanes, {{resources}} resources assigned.", | ||
| "span_no_lead": "- No lead assigned: {{lanes}}", | ||
| "span_over_row": "- {{lane}} is carrying {{count}} resources (limit {{limit}}) — consider splitting it or adding a supervisor.", | ||
| "span_under_row": "- {{lane}} has {{count}} resources against a minimum of {{minimum}}.", | ||
| "staging": "Staging: {{location}}.", | ||
| "suggestions": { | ||
| "briefing": "Transfer of command", | ||
| "missing": "What am I missing?", | ||
| "on_scene": "What's on scene?", | ||
| "open_needs": "Open needs", | ||
| "open_objectives": "Open objectives", | ||
| "par": "PAR", | ||
| "recent": "Last 30 minutes", | ||
| "rit": "Do I have a RIT?", | ||
| "safety": "Safety Officer", | ||
| "span": "Span of control", | ||
| "status": "Incident status", | ||
| "timers": "Timers", | ||
| "unassigned": "Unassigned", | ||
| "unfilled_roles": "Unfilled ICS positions", | ||
| "wind": "Wind and weather" | ||
| }, | ||
| "thinking": "Checking the board…", | ||
| "this_incident": "this incident", | ||
| "time_in_lane": "({{duration}} in lane)", | ||
| "timeline_empty": "Nothing has been logged on {{incident}} yet.", | ||
| "timeline_empty_window": "Nothing has been logged in the last {{minutes}} minutes on {{incident}}.", | ||
| "timeline_header": "{{incident}} — last {{count}} log entries:", | ||
| "timeline_row": "- {{time}}: {{description}}{{who}}", | ||
| "timeline_window_header": "{{incident}} — last {{minutes}} minutes ({{count}} entries):", | ||
| "timer_due_row": "- {{name}}: DUE NOW", | ||
| "timer_no_due_row": "- {{name}}: running", | ||
| "timer_running_row": "- {{name}}: due in {{remaining}}", | ||
| "timers_header": "{{incident}}: {{count}} timer(s).", | ||
| "title": "Incident Assistant", | ||
| "unassigned_header": "Unassigned on {{incident}} ({{count}}):", | ||
| "unassigned_line": "- Unassigned pool: {{count}}", | ||
| "unknown": "unknown" | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Translate the non-English assistant resources.
The Arabic, German, and Spanish incident_assistant namespaces contain English strings. Users in these locales will see English titles, controls, status messages, and generated assistant text.
src/translations/ar.json#L954-L1093: replace English values with Arabic translations.src/translations/de.json#L954-L1093: replace English values with German translations.src/translations/es.json#L954-L1093: replace English values with Spanish translations.
Keep keys and interpolation tokens aligned with src/translations/en.json.
📍 Affects 3 files
src/translations/ar.json#L954-L1093(this comment)src/translations/de.json#L954-L1093src/translations/es.json#L954-L1093
🤖 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/ar.json` around lines 954 - 1093, Translate every English
value in the incident_assistant namespace into Arabic in
src/translations/ar.json lines 954-1093, German in src/translations/de.json
lines 954-1093, and Spanish in src/translations/es.json lines 954-1093. Preserve
all keys and interpolation tokens exactly as defined in
src/translations/en.json.
| "incident_assistant": { | ||
| "briefing_accountability": "ACCOUNTABILITY", | ||
| "briefing_action_plan": "Action plan: {{text}}", | ||
| "briefing_address": "Location: {{address}}", | ||
| "briefing_command": "COMMAND", | ||
| "briefing_commander": "Incident Commander: {{name}}", | ||
| "briefing_established": "Command established: {{time}} (running {{duration}})", | ||
| "briefing_header": "TRANSFER OF COMMAND BRIEFING — {{incident}}", | ||
| "briefing_icp": "Command post: {{location}}", | ||
| "briefing_important": "Important information: {{text}}", | ||
| "briefing_lane_row": "{{lane}} (lead: {{lead}}) — {{count}} resources: {{names}}", | ||
| "briefing_needs": "OUTSTANDING NEEDS", | ||
| "briefing_no_lanes": "- No lanes established", | ||
| "briefing_no_needs": "- None", | ||
| "briefing_no_objectives": "- No objectives recorded", | ||
| "briefing_no_par": "No personnel accountability is being tracked.", | ||
| "briefing_objectives": "OBJECTIVES", | ||
| "briefing_organization": "ORGANIZATION AND RESOURCES", | ||
| "briefing_rehab": "Rehab: {{location}}", | ||
| "briefing_situation": "SITUATION", | ||
| "briefing_staging": "Staging: {{location}}", | ||
| "briefing_type": "Incident type: {{type}}", | ||
| "check_action_plan": "Action plan or objectives recorded", | ||
| "check_command": "Command established with a named IC", | ||
| "check_icp": "Command post location set", | ||
| "check_par": "Accountability / check-in running", | ||
| "check_safety": "Safety Officer assigned", | ||
| "check_staging": "Staging designated", | ||
| "checklist_confirm": "Standard {{type}} items to confirm:", | ||
| "checklist_disclaimer": "This is general ICS guidance, not your department's policy — your SOGs and your judgement come first.", | ||
| "checklist_done": "Already done on the board: {{items}}.", | ||
| "checklist_header": "{{type}} checklist for {{incident}}.", | ||
| "checklist_outstanding": "Not showing on the board yet:", | ||
| "clear": "Clear conversation", | ||
| "command_post": "ICP: {{location}}.", | ||
| "commander": "IC: {{name}}.", | ||
| "elapsed": "Command running {{duration}}.", | ||
| "empty": "Ask about this incident — accountability, resources, objectives, needs, positions, or what you might be missing.", | ||
| "error": "Something went wrong answering that. Try again.", | ||
| "estimated_end": "Estimated end: {{time}}.", | ||
| "external_resources": "- External / mutual aid resources tracked: {{count}}", | ||
| "important": "Important: {{text}}", | ||
| "lane_empty": "Nothing is assigned to this lane.", | ||
| "lane_header": "{{lane}} ({{type}}): {{count}} resources.", | ||
| "lane_lead": "Lead: {{name}}.", | ||
| "lane_line": "- {{lane}}: {{count}} — {{names}}", | ||
| "lane_not_found": "I don't see a lane called \"{{lane}}\". Lanes on the board: {{lanes}}", | ||
| "lane_objective": "Primary objective: {{name}} ({{progress}}%).", | ||
| "missing_benchmarks": "Common {{type}} benchmarks not on the board yet: {{benchmarks}}", | ||
| "need_row": "- {{name}} [{{category}}] {{quantity}} — open {{age}}", | ||
| "needs_all_met": "Everything ordered has been filled.", | ||
| "needs_header": "{{incident}}: {{outstanding}} needs outstanding ({{met}} met, {{cancelled}} cancelled).", | ||
| "no_answer": "I couldn't answer that one.", | ||
| "no_board": "No command board is loaded for this incident yet.", | ||
| "no_lanes": "none yet", | ||
| "no_lanes_yet": "No lanes have been created on {{incident}} yet, so there's no span of control to check.", | ||
| "no_lead": "no lead", | ||
| "no_needs": "No needs have been recorded on {{incident}}.", | ||
| "no_notes": "No status notes have been recorded on {{incident}}.", | ||
| "no_objectives": "No tactical objectives have been set on {{incident}} yet.", | ||
| "no_resources": "Nothing is assigned to {{incident}} yet.", | ||
| "no_rit": "I don't see a RIT/RIC lane on {{incident}}. On a working incident that's worth assigning before crews go interior.", | ||
| "no_timers": "No timers are running on {{incident}}.", | ||
| "no_unassigned": "Everything tracked on {{incident}} is placed in a lane.", | ||
| "note_row": "- {{time}}: {{body}} ({{who}})", | ||
| "notes_header": "{{incident}} — {{count}} status note(s):", | ||
| "objective_complete": "complete", | ||
| "objective_in_progress": "in progress", | ||
| "objective_overdue": " [past target]", | ||
| "objective_pending": "pending", | ||
| "objective_row": "{{name}} — {{status}} ({{progress}}%){{overdue}}", | ||
| "objective_summary": "Objectives: {{complete}} of {{total}} complete.", | ||
| "objectives_all_complete": "Every objective on the board is complete.", | ||
| "objectives_header": "{{incident}}: {{complete}} of {{total}} objectives complete. Still open:", | ||
| "offline_badge": "Offline", | ||
| "offline_cannot_answer": "I can't answer that without a connection. Offline I can still give you PAR, resources, objectives, needs, ICS positions, timers, the incident log, a briefing, and the checklist for this incident type.", | ||
| "offline_hint": "No connection — answers come from the board cached on this device.", | ||
| "open_needs": "{{count}} needs still open.", | ||
| "par_all_good": "Everyone is accounted for.", | ||
| "par_counts": "{{green}} green, {{warning}} approaching, {{critical}} overdue.", | ||
| "par_critical_header": "Overdue — not accounted for:", | ||
| "par_due_row": "- {{name}}: due in {{minutes}} min", | ||
| "par_header": "PAR for {{incident}}: {{count}} personnel tracked.", | ||
| "par_none": "No personnel accountability is being tracked on {{incident}} — nobody has checked in and no check-in timer is running.", | ||
| "par_overdue_row": "- {{name}}: {{minutes}} min overdue", | ||
| "par_summary": "PAR: {{total}} tracked, {{critical}} overdue, {{warning}} approaching.", | ||
| "par_warning_header": "Approaching check-in:", | ||
| "placeholder": "Ask about this incident", | ||
| "resource_counts": "{{units}} units and {{personnel}} personnel across {{lanes}} lanes ({{unassigned}} unassigned).", | ||
| "resources_header": "{{incident}}: {{units}} units and {{personnel}} personnel working.", | ||
| "rit_found": "{{lane}} is standing by with {{count}} resource(s).", | ||
| "role_filled": "{{role}}: {{name}}", | ||
| "role_row": "{{role}}: {{name}}", | ||
| "role_unfilled": "No {{role}} is assigned on {{incident}}.", | ||
| "role_unknown": "I don't recognize the position \"{{role}}\". Ask for ICS roles to see everything assigned.", | ||
| "roles_header": "{{incident}}: {{count}} ICS position(s) assigned.", | ||
| "roles_unfilled": "Unfilled positions a {{type}} usually needs: {{roles}}", | ||
| "send": "Send", | ||
| "source_device": "Answered on this device", | ||
| "source_server": "Answered by Resgrid", | ||
| "span_all_good": "Span of control looks reasonable — every lane is within its limits and no lane is over {{ceiling}} resources.", | ||
| "span_header": "{{incident}}: {{lanes}} lanes, {{resources}} resources assigned.", | ||
| "span_no_lead": "- No lead assigned: {{lanes}}", | ||
| "span_over_row": "- {{lane}} is carrying {{count}} resources (limit {{limit}}) — consider splitting it or adding a supervisor.", | ||
| "span_under_row": "- {{lane}} has {{count}} resources against a minimum of {{minimum}}.", | ||
| "staging": "Staging: {{location}}.", | ||
| "suggestions": { | ||
| "briefing": "Transfer of command", | ||
| "missing": "What am I missing?", | ||
| "on_scene": "What's on scene?", | ||
| "open_needs": "Open needs", | ||
| "open_objectives": "Open objectives", | ||
| "par": "PAR", | ||
| "recent": "Last 30 minutes", | ||
| "rit": "Do I have a RIT?", | ||
| "safety": "Safety Officer", | ||
| "span": "Span of control", | ||
| "status": "Incident status", | ||
| "timers": "Timers", | ||
| "unassigned": "Unassigned", | ||
| "unfilled_roles": "Unfilled ICS positions", | ||
| "wind": "Wind and weather" | ||
| }, | ||
| "thinking": "Checking the board…", | ||
| "this_incident": "this incident", | ||
| "time_in_lane": "({{duration}} in lane)", | ||
| "timeline_empty": "Nothing has been logged on {{incident}} yet.", | ||
| "timeline_empty_window": "Nothing has been logged in the last {{minutes}} minutes on {{incident}}.", | ||
| "timeline_header": "{{incident}} — last {{count}} log entries:", | ||
| "timeline_row": "- {{time}}: {{description}}{{who}}", | ||
| "timeline_window_header": "{{incident}} — last {{minutes}} minutes ({{count}} entries):", | ||
| "timer_due_row": "- {{name}}: DUE NOW", | ||
| "timer_no_due_row": "- {{name}}: running", | ||
| "timer_running_row": "- {{name}}: due in {{remaining}}", | ||
| "timers_header": "{{incident}}: {{count}} timer(s).", | ||
| "title": "Incident Assistant", | ||
| "unassigned_header": "Unassigned on {{incident}} ({{count}}):", | ||
| "unassigned_line": "- Unassigned pool: {{count}}", | ||
| "unknown": "unknown" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Translate the added assistant namespace for each locale.
The new incident_assistant values are English in every listed non-English locale. Users selected for French, Italian, Polish, Swedish, or Ukrainian still receive English incident guidance, including offline and accountability messages. Translate each value while preserving keys and interpolation tokens.
src/translations/fr.json#L954-L1092: provide French values.src/translations/it.json#L954-L1092: provide Italian values.src/translations/pl.json#L954-L1092: provide Polish values.src/translations/sv.json#L954-L1092: provide Swedish values.src/translations/uk.json#L954-L1092: provide Ukrainian values.
As per coding guidelines, keep translation keys identical across all supported locale files.
📍 Affects 5 files
src/translations/fr.json#L954-L1092(this comment)src/translations/it.json#L954-L1092src/translations/pl.json#L954-L1092src/translations/sv.json#L954-L1092src/translations/uk.json#L954-L1092
🤖 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/fr.json` around lines 954 - 1092, Translate every value in
the incident_assistant namespace into the target locale while preserving all
keys and interpolation tokens. Apply the translations in
src/translations/fr.json lines 954-1092, src/translations/it.json lines
954-1092, src/translations/pl.json lines 954-1092, src/translations/sv.json
lines 954-1092, and src/translations/uk.json lines 954-1092; keep the namespace
structure and key sets identical across all locale files.
Source: Coding guidelines
This comment has been minimized.
This comment has been minimized.
Signing out while initializeApp was still awaiting left the stale run free to connect the SignalR hubs and mark the app initialized for a session that had already ended. Capture a generation token at the start of each run, bump it whenever the session leaves the signed-in state, and bail at every checkpoint that is no longer current. Clearing the in-progress guard on sign-out also stops a fast sign-out then sign-in from being skipped as "already initializing", which previously left the new session uninitialized with no retry pending. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X8YKbDjQeSLXJs4kU1qdXe
| * 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.
Missing error handling: this external HTTP call to api.post has no try/catch. Rule [27] requires network/external calls to be wrapped in try/catch with context (callId/question) added and errors mapped to application-level errors.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/api/chat/chatbot.ts:
Line 37:
Missing error handling: this external HTTP call to `api.post` has no try/catch. Rule [27] requires network/external calls to be wrapped in try/catch with context (`callId`/`question`) added and errors mapped to application-level errors.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| * 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) => { |
There was a problem hiding this comment.
Missing JSDoc return documentation: askIncidentAssistant omits 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
File src/api/chat/chatbot.ts:
Line 36:
Missing JSDoc return documentation: `askIncidentAssistant` omits 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<...>}`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| setEditMessage(m); | ||
| setEditText(m.Body ?? ''); | ||
| }} | ||
| onEdit={() => undefined} |
There was a problem hiding this comment.
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 LLM
File src/app/(app)/chatbot.tsx:
Line 158:
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.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const raw = parseMetadata<Record<string, unknown>>(metadataJson); | ||
| if (!raw) return null; | ||
| const nested = (raw.gif ?? raw.Gif) as Record<string, unknown> | undefined; | ||
| const source = nested ?? raw; |
There was a problem hiding this comment.
Duplicated statement sequence: the four-step parse → null-guard → extract nested-or-flat key → assign source pattern appears in both parseLocationMetadata (lines 92–95) and parseGifMetadata (lines 103–106). Extract a reusable helper like extractMetadataSource(metadataJson, camelKey, pascalKey) to eliminate drift risk.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File src/components/chat/chat-utils.ts:
Line 103 to 106:
Duplicated statement sequence: the four-step parse → null-guard → extract nested-or-flat key → assign `source` pattern appears in both `parseLocationMetadata` (lines 92–95) and `parseGifMetadata` (lines 103–106). Extract a reusable helper like `extractMetadataSource(metadataJson, camelKey, pascalKey)` to eliminate drift risk.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return; | ||
| } | ||
| setQuestion(''); | ||
| void useIncidentAssistantStore.getState().ask(callId, trimmed, t); |
There was a problem hiding this comment.
Unhandled promise rejection: the async ask call is fire-and-forget with void, silently swallowing rejections from network failures or backend errors. Wrap in try/catch with await or chain .catch() to log/handle the error.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/components/command/assistant-sheet.tsx:
Line 59:
Unhandled promise rejection: the async `ask` call is fire-and-forget with `void`, silently swallowing rejections from network failures or backend errors. Wrap in try/catch with `await` or chain `.catch()` to log/handle the error.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ) : ( | ||
| <VStack space="sm" testID="incident-assistant-messages"> | ||
| {messages.map((entry) => | ||
| entry.role === 'user' ? ( |
There was a problem hiding this comment.
Magic string 'user' represents a finite role member without compile-time safety. Define a const object (e.g., const MessageRole = { User: 'user', Assistant: 'assistant' } as const) and reference it instead.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/components/command/assistant-sheet.tsx:
Line 121:
Magic string `'user'` represents a finite role member without compile-time safety. Define a const object (e.g., `const MessageRole = { User: 'user', Assistant: 'assistant' } as const`) and reference it instead.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ); | ||
| }; | ||
|
|
||
| export default IncidentAssistantSheet; |
There was a problem hiding this comment.
Default export reduces clarity and harms refactoring safety. Use a named export: export { IncidentAssistantSheet }; and update importing files accordingly.
Kody rule violation: Avoid default exports
Prompt for LLM
File src/components/command/assistant-sheet.tsx:
Line 180:
Default export reduces clarity and harms refactoring safety. Use a named export: `export { IncidentAssistantSheet };` and update importing files accordingly.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // --------------------------------------------------------------------------- | ||
|
|
||
| export const answerStatus = (context: IncidentAnswerContext, t: TFunction): string => { | ||
| const board = context.board!; |
There was a problem hiding this comment.
Non-null assertion (!) on context.board (typed IncidentCommandBoard | null) suppresses compiler checks without adding a runtime guard, risking a TypeError on every subsequent dereference. Guard with an early return or use optional chaining throughout.
Kody rule violation: Add null checks before accessing properties
Prompt for LLM
File src/services/incident-assistant/answerers.ts:
Line 230:
Non-null assertion (`!`) on `context.board` (typed `IncidentCommandBoard | null`) suppresses compiler checks without adding a runtime guard, risking a `TypeError` on every subsequent dereference. Guard with an early return or use optional chaining throughout.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const isUnitKind = (kind: number): boolean => kind === ResourceAssignmentKind.RealUnit || kind === ResourceAssignmentKind.LinkedDeptUnit || kind === ResourceAssignmentKind.AdHocUnit; | ||
|
|
||
| const isCriticalPar = (row: PersonnelCallCheckInStatus): boolean => row.Status === 'Critical' || row.NeedsCheckIn; |
There was a problem hiding this comment.
Magic string 'Critical' is used inline for status comparison without refactor safety. Extract a shared constant like PAR_STATUS_CRITICAL = 'Critical' to a constants module.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/services/incident-assistant/answerers.ts:
Line 101:
Magic string `'Critical'` is used inline for status comparison without refactor safety. Extract a shared constant like `PAR_STATUS_CRITICAL = 'Critical'` to a constants module.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }); | ||
| } | ||
|
|
||
| const inLane = assignments.filter((a) => a.CommandStructureNodeId === node.CommandStructureNodeId); |
There was a problem hiding this comment.
Repeated predicate a => a.CommandStructureNodeId === node.CommandStructureNodeId appears at lines 323, 352, 444, and 597, risking copy-paste bugs. Extract a reusable helper like assignmentsInLane(assignments, nodeId).
Kody rule violation: Extract common query logic
Prompt for LLM
File src/services/incident-assistant/answerers.ts:
Line 296:
Repeated predicate `a => a.CommandStructureNodeId === node.CommandStructureNodeId` appears at lines 323, 352, 444, and 597, risking copy-paste bugs. Extract a reusable helper like `assignmentsInLane(assignments, nodeId)`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| 'Initial size-up / CAN report (Conditions, Actions, Needs) transmitted', | ||
| 'Incident action plan recorded on the board', | ||
| 'Safety Officer assigned once the incident is working', | ||
| 'Accountability (PAR) timer running', |
There was a problem hiding this comment.
Unsafe type cast violates team rule 'Use safe type casting with as operator'. Apply the as operator or pattern matching for safe casts and guard null results before usage.
Prompt for LLM
File src/services/incident-assistant/ics-playbooks.ts:
Line 90:
Unsafe type cast violates team rule 'Use safe type casting with as operator'. Apply the `as` operator or pattern matching for safe casts and guard null results before usage.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| * an ordinary personnel question rather than being swallowed as a role query. | ||
| */ | ||
| const ROLE_WORDS = | ||
| '(ic|incident\\s+commander|deputy(\\s+incident)?(\\s+commander)?|commander|unified\\s+command|safety(\\s+officer)?|' + |
There was a problem hiding this comment.
String concatenation using + reduces readability and is error-prone. Replace with template literals for improved clarity.
Kody rule violation: Use Template Literals Instead of String Concatenation
Prompt for LLM
File src/services/incident-assistant/intent-matcher.ts:
Line 49:
String concatenation using `+` reduces readability and is error-prone. Replace with template literals for improved clarity.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| // A confident on-device match is answered on-device even with a connection: it is instant, | ||
| // costs nothing, and says exactly what the server would say. | ||
| if (!local.requiresServer && local.confidence >= 1 && local.answer) { |
There was a problem hiding this comment.
Unnamed confidence threshold: the literal 1 provides no context for whether it means '100% match' or another cutoff. Extract a named constant such as FULL_CONFIDENCE_THRESHOLD = 1 at module scope.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/stores/command/assistant-store.ts:
Line 121:
Unnamed confidence threshold: the literal `1` provides no context for whether it means '100% match' or another cutoff. Extract a named constant such as `FULL_CONFIDENCE_THRESHOLD = 1` at module scope.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| callType: call?.Type ?? null, | ||
| callNature: call?.Nature ?? null, | ||
| resolveUserName: (userId: string) => { | ||
| const user = users.find((u) => u.UserId === userId); |
There was a problem hiding this comment.
O(n) linear scan: resolveUserName calls Array.find over the entire users array per invocation, resulting in O(n*m) overall when resolving many names. Build a Map<string, User> keyed by UserId once inside buildAnswerContext for O(1) lookups.
Kody rule violation: Use Dictionary lookups instead of linear searches
Prompt for LLM
File src/stores/command/assistant-store.ts:
Line 75:
O(n) linear scan: `resolveUserName` calls `Array.find` over the entire `users` array per invocation, resulting in O(n*m) overall when resolving many names. Build a `Map<string, User>` keyed by `UserId` once inside `buildAnswerContext` for O(1) lookups.
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:
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(app)/_layout.tsx (1)
180-205: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winCheck the generation before each subsequent external operation.
Line 180 runs only after all six store operations finish. Lines 182-183 start both hub connections before the next check. If the session retires while
connectUpdateHub()is pending, this run can still startconnectGeolocationHub().Add an
isCurrentRun()check after every awaited operation, including after the chat-hub attempt and beforesyncFromServer()starts.🤖 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)/_layout.tsx around lines 180 - 205, Update the initialization flow around connectUpdateHub, connectGeolocationHub, and connectChatHub to call isCurrentRun() after each awaited hub operation, including when the chat attempt fails. Add a final generation check after the chat try/catch and before starting syncFromServer(), while preserving the existing early-return behavior.
🧹 Nitpick comments (1)
src/app/chat/thread/[messageId].tsx (1)
136-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a stable no-op typing callback.
Line 138 creates a new
onTypingcallback on everyThreadScreenrender. Define a typed no-op once outside the component and pass it toMessageComposer. This keeps the composer callback dependencies stable. As per coding guidelines, “avoid anonymous functions inrenderItemand event handlers.”🤖 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 136 - 138, Define a typed, stable no-op typing callback outside the ThreadScreen component, then pass that named callback to MessageComposer instead of the inline onTyping arrow. Keep the existing onSendText, onSendLocation, placeholder, and allowUrgent behavior unchanged.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)/__tests__/init-session-generation.test.tsx:
- Around line 70-139: Capture the unmount function returned by each renderHook
call in the three useInitGuard tests, and call unmount() during each test’s
cleanup after assertions or awaited work completes. Preserve the existing
initialization and sign-out behavior while ensuring every rendered hook is
explicitly unmounted.
In `@src/components/chat/__tests__/message-composer.test.tsx`:
- Around line 48-93: Update the MessageComposer attachment-actions tests to
capture each render result and unmount the rendered component during afterEach
cleanup. Apply this consistently to every test in the “MessageComposer
attachment actions” suite, while preserving the existing beforeEach mock reset
and assertions.
In `@src/services/signalr.service.ts`:
- Around line 237-239: Update the manual fallback reconnection flow in
handleConnectionClose/connectToHubWithEventingUrl to emit HUB_RECONNECTED_EVENT
after the replacement connection successfully reaches Connected state. Preserve
the existing onreconnected emission for automatic reconnects, and ensure the
fallback event uses the affected hub’s config.name so scoped chat listeners
re-arm.
In `@src/stores/signalr/signalr-store.ts`:
- Around line 615-617: Update runChatArm and the disconnect cleanup around
stopChatArmRetry to use a connection generation or cancellation token that
invalidates in-flight arm operations. Increment or cancel it on disconnect, then
verify it after every await before scheduling retries, starting the heartbeat,
or marking the connection as connected; preserve existing behavior for the
current connection.
- Around line 580-583: Reset lastChatResyncAt at the start of onChatReconnected,
before calling armChatSession, matching the reset performed by
onChatDisconnected so reconnect arming always permits the required chat resync.
---
Outside diff comments:
In `@src/app/`(app)/_layout.tsx:
- Around line 180-205: Update the initialization flow around connectUpdateHub,
connectGeolocationHub, and connectChatHub to call isCurrentRun() after each
awaited hub operation, including when the chat attempt fails. Add a final
generation check after the chat try/catch and before starting syncFromServer(),
while preserving the existing early-return behavior.
---
Nitpick comments:
In `@src/app/chat/thread/`[messageId].tsx:
- Around line 136-138: Define a typed, stable no-op typing callback outside the
ThreadScreen component, then pass that named callback to MessageComposer instead
of the inline onTyping arrow. Keep the existing onSendText, onSendLocation,
placeholder, and allowUrgent behavior unchanged.
🪄 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: 42a7ed75-1130-4cff-bb7b-2ad3d4e4cd83
📒 Files selected for processing (8)
src/app/(app)/__tests__/init-session-generation.test.tsxsrc/app/(app)/_layout.tsxsrc/app/chat/[channelId].tsxsrc/app/chat/thread/[messageId].tsxsrc/components/chat/__tests__/message-composer.test.tsxsrc/components/chat/message-composer.tsxsrc/services/signalr.service.tssrc/stores/signalr/signalr-store.ts
💤 Files with no reviewable changes (1)
- src/app/chat/[channelId].tsx
| 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]); | ||
| }); | ||
|
|
||
| // Exactly one run reached the effects: the current one. | ||
| expect(effects.markInitialized).toHaveBeenCalledTimes(1); | ||
| expect(effects.startLocation).toHaveBeenCalledTimes(1); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Unmount each hook after the test.
Each renderHook call leaves cleanup to the test library. Capture and call unmount() during cleanup.
As per coding guidelines, src/**/*.test.{ts,tsx} must “call unmount() to clean up.”
🤖 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)/__tests__/init-session-generation.test.tsx around lines 70 -
139, Capture the unmount function returned by each renderHook call in the three
useInitGuard tests, and call unmount() during each test’s cleanup after
assertions or awaited work completes. Preserve the existing initialization and
sign-out behavior while ensuring every rendered hook is explicitly unmounted.
Source: Coding guidelines
| describe('MessageComposer attachment actions', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('offers image and GIF when both callbacks are provided', () => { | ||
| render(<MessageComposer {...baseProps} onSendImage={jest.fn()} onOpenGif={jest.fn()} />); | ||
|
|
||
| expect(screen.queryByLabelText('chat.add_image')).not.toBeNull(); | ||
| expect(screen.queryByLabelText('chat.add_gif')).not.toBeNull(); | ||
| expect(screen.queryByLabelText('chat.emoji')).not.toBeNull(); | ||
| }); | ||
|
|
||
| it('hides both when neither callback is provided, as thread replies do', () => { | ||
| render(<MessageComposer {...baseProps} allowUrgent={false} />); | ||
|
|
||
| expect(screen.queryByLabelText('chat.add_image')).toBeNull(); | ||
| expect(screen.queryByLabelText('chat.add_gif')).toBeNull(); | ||
| // The actions a thread can still perform stay available. | ||
| expect(screen.queryByLabelText('chat.emoji')).not.toBeNull(); | ||
| expect(screen.queryByLabelText('chat.share_location')).not.toBeNull(); | ||
| }); | ||
|
|
||
| it('hides only the GIF action when images are supported but GIFs are not', () => { | ||
| render(<MessageComposer {...baseProps} onSendImage={jest.fn()} />); | ||
|
|
||
| expect(screen.queryByLabelText('chat.add_image')).not.toBeNull(); | ||
| expect(screen.queryByLabelText('chat.add_gif')).toBeNull(); | ||
| }); | ||
|
|
||
| it('hides only the image action when GIFs are supported but images are not', () => { | ||
| render(<MessageComposer {...baseProps} onOpenGif={jest.fn()} />); | ||
|
|
||
| expect(screen.queryByLabelText('chat.add_image')).toBeNull(); | ||
| expect(screen.queryByLabelText('chat.add_gif')).not.toBeNull(); | ||
| }); | ||
|
|
||
| it('invokes the GIF callback when the action is used', () => { | ||
| const onOpenGif = jest.fn(); | ||
| render(<MessageComposer {...baseProps} onOpenGif={onOpenGif} />); | ||
|
|
||
| fireEvent.press(screen.getByLabelText('chat.add_gif')); | ||
|
|
||
| expect(onOpenGif).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Unmount each rendered composer.
Capture each render() result and call unmount() during afterEach. This ensures component cleanup runs consistently. As per coding guidelines, “always call unmount() during cleanup.”
🤖 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/__tests__/message-composer.test.tsx` around lines 48 -
93, Update the MessageComposer attachment-actions tests to capture each render
result and unmount the rendered component during afterEach cleanup. Apply this
consistently to every test in the “MessageComposer attachment actions” suite,
while preserving the existing beforeEach mock reset and assertions.
Source: Coding guidelines
| // A reconnect issues a new connection id, so any server-side group this connection | ||
| // belonged to is gone. Subscribers must re-announce themselves. | ||
| this.emitHubLifecycle(SignalRService.HUB_RECONNECTED_EVENT, config.name); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'signalr.service.test.ts' src/services --exec sh -c '
rg -n -C 5 "onclose|onreconnected|handleConnectionClose|HUB_RECONNECTED_EVENT" "$1"
' sh {}Repository: Resgrid/IC
Length of output: 6643
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'signalr.service.ts' src/services | while read -r file; do
echo "== $file =="
wc -l "$file"
sed -n '200,260p' "$file" | cat -n -v
sed -n '640,700p' "$file" | cat -n -v
done
echo "== Search for lifecycle/chat rearm =="
rg -n -C 4 "HUB_RECONNECTED_EVENT|onCloseCallback|handleConnectionClose|connectToHubWithEventingUrl|Connect|heartbeat|chat" src | head -n 240Repository: Resgrid/IC
Length of output: 23980
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate signalr service source around connect methods =="
rg -n -C 6 "handleConnectionClose|connectToHubWithEventingUrl|emitHubLifecycle\\(SignalRService\\.HUB_RECONNECTED_EVENT|start\\(|invoke\\(|reconnectAttempts" src/services/signalr.service.ts
echo "== Locate chat connection state/rearm listeners =="
rg -n -C 6 "HUB_RECONNECTED_EVENT|connectionState|setConnectionState|clear|Chat|Connect|heartbeat|signalRService\\.on\\(" src/stores src/components src/services | head -n 260Repository: Resgrid/IC
Length of output: 27637
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== signalr.service.ts fallback path =="
sed -n '414,500p' src/services/signalr.service.ts | cat -n -v
echo "== occurrences of __hubReconnected/event listeners =="
rg -n -C 5 "__hubReconnected|HUB_RECONNECTED_EVENT|signalRService\.on\\(" src | head -n 260
echo "== occurrences of close/chat state/rearm references =="
rg -n -C 5 "connectionState|rearm|arm|heartbeat|Connect|__hubDisconnected|hubDisconnected" src/stores src/services src/components src/lib | head -n 300Repository: Resgrid/IC
Length of output: 43810
Emit the reconnect lifecycle event after fallback reconnection.
connection.onreconnected only fires for SignalR’s automatic reconnect path. When handleConnectionClose schedules a manual fallback via connectToHubWithEventingUrl, the replacement connection reaches Connected state without emitting this event. Chat listeners clear connection state on close and re-arm only on the scoped reconnect event, so chat can remain silent after terminal reconnection. Emit HUB_RECONNECTED_EVENT after successful fallback reconnection completes.
🤖 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/signalr.service.ts` around lines 237 - 239, Update the manual
fallback reconnection flow in handleConnectionClose/connectToHubWithEventingUrl
to emit HUB_RECONNECTED_EVENT after the replacement connection successfully
reaches Connected state. Preserve the existing onreconnected emission for
automatic reconnects, and ensure the fallback event uses the affected hub’s
config.name so scoped chat listeners re-arm.
| const onChatReconnected = () => { | ||
| void armChatSession({ resetAttempts: true }).catch(() => { | ||
| // runChatArm already logged and scheduled its retry. | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'signalr-store.test.ts' src/stores/signalr --exec sh -c '
rg -n -C 5 "HUB_RECONNECTED_EVENT|lastChatResyncAt|resyncChat|useFakeTimers|useRealTimers" "$1"
' sh {}Repository: Resgrid/IC
Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target source and all signalr-related test files.
fd -a 'signalr-store\.ts|signalr.*test.*\.(ts|tsx)$' src/stores src | sort
echo '--- target snippet ---'
if [ -f src/stores/signalr/signalr-store.ts ]; then
nl -ba src/stores/signalr/signalr-store.ts | sed -n '520,625p'
fiRepository: Resgrid/IC
Length of output: 893
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- target source slice ---'
awk 'NR>=520 && NR<=625 { printf "%7d %s\n", NR, $0 }' src/stores/signalr/signalr-store.ts
echo '--- relevant signalr-store state/events/resync code ---'
rg -n -C 4 'lastChatResyncAt|resyncChat|HUB_RECONNECTED_EVENT|disconnectChatHub|onChatDisconnected|handleChatDisconnect' src/stores/signalr/signalr-store.ts src/stores/signalr/__tests__/signalr-store.test.ts
echo '--- test lifecycle related snippets ---'
rg -n -C 4 'HUB_RECONNECTED_EVENT|lastChatResyncAt|resyncChat|useFakeTimers|useRealTimers|reconnect' src/app/\(app\)/__tests__/signalr-lifecycle.test.tsx src/hooks/__tests__/use-signalr-lifecycle.test.tsx src/services/__tests__/signalr.service.reconnect-fix.test.ts src/services/__tests__/signalr.service.test.ts src/stores/signalr/__tests__/signalr-store.test.tsRepository: Resgrid/IC
Length of output: 50366
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- constants and top-level resync/arm helpers ---'
awk 'NR>=1 && NR<=220 { printf "%7d %s\n", NR, $0 }' src/stores/signalr/signalr-store.ts
echo '--- onreconnected implementation in signalR service ---'
rg -n -C 7 'onreconnected|HUB_RECONNECTED_EVENT|setHubState|Reconnect|reconnect' src/services signalr.service.ts src/services/signalr.service.ts 2>/dev/null || true
echo '--- tests that cover onChatReconnected / lastChatResyncAt ---'
rg -n -C 6 'onChatReconnected|lastChatResyncAt|reconnected|lastChatResyncAt|HUB_RECONNECTED_EVENT|resyncChat' src/stores/signalr/__tests__/signalr-store.test.tsRepository: Resgrid/IC
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const CHAT_RESYNC_DEBOUNCE_MS = 2000;
let lastChatResyncAt = 0;
function resyncChat() {
const now = Date.now();
if (now - lastChatResyncAt < CHAT_RESYNC_DEBOUNCE_MS) return;
lastChatResyncAt = now;
return 'handleChatConnectedCalled';
}
async function armChatSession({ resetAttempts } = {}) {
if (resetAttempts) chatArmAttempts = 0;
await Promise.resolve();
resyncChat();
}
// Simulate a 1500ms automatic reconnect after the last resync timestamp was recorded.
let chatArmAttempts = 0;
lastChatResyncAt = Date.now();
void (async () => {
jestAdvance(1500);
console.log('resync after 1500ms:', resyncChat());
})().then(() => {
// No explicit output expected for demonstration.
});
function jestAdvance(ms) {
lastChatResyncAt += ms;
}
JSRepository: Resgrid/IC
Length of output: 179
Reset lastChatResyncAt in the reconnect handler before arming.
HUB_RECONNECTED_EVENT does not reset lastChatResyncAt, while onChatDisconnected does. If an automatic reconnect completes within 2000 ms of the last resync, resyncChat() returns early inside runChatArm() and the new connection can miss message backfill for the reconnect gap.
🤖 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 580 - 583, Reset
lastChatResyncAt at the start of onChatReconnected, before calling
armChatSession, matching the reset performed by onChatDisconnected so reconnect
arming always permits the required chat resync.
| stopChatArmRetry(); | ||
| chatArmAttempts = 0; | ||
| lastChatResyncAt = 0; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Invalidate an active arm operation during disconnect.
stopChatArmRetry() only clears an existing timer. If runChatArm is awaiting invoke, it can schedule a new retry or start a heartbeat after this cleanup. Add a connection generation or cancellation token. Check it after each await before scheduling retries, starting the heartbeat, or setting connected state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/stores/signalr/signalr-store.ts` around lines 615 - 617, Update
runChatArm and the disconnect cleanup around stopChatArmRetry to use a
connection generation or cancellation token that invalidates in-flight arm
operations. Increment or cancel it on disconnect, then verify it after every
await before scheduling retries, starting the heartbeat, or marking the
connection as connected; preserve existing behavior for the current connection.
|
|
||
| await act(async () => { | ||
| first.resolve(); | ||
| await Promise.all([pending, second]); |
There was a problem hiding this comment.
Promise.all([pending, second]) aborts remaining tasks on first rejection instead of settling all items independently. Replace with Promise.allSettled and handle per-item results for batch operations with partial failures.
Kody rule violation: Use Promise.allSettled for batch operations with partial failures
Prompt for LLM
File src/app/(app)/__tests__/init-session-generation.test.tsx:
Line 133:
`Promise.all([pending, second])` aborts remaining tasks on first rejection instead of settling all items independently. Replace with `Promise.allSettled` and handle per-item results for batch operations with partial failures.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const MAX_INIT_RETRIES = 3; | ||
| useEffect(() => { | ||
| if (status !== 'signedIn' && initRetryCount > 0) { | ||
| if (status === 'signedIn') return; |
There was a problem hiding this comment.
Raw string literal 'signedIn' represents a finite-set auth status inline, lacking the type safety and discoverability already established by constants like FeatureFlagKeys.ChatSystem. Extract it into an AuthStatus enum or as const object and reference AuthStatus.SignedIn in the comparison.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/app/(app)/_layout.tsx:
Line 284:
Raw string literal `'signedIn'` represents a finite-set auth status inline, lacking the type safety and discoverability already established by constants like `FeatureFlagKeys.ChatSystem`. Extract it into an `AuthStatus` enum or `as const` object and reference `AuthStatus.SignedIn` in the comparison.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const MAX_INIT_RETRIES = 3; | ||
| useEffect(() => { | ||
| if (status !== 'signedIn' && initRetryCount > 0) { | ||
| if (status === 'signedIn') return; |
There was a problem hiding this comment.
Shared string literal 'signedIn' is inlined directly in the comparison, creating duplication risk across consumers. Extract it into a centralized constant (e.g., export const SIGNED_IN = 'signedIn') and import it here so every reference points to a single source of truth.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/app/(app)/_layout.tsx:
Line 284:
Shared string literal `'signedIn'` is inlined directly in the comparison, creating duplication risk across consumers. Extract it into a centralized constant (e.g., `export const SIGNED_IN = 'signedIn'`) and import it here so every reference points to a single source of truth.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <MessageComposer onSendText={handleSendText} onSendImage={() => undefined} onSendLocation={handleSendLocation} onOpenGif={handleSendGif} onTyping={() => undefined} placeholder={t('chat.reply_placeholder')} allowUrgent={false} /> | ||
| {/* Threads carry text and location only; omitting the image/GIF callbacks keeps | ||
| those actions out of the composer instead of showing dead buttons. */} | ||
| <MessageComposer onSendText={handleSendText} onSendLocation={handleSendLocation} onTyping={() => undefined} placeholder={t('chat.reply_placeholder')} allowUrgent={false} /> |
There was a problem hiding this comment.
Inline arrow function in JSX prop creates a new function instance on every render, violating the team rule against .bind() or arrow functions in JSX props. Move the onTyping handler definition outside the render method.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/chat/thread/[messageId].tsx:
Line 138:
Inline arrow function in JSX prop creates a new function instance on every render, violating the team rule against `.bind()` or arrow functions in JSX props. Move the `onTyping` handler 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.
| }); | ||
| if (chatArmAttempts < CHAT_ARM_MAX_ATTEMPTS) { | ||
| chatArmRetryTimer = setTimeout(() => { | ||
| void armChatSession(); |
There was a problem hiding this comment.
Unhandled promise rejection in the setTimeout retry callback. void armChatSession() fires without a .catch() handler, so if the retry exhausts max attempts and rejects, the rejection propagates unguarded. Attach a .catch() that logs with structured context.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 104:
Unhandled promise rejection in the `setTimeout` retry callback. `void armChatSession()` fires without a `.catch()` handler, so if the retry exhausts max attempts and rejects, the rejection propagates unguarded. Attach a `.catch()` that logs with structured context.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| void armChatSession({ resetAttempts: true }).catch(() => { | ||
| // runChatArm already logged and scheduled its retry. | ||
| }); |
There was a problem hiding this comment.
Empty .catch(() => { /* comment */ }) block silently swallows armChatSession errors without structured logging, violating the rule requiring explicit handling with context. Add at minimum a logger.debug call so the suppression is visible at this call site (also found in src/stores/signalr/signalr-store.ts:114-116).
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 581 to 583:
Empty `.catch(() => { /* comment */ })` block silently swallows `armChatSession` errors without structured logging, violating the rule requiring explicit handling with context. Add at minimum a `logger.debug` call so the suppression is visible at this call site (also found in `src/stores/signalr/signalr-store.ts:114-116`).
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (chatArmAttempts < CHAT_ARM_MAX_ATTEMPTS) { | ||
| chatArmRetryTimer = setTimeout(() => { | ||
| void armChatSession(); | ||
| }, CHAT_ARM_RETRY_MS); | ||
| } |
There was a problem hiding this comment.
Race condition between retry scheduling and teardown in runChatArm. disconnectChatHub unregisters the onChatDisconnected handler (line 618) before calling disconnectFromHub (line 619), so when connection.stop() triggers onclose and rejects the in-flight invoke('Connect'), the catch block schedules a fresh retry timer that neither stopChatArmRetry (already ran) nor onChatDisconnected (already unregistered) can clear — causing wasted invoke('Connect') calls and log warnings after sign-out. Add a module-level chatArmDisposed flag set by disconnectChatHub and checked before scheduling and firing retries.
} catch (error) {
chatArmAttempts += 1;
logger.warn({
message: 'Failed to announce presence to chat hub',
context: { error, attempt: chatArmAttempts, maxAttempts: CHAT_ARM_MAX_ATTEMPTS },
});
if (chatArmAttempts < CHAT_ARM_MAX_ATTEMPTS && !chatArmDisposed) {
chatArmRetryTimer = setTimeout(() => {
if (!chatArmDisposed) void armChatSession();
}, CHAT_ARM_RETRY_MS);
}
throw error;
}Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 102 to 106:
Race condition between retry scheduling and teardown in `runChatArm`. `disconnectChatHub` unregisters the `onChatDisconnected` handler (line 618) before calling `disconnectFromHub` (line 619), so when `connection.stop()` triggers `onclose` and rejects the in-flight `invoke('Connect')`, the catch block schedules a fresh retry timer that neither `stopChatArmRetry` (already ran) nor `onChatDisconnected` (already unregistered) can clear — causing wasted `invoke('Connect')` calls and log warnings after sign-out. Add a module-level `chatArmDisposed` flag set by `disconnectChatHub` and checked before scheduling and firing retries.
Suggested Code:
} catch (error) {
chatArmAttempts += 1;
logger.warn({
message: 'Failed to announce presence to chat hub',
context: { error, attempt: chatArmAttempts, maxAttempts: CHAT_ARM_MAX_ATTEMPTS },
});
if (chatArmAttempts < CHAT_ARM_MAX_ATTEMPTS && !chatArmDisposed) {
chatArmRetryTimer = setTimeout(() => {
if (!chatArmDisposed) void armChatSession();
}, CHAT_ARM_RETRY_MS);
}
throw error;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
2 participants
Pull Request Description
This PR introduces an on-device Incident Assistant for the Command Board, along with several fixes to the chat/realtime infrastructure.
Incident Assistant (Primary Feature)
Adds a new assistant accessible from the Command Board that answers Incident Commander questions (PAR, resources, objectives, roles, span of control, needs, timeline, briefings, checklists, etc.) directly on-device from cached board data — requiring no network connection, model download, or server round-trip. This is critical because commanders most need situational answers when scenes have poor or no signal.
Chat & Realtime Fixes
userId, isOnlineseparately)JoinChannel,Typing, andMarkReadnow pass all required arguments, fixing channels that were permanently silent due to rejected SignalR invocationsReactionsandAttachmentsarrays, preventing crashes on messages persisted before normalization existed{location: {latitude, longitude}}) matching the web client's wire contract, while parsers remain backward-compatible with the old flat PascalCase formatOther
Summary by CodeRabbit