Conversation
This comment has been minimized.
This comment has been minimized.
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe update adds commander messaging, resource and lane detail workflows, tabbed maps, fullscreen command logs, notification mapping and localization, push-registration routing, realtime command synchronization, resilient unit loading, dispatch deduplication, and release-note normalization. ChangesCommand workflows
Notification inbox
Platform and data support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant CommandBoard
participant MessageCommanderSheet
participant CommandStore
participant IncidentCommandAPI
Operator->>CommandBoard: Open commander message
CommandBoard->>MessageCommanderSheet: Render message form
Operator->>MessageCommanderSheet: Submit message and deputy option
MessageCommanderSheet->>CommandBoard: Return validated message
CommandBoard->>CommandStore: Send commander message
CommandStore->>IncidentCommandAPI: POST SendMessageToCommand
IncidentCommandAPI-->>CommandStore: Return delivery count
CommandStore-->>CommandBoard: Return success status
CommandBoard-->>Operator: Show success or error toast
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/notifications/NotificationInbox.tsx (1)
246-275: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReconcile and log partial deletion failures.
Promise.allrejects on the first failed request while other deletes may still succeed. The catch skips refresh, leaves stale selected IDs, and discards the failure details. UsePromise.allSettled, retain only failed IDs, refresh after settlement, and log redacted error/count context before showing the toast.As per coding guidelines, “Handle asynchronous operations with
try/catch, logging failures and providing user feedback through toast notifications where appropriate.”🤖 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/notifications/NotificationInbox.tsx` around lines 246 - 275, The confirmBulkDelete flow should use Promise.allSettled so every selected deletion completes, retain only IDs whose deletions failed, refresh notifications after settlement, and update selection state to those failed IDs. Log redacted failure details with relevant count context before showing the existing error toast, while preserving the success toast and exit-selection behavior when all deletions succeed.Source: Coding guidelines
🧹 Nitpick comments (11)
src/app/resource-map/index.tsx (2)
26-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace hardcoded fallback marker colors with semantic tokens.
Keep lane-provided colors dynamic, but use semantic error/background tokens for the default marker and border. As per coding guidelines: “use semantic Tailwind color tokens instead of hardcoded hex values.”
Also applies to: 72-72
🤖 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/resource-map/index.tsx` at line 26, Update the color fallback in the resource-map marker styling to use the established semantic error/background token instead of the hardcoded `#E53E3E` value, while preserving validated lane-provided colors. Apply the same semantic token replacement to the default border styling at the referenced second location.Source: Coding guidelines
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the configured environment import.
Import
Envfrom@env, not@/lib/env, to preserve the project’s environment-access contract. As per coding guidelines: “Import environment values throughEnvfrom@env.”🤖 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/resource-map/index.tsx` at line 11, Update the Env import in the resource map module to use the configured `@env` alias instead of '`@/lib/env`', preserving the project’s standard environment-access contract.Source: Coding guidelines
src/app/call/[id].tsx (1)
550-550: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRender newly added Lucide icons directly.
Render the Lucide icons directly rather than through gluestack wrappers:
src/app/call/[id].tsx: renderMessageSquareIcondirectly inside the button.src/components/command/structure-section.tsx: renderEyedirectly instead of throughIcon.src/components/command/incident-maps-section.tsx: renderPlusdirectly inside the button.🤖 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/call/`[id].tsx at line 550, Render the Lucide icons directly instead of through gluestack wrappers: in src/app/call/[id].tsx lines 550-550, update the button to render MessageSquareIcon directly; in src/components/command/structure-section.tsx lines 164-165, render Eye directly instead of using Icon; and in src/components/command/incident-maps-section.tsx line 78, render Plus directly inside the button.Source: Coding guidelines
src/app/(app)/command.tsx (1)
815-827: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFilter row can leave an empty list with no explanation.
With
resourceFilterset toassigned/unassignedand no matches, the section renders nothing while the header count still shows the unfiltered total. A short "no matching resources" line would make the state self-explanatory.🤖 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)/command.tsx around lines 815 - 827, Update the resource list rendering near the resource-filter row and its section count so that when resourceFilter is assigned or unassigned and the filtered results are empty, it renders a short translated “no matching resources” message instead of only an empty list. Keep the existing unfiltered rendering and header behavior unchanged for filters with matches or the all option.src/components/command/maps-tabbed-card.tsx (2)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a type-only import statement.
All three specifiers are types.
♻️ Proposed change
-import { type IncidentCommand, type IncidentMap, type IncidentMapAnnotation } from '`@/models/v4/incidentCommand/incidentCommandModels`'; +import type { IncidentCommand, IncidentMap, IncidentMapAnnotation } from '`@/models/v4/incidentCommand/incidentCommandModels`';As per coding guidelines, "use
import typefor type-only 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/components/command/maps-tabbed-card.tsx` at line 9, Update the import of IncidentCommand, IncidentMap, and IncidentMapAnnotation to use a type-only import declaration, preserving the existing specifiers and module source.Source: Coding guidelines
33-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpose the tab selection state to assistive tech.
Visual
solid/outlinevariance is the only selection cue; screen readers can't tell which pane is active.♿ Proposed change
- <Button size="xs" variant={tab === 'incident' ? 'solid' : 'outline'} onPress={() => setTab('incident')} testID="maps-tab-incident"> + <Button size="xs" variant={tab === 'incident' ? 'solid' : 'outline'} accessibilityRole="tab" accessibilityState={{ selected: tab === 'incident' }} onPress={() => setTab('incident')} testID="maps-tab-incident"> <ButtonText>{t('command.incident_map_section')}</ButtonText> </Button> - <Button size="xs" variant={tab === 'tactical' ? 'solid' : 'outline'} onPress={() => setTab('tactical')} testID="maps-tab-tactical"> + <Button size="xs" variant={tab === 'tactical' ? 'solid' : 'outline'} accessibilityRole="tab" accessibilityState={{ selected: tab === 'tactical' }} onPress={() => setTab('tactical')} testID="maps-tab-tactical">As per coding guidelines, "Follow WCAG mobile accessibility practices, including semantic components, accessible labels".
🤖 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/maps-tabbed-card.tsx` around lines 33 - 40, Update the incident and tactical tab Buttons in the tab selector to expose their selected state through the accessibility API, while preserving the existing visual variants and tab switching behavior. Use the framework’s supported accessibility property for selected tabs on both buttons, with each value derived from the current tab state.Source: Coding guidelines
src/app/command-log/[callId].tsx (3)
68-68: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueGuard refresh against an empty
callId.The mount effect checks
callIdbut the refresh handler doesn't, so a malformed route param fires a request with an empty id.🤖 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/command-log/`[callId].tsx at line 68, Update the refresh handler on the Button to validate callId before invoking fetchTimeline, matching the mount effect’s existing guard; skip the request when callId is empty while preserving refresh behavior for valid IDs.
82-98: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
@shopify/flash-listand hoist the render callbacks.The log is unbounded, and
renderItem,ListEmptyComponent, andcontentContainerStyleare all re-created each render.♻️ Proposed change
-import { FlatList } from 'react-native'; +import { FlashList } from '`@shopify/flash-list`'; +import { StyleSheet } from 'react-native'; @@ - <FlatList - contentContainerStyle={{ padding: 12 }} + <FlashList + contentContainerStyle={styles.listContent} data={filtered} - initialNumToRender={30} + estimatedItemSize={48} keyExtractor={(entry) => entry.CommandLogEntryId} - ListEmptyComponent={ - <Text className="py-8 text-center text-sm text-gray-500 dark:text-gray-400" testID="command-log-empty"> - {search.trim() ? t('command.log_no_results') : t('command.empty_timeline')} - </Text> - } - renderItem={({ item: entry }) => ( - <HStack className="mb-1.5 items-start rounded-lg bg-white px-3 py-2 dark:bg-gray-900" space="sm" testID={`command-log-entry-${entry.CommandLogEntryId}`}> - <Text className="shrink-0 text-xs tabular-nums text-gray-500 dark:text-gray-400">{formatTimestamp(entry.OccurredOn)}</Text> - <Text className="min-w-0 flex-1 text-sm text-gray-900 dark:text-white">{entry.Description}</Text> - </HStack> - )} + ListEmptyComponent={emptyComponent} + renderItem={renderLogEntry} />Define
renderLogEntryas a module-level (oruseCallback) function andstyles.listContentviaStyleSheet.create, withemptyComponentmemoized on the search/empty state.As per coding guidelines, "Use
@shopify/flash-listfor performant lists ... 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/command-log/`[callId].tsx around lines 82 - 98, Replace the FlatList in the command-log screen with `@shopify/flash-list`, and hoist the renderLogEntry callback to module scope or memoize it with useCallback instead of defining it inline. Move contentContainerStyle into a StyleSheet.create styles.listContent definition, and memoize the emptyComponent based on the search/empty state while preserving the existing translated messages and test ID.Source: Coding guidelines
57-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIcon-only buttons need accessible labels.
Neither the search toggle nor the refresh button exposes any text, so assistive tech announces nothing actionable.
♿ Proposed change
<Button size="xs" variant={isSearchOpen ? 'solid' : 'outline'} + accessibilityRole="button" + accessibilityLabel={isSearchOpen ? t('common.close') : t('command.log_search_placeholder')} onPress={() => { @@ - <Button size="xs" variant="outline" onPress={() => fetchTimeline(callId)} testID="command-log-refresh"> + <Button size="xs" variant="outline" accessibilityRole="button" accessibilityLabel={t('common.refresh')} onPress={() => fetchTimeline(callId)} testID="command-log-refresh">As per coding guidelines, "Follow WCAG accessibility practices, including semantic components, accessible labels". Confirm the label keys exist in all locales.
🤖 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/command-log/`[callId].tsx around lines 57 - 70, Add accessible labels to both icon-only buttons in the command-log view: give the search toggle a label that reflects its current action/state and give the refresh button a refresh label. Use the existing localization mechanism and confirm the referenced label keys are defined in every locale.Source: Coding guidelines
src/components/command/resource-details-sheet.tsx (2)
314-314: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a semantic Tailwind token for the badge fallback instead of a hardcoded hex.
#6b7280bypasses the theme and won't adapt to light/dark. Since the color is static, apply it viaclassNameand drop the style entry.♻️ Proposed change
- <Badge style={hex ? { backgroundColor: hex } : styles.badgeFallback} variant="solid" testID={testID}> + <Badge style={hex ? { backgroundColor: hex } : undefined} className={hex ? undefined : 'bg-gray-500 dark:bg-gray-600'} variant="solid" testID={testID}>const styles = StyleSheet.create({ - badgeFallback: { - backgroundColor: '`#6b7280`', - }, laneDot: {As per coding guidelines, "Support light and dark modes, use semantic Tailwind color tokens instead of hardcoded hex values".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/command/resource-details-sheet.tsx` at line 314, Replace the hardcoded backgroundColor fallback with a semantic Tailwind color class applied through className for the badge, and remove the corresponding style entry. Preserve the existing fallback appearance while ensuring it adapts to light and dark themes.Source: Coding guidelines
176-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRender lucide icons directly rather than through the gluestack
Iconwrapper.
resource-cards.tsxin this same PR already renders<Eye className=... size={16} />directly; keeping both patterns is inconsistent.As per coding guidelines, "Use
lucide-react-nativeicons directly, not through the gluestack Icon wrapper".Also applies to: 239-239, 247-247
🤖 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/resource-details-sheet.tsx` at line 176, Update the icon rendering in the resource details sheet, including the instances at the highlighted locations, to render the lucide components directly instead of wrapping them with gluestack’s Icon component. Preserve each icon’s existing component selection, className, and size props, and remove the unnecessary Icon wrapper and import.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 `@scripts/extract-release-notes.sh`:
- Around line 38-40: Update the sed expression in the cleaned_body
transformation to include underscore as a non-boundary character alongside
alphanumerics, preventing replacements within tokens such as PR_STATUS while
preserving standalone PR replacement. Add regression coverage for PR, PRs,
PROXY, and PR_STATUS.
In `@src/app/`(app)/command.tsx:
- Around line 478-495: The handleDeleteLane flow should not call deleteNode when
any moveResourceAssignment or releaseResourceAssignment operation reports a
blocked or failed outcome. Track each disposition result, surface a toast using
the existing notification mechanism when a step fails, and return before
deleting the lane; retain deletion only when all assignments are successfully
processed.
In `@src/app/command-log/`[callId].tsx:
- Line 27: Update the command-log selector around useCommandStore to use a
module-level typed empty-array constant instead of allocating [] in the ??
fallback; import CommandLogEntry from its existing declaration and preserve the
timeline selection behavior.
In `@src/app/resource-map/index.tsx`:
- Line 28: Update the isValid coordinate check in the resource map component to
require finite latitude within [-90, 90] and longitude within [-180, 180], while
preserving the existing non-zero coordinate requirement. Use this validation to
prevent invalid coordinates from reaching Mapbox.Camera or Mapbox.MarkerView.
In `@src/components/calls/call-card.tsx`:
- Line 167: Update the HStack key in the call-card mapping to use the stable
d.Id when available, falling back to d.Name or index only when no ID exists; do
not include index in keys for items with stable IDs.
In `@src/components/command/lane-details-sheet.tsx`:
- Line 152: Update the locale translation resources for
command.delete_lane_resources_message to add the required plural-form keys,
preserving the existing sentence as the base: en needs _one and _other; ar, pl,
and uk need _one, _few, _many, and _other; add matching plural forms for every
other locale using its existing sentence. Keep the resourceCount interpolation
intact so i18next selects the locale-appropriate form.
In `@src/components/command/message-commander-sheet.tsx`:
- Around line 45-56: Update handleSend to wrap the awaited onSend call in
try/catch/finally, resetting isSending in finally so rejection cannot leave the
sheet stuck; preserve the existing success path that closes the sheet when
onSend returns true.
In `@src/components/command/resource-details-sheet.tsx`:
- Around line 150-169: Update openResourceMap to store the 300 ms setTimeout
handle in a ref, and clear that pending timeout during component unmount cleanup
before it can call router.push. Preserve the existing delayed navigation and
parameter construction, and ensure the ref is reset after the callback runs.
In `@src/components/command/timeline-section.tsx`:
- Line 33: Update TimelineSection and TimelineSectionProps so callers rendering
the full log route must provide a callId, or add an equivalent fallback
expansion path when callId is absent. Ensure entries beyond VISIBLE_BATCH remain
accessible instead of limiting no-callId timelines to the initial batch.
In `@src/components/notifications/NotificationInbox.tsx`:
- Around line 38-47: Restrict referenceType to supported actionable values
before constructing the notification payload or triggering navigation, rather
than asserting arbitrary strings into the union. Update the reference resolution
around referenceType, referenceId, and eventCode to implement the documented N/M
event-code mappings, or expose only valid call references. Ensure unsupported
message, status, note, other, and unknown types do not dismiss the inbox without
a destination.
In `@src/stores/command/store.ts`:
- Around line 1651-1658: Update sendMessageToCommander to validate callId as a
positive safe integer without accepting trailing characters such as “42junk”;
use strict numeric parsing and require the resulting ID to be greater than zero
and within Number.MAX_SAFE_INTEGER. Capture the result of sendMessageToCommand
and return true only when result.Data is greater than zero; otherwise return
false.
In `@src/stores/signalr/signalr-store.ts`:
- Line 9: Update the useCommandStore import in signalr-store.ts to use the
configured "`@/stores/command/store`" path alias instead of the relative path,
without changing its usage.
- Around line 182-196: Add tests for the new store logic at
src/stores/signalr/signalr-store.ts:182-196 covering tracked-call refresh,
missing or untracked calls triggering full sync, and refresh failure handling;
at src/stores/units/store.ts:31-54 covering each Promise.allSettled failure path
and preserving cached roster data when fetches reject; and at
src/stores/calls/store.ts:144-152 verifying duplicate dispatches are removed
while distinct dispatches remain.
- Around line 182-195: Update the incidentCommandUpdated handler and its
command-board refresh flow to coalesce refreshes per callId: serialize
refreshBoard(callId) executions, track when an event arrives while a refresh is
in flight, and perform exactly one follow-up refresh for that callId after
completion when dirty. Preserve the existing warning logging and full sync
behavior for unknown or untracked incidents.
In `@src/stores/units/store.ts`:
- Around line 48-52: Update the empty-roster eviction logic in getUnits so
cacheManager.remove('/Units/GetAllUnits') runs only when the fetch fulfills with
an empty roster, not when a rejected request is represented as units = [].
Preserve the existing cache behavior for non-empty responses and failed fetches.
In `@src/translations/ar.json`:
- Around line 1025-1043: Update the Arabic notification count translations used
by confirm_delete_message and delete_success to include the configured v3 plural
suffixes: zero, one, two, few, many, and other, with Arabic text appropriate to
each count range. Keep the existing notification keys and interpolation intact,
and ensure both count-dependent message groups use the complete set.
---
Outside diff comments:
In `@src/components/notifications/NotificationInbox.tsx`:
- Around line 246-275: The confirmBulkDelete flow should use Promise.allSettled
so every selected deletion completes, retain only IDs whose deletions failed,
refresh notifications after settlement, and update selection state to those
failed IDs. Log redacted failure details with relevant count context before
showing the existing error toast, while preserving the success toast and
exit-selection behavior when all deletions succeed.
---
Nitpick comments:
In `@src/app/`(app)/command.tsx:
- Around line 815-827: Update the resource list rendering near the
resource-filter row and its section count so that when resourceFilter is
assigned or unassigned and the filtered results are empty, it renders a short
translated “no matching resources” message instead of only an empty list. Keep
the existing unfiltered rendering and header behavior unchanged for filters with
matches or the all option.
In `@src/app/call/`[id].tsx:
- Line 550: Render the Lucide icons directly instead of through gluestack
wrappers: in src/app/call/[id].tsx lines 550-550, update the button to render
MessageSquareIcon directly; in src/components/command/structure-section.tsx
lines 164-165, render Eye directly instead of using Icon; and in
src/components/command/incident-maps-section.tsx line 78, render Plus directly
inside the button.
In `@src/app/command-log/`[callId].tsx:
- Line 68: Update the refresh handler on the Button to validate callId before
invoking fetchTimeline, matching the mount effect’s existing guard; skip the
request when callId is empty while preserving refresh behavior for valid IDs.
- Around line 82-98: Replace the FlatList in the command-log screen with
`@shopify/flash-list`, and hoist the renderLogEntry callback to module scope or
memoize it with useCallback instead of defining it inline. Move
contentContainerStyle into a StyleSheet.create styles.listContent definition,
and memoize the emptyComponent based on the search/empty state while preserving
the existing translated messages and test ID.
- Around line 57-70: Add accessible labels to both icon-only buttons in the
command-log view: give the search toggle a label that reflects its current
action/state and give the refresh button a refresh label. Use the existing
localization mechanism and confirm the referenced label keys are defined in
every locale.
In `@src/app/resource-map/index.tsx`:
- Line 26: Update the color fallback in the resource-map marker styling to use
the established semantic error/background token instead of the hardcoded `#E53E3E`
value, while preserving validated lane-provided colors. Apply the same semantic
token replacement to the default border styling at the referenced second
location.
- Line 11: Update the Env import in the resource map module to use the
configured `@env` alias instead of '`@/lib/env`', preserving the project’s standard
environment-access contract.
In `@src/components/command/maps-tabbed-card.tsx`:
- Line 9: Update the import of IncidentCommand, IncidentMap, and
IncidentMapAnnotation to use a type-only import declaration, preserving the
existing specifiers and module source.
- Around line 33-40: Update the incident and tactical tab Buttons in the tab
selector to expose their selected state through the accessibility API, while
preserving the existing visual variants and tab switching behavior. Use the
framework’s supported accessibility property for selected tabs on both buttons,
with each value derived from the current tab state.
In `@src/components/command/resource-details-sheet.tsx`:
- Line 314: Replace the hardcoded backgroundColor fallback with a semantic
Tailwind color class applied through className for the badge, and remove the
corresponding style entry. Preserve the existing fallback appearance while
ensuring it adapts to light and dark themes.
- Line 176: Update the icon rendering in the resource details sheet, including
the instances at the highlighted locations, to render the lucide components
directly instead of wrapping them with gluestack’s Icon component. Preserve each
icon’s existing component selection, className, and size props, and remove the
unnecessary Icon wrapper and import.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b10738e8-44a4-4bc3-95ff-352f17de72ed
📒 Files selected for processing (46)
scripts/extract-release-notes.shsrc/api/incidentCommand/incidentCommand.tssrc/app/(app)/__tests__/command.test.tsxsrc/app/(app)/_layout.tsxsrc/app/(app)/command.tsxsrc/app/call/[id].tsxsrc/app/call/__tests__/[id].security.test.tsxsrc/app/call/__tests__/[id].test.tsxsrc/app/command-log/[callId].tsxsrc/app/resource-map/index.tsxsrc/components/calls/call-card.tsxsrc/components/command/__tests__/lane-details-sheet.test.tsxsrc/components/command/__tests__/message-commander-sheet.test.tsxsrc/components/command/__tests__/resource-cards.test.tsxsrc/components/command/__tests__/resource-details-sheet.test.tsxsrc/components/command/__tests__/timeline-section.test.tsxsrc/components/command/incident-map-card.tsxsrc/components/command/incident-maps-section.tsxsrc/components/command/landscape-structure-board.tsxsrc/components/command/lane-details-sheet.tsxsrc/components/command/maps-tabbed-card.tsxsrc/components/command/message-commander-sheet.tsxsrc/components/command/resource-cards.tsxsrc/components/command/resource-details-sheet.tsxsrc/components/command/structure-section.tsxsrc/components/command/timeline-section.tsxsrc/components/notifications/NotificationDetail.tsxsrc/components/notifications/NotificationInbox.tsxsrc/models/v4/device/pushRegistrationInput.tssrc/models/v4/incidentCommand/incidentCommandModels.tssrc/services/__tests__/push-notification.test.tssrc/services/push-notification.tssrc/stores/calls/store.tssrc/stores/command/store.tssrc/stores/signalr/signalr-store.tssrc/stores/units/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.jsonsrc/types/notification.ts
| # Replace standalone occurrences of "PR" with "Release" (word boundaries only, | ||
| # so words like "PROXY" or "PRs" inside other tokens are untouched) | ||
| cleaned_body="$(printf '%s\n' "$cleaned_body" | sed -E 's/(^|[^[:alnum:]])PR([^[:alnum:]]|$)/\1Release\2/g')" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
sed -n '1,90p' scripts/extract-release-notes.sh | cat -n
echo
echo "== sed boundary behavior probe =="
python3 - <<'PY'
import subprocess
pattern = r'<(^|[^\[:alnum:]])PR([^\[:alnum:]]|$)>\1Release\2/g'
# Try sed with equivalent raw POSIX pattern that matches alphanumeric boundary.
inputs = ["PR", "PR_STATUS", "xPRy", "PRx", "xPR", "PROXY", "PRs", "Release PR STATUS", "PR x"]
for p in [
"s/(^|[^[:alnum:]])PR([^[:alnum:]]|$)/\\1Release\\2/g",
"s/(^|[^[:alnum:]_])PR([^[:alnum:]_]|$)/\\1Release\\2/g",
]:
print(f"-- pattern: {p}")
for s in inputs:
out = subprocess.check_output(["sed", "-E", p], text=True, input=f"{s}\n").rstrip()
print(f"{s!r} -> {out!r}")
PYRepository: Resgrid/IC
Length of output: 4668
Treat underscores as part of the token boundary.
[:alnum:] does not include _, so inputs like PR_STATUS or Release PR STATUS are incorrectly rewritten to Release_STATUS / Release Release STATUS. Include the underscore in both surrounding boundary classes.
Suggested fix
- cleaned_body="$(printf '%s\n' "$cleaned_body" | sed -E 's/(^|[^[:alnum:]])PR([^[:alnum:]]|$)/\1Release\2/g')"
+ cleaned_body="$(printf '%s\n' "$cleaned_body" | sed -E 's/(^|[^[:alnum:]_])PR([^[:alnum:]_]|$)/\1Release\2/g')"Add regression coverage for PR, PRs, PROXY, and PR_STATUS.
📝 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.
| # Replace standalone occurrences of "PR" with "Release" (word boundaries only, | |
| # so words like "PROXY" or "PRs" inside other tokens are untouched) | |
| cleaned_body="$(printf '%s\n' "$cleaned_body" | sed -E 's/(^|[^[:alnum:]])PR([^[:alnum:]]|$)/\1Release\2/g')" | |
| # Replace standalone occurrences of "PR" with "Release" (word boundaries only, | |
| # so words like "PROXY" or "PRs" inside other tokens are untouched) | |
| cleaned_body="$(printf '%s\n' "$cleaned_body" | sed -E 's/(^|[^[:alnum:]_])PR([^[:alnum:]_]|$)/\1Release\2/g')" |
🤖 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 `@scripts/extract-release-notes.sh` around lines 38 - 40, Update the sed
expression in the cleaned_body transformation to include underscore as a
non-boundary character alongside alphanumerics, preventing replacements within
tokens such as PR_STATUS while preserving standalone PR replacement. Add
regression coverage for PR, PRs, PROXY, and PR_STATUS.
|
|
||
| return ( | ||
| <HStack key={`${d.Id || index}-${d.Name}`} style={{ backgroundColor: 'rgba(255,255,255,0.25)', borderRadius: 6, overflow: 'hidden' }} className="items-center"> | ||
| <HStack key={`${d.Id || 'noid'}-${d.Name}-${index}`} style={{ backgroundColor: 'rgba(255,255,255,0.25)', borderRadius: 6, overflow: 'hidden' }} className="items-center"> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)src/components/calls/call-card\.tsx$|(^|/)src/app/\(app\)/calls\.tsx$' || true
echo "== outline call-card =="
if [ -f src/components/calls/call-card.tsx ]; then
wc -l src/components/calls/call-card.tsx
ast-grep outline src/components/calls/call-card.tsx --view compact || true
fi
echo "== relevant call-card section =="
if [ -f src/components/calls/call-card.tsx ]; then
sed -n '120,190p' src/components/calls/call-card.tsx | cat -n
fi
echo "== search relevant keys/list data =="
rg -n "key=|dispatch|Id|Name|index" src/components/calls/call-card.tsx src/app/\(app\)/calls.tsx 2>/dev/null || true
echo "== deterministic key behavior probe =="
python3 - <<'PY'
def current_key(d, index):
return f"{d.get('Id') or 'noid'}-{d.get('Name')}-{index}"
def suggested_key(d, index):
return f"{d['Id']}-{d['Name']}" if d.get('Id') is not None else f"noid-{d['Name']}-{index}"
original = [{'Id': 'd1', 'Name': 'A'}, {'Id': 'd2', 'Name': 'B'}]
reordered = [{'Id': 'd2', 'Name': 'B'}, {'Id': 'd1', 'Name': 'A'}]
for label, data in [('original order', original), ('insert reordered after first', [{*original[0]}, original[1]]), ('reorder', reordered)]:
print(label)
prev = {}
for i, d in enumerate(data):
cur = suggested_key(d, i)
print(" ", i, d, "current", current_key(d, i), "old_key", prev.get((d.get('Id'), d.get('Name'))), "current_changes_with_index?", current_key(d, i) != cur)
prev[(d.get('Id'), d.get('Name'))] = current_key(d, i)
PYRepository: Resgrid/IC
Length of output: 11138
Keep stable IDs out of index-based keys.
The key at src/components/calls/call-card.tsx:167 changes if an item moves, even when d.Id and d.Name stay the same, causing unnecessary row remounts. Use index only when the ID is unavailable.
Suggested fix
- <HStack key={`${d.Id || 'noid'}-${d.Name}-${index}`} style={{ backgroundColor: 'rgba(255,255,255,0.25)', borderRadius: 6, overflow: 'hidden' }} className="items-center">
+ <HStack key={d.Id != null ? `${d.Id}-${d.Name}` : `noid-${d.Name}-${index}`} style={{ backgroundColor: 'rgba(255,255,255,0.25)', borderRadius: 6, overflow: 'hidden' }} className="items-center">📝 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.
| <HStack key={`${d.Id || 'noid'}-${d.Name}-${index}`} style={{ backgroundColor: 'rgba(255,255,255,0.25)', borderRadius: 6, overflow: 'hidden' }} className="items-center"> | |
| <HStack key={d.Id != null ? `${d.Id}-${d.Name}` : `noid-${d.Name}-${index}`} style={{ backgroundColor: 'rgba(255,255,255,0.25)', borderRadius: 6, overflow: 'hidden' }} className="items-center"> |
🤖 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/calls/call-card.tsx` at line 167, Update the HStack key in the
call-card mapping to use the stable d.Id when available, falling back to d.Name
or index only when no ID exists; do not include index in keys for items with
stable IDs.
| import { signalRService } from '@/services/signalr.service'; | ||
|
|
||
| import { useCoreStore } from '../app/core-store'; | ||
| import { useCommandStore } from '../command/store'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the configured path alias.
Replace the relative store import with @/stores/command/store. As per coding guidelines, “Use configured path aliases (@/*, @env, @assets/*) 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/stores/signalr/signalr-store.ts` at line 9, Update the useCommandStore
import in signalr-store.ts to use the configured "`@/stores/command/store`" path
alias instead of the relative path, without changing its usage.
Source: Coding guidelines
| signalRService.on('incidentCommandUpdated', (message) => { | ||
| set({ lastUpdateMessage: JSON.stringify(message), lastUpdateTimestamp: Date.now() }); | ||
| const callId = extractCommandCallId(message); | ||
| const commandState = useCommandStore.getState(); | ||
| if (callId && commandState.boards[callId]) { | ||
| commandState.refreshBoard(callId).catch((error) => { | ||
| logger.warn({ message: 'incidentCommandUpdated: failed to refresh board', context: { callId, error } }); | ||
| }); | ||
| } else { | ||
| // Unknown or untracked incident — resync the full bundle. | ||
| commandState.syncFromServer().catch((error) => { | ||
| logger.warn({ message: 'incidentCommandUpdated: failed to sync from server', context: { message, error } }); | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add coverage for the new store behavior.
src/stores/signalr/signalr-store.ts#L182-L196: test tracked-call refresh, missing/untracked-call full sync, and refresh failure handling.src/stores/units/store.ts#L31-L54: test eachPromise.allSettledfailure path and ensure rejected roster fetches do not evict cached data.src/stores/calls/store.ts#L144-L152: test duplicate dispatch removal while preserving distinct dispatches.
As per coding guidelines, “generate tests for new components, services, and logic.”
📍 Affects 3 files
src/stores/signalr/signalr-store.ts#L182-L196(this comment)src/stores/units/store.ts#L31-L54src/stores/calls/store.ts#L144-L152
🤖 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 182 - 196, Add tests for
the new store logic at src/stores/signalr/signalr-store.ts:182-196 covering
tracked-call refresh, missing or untracked calls triggering full sync, and
refresh failure handling; at src/stores/units/store.ts:31-54 covering each
Promise.allSettled failure path and preserving cached roster data when fetches
reject; and at src/stores/calls/store.ts:144-152 verifying duplicate dispatches
are removed while distinct dispatches remain.
Source: Coding guidelines
| "notifications": { | ||
| "additional_info": "معلومات إضافية", | ||
| "confirm_delete_message_one": "هل أنت متأكد من حذف {{count}} إشعار؟ لا يمكن التراجع عن هذا الإجراء.", | ||
| "confirm_delete_message_other": "هل أنت متأكد من حذف {{count}} إشعارات؟ لا يمكن التراجع عن هذا الإجراء.", | ||
| "confirm_delete_title": "تأكيد الحذف", | ||
| "delete_error": "فشل حذف الإشعارات", | ||
| "delete_one_error": "فشل حذف الإشعار", | ||
| "delete_one_success": "تم حذف الإشعار", | ||
| "delete_success_one": "تم حذف {{count}} إشعار", | ||
| "delete_success_other": "تم حذف {{count}} إشعارات", | ||
| "deselect_all": "إلغاء تحديد الكل", | ||
| "empty": "لا توجد تحديثات متاحة", | ||
| "load_error": "تعذر تحميل الإشعارات", | ||
| "notification": "إشعار", | ||
| "select_all": "تحديد الكل", | ||
| "selected_count": "{{count}} محدد", | ||
| "title": "الإشعارات", | ||
| "view_reference": "عرض {{type}}" | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "compatibilityJSON|plural|i18next" package.json srcRepository: Resgrid/IC
Length of output: 18224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== i18n config =="
sed -n '1,80p' src/lib/i18n/index.tsx
echo
echo "== notification translation keys across Arabic and English =="
python3 - <<'PY'
import json
from pathlib import Path
for name in ['en.json','ar.json']:
path = Path('src/translations', name)
data = json.loads(path.read_text())
keys = sorted(data.get('notifications',{}).keys())
print(name, '\n'.join(keys))
PY
echo
echo "== i18next plural behavior probe for v3 v4 if dependencies allow =="
python3 - <<'PY'
try:
from i18next_plural_rules import plural
for count in [0,1,2,3,4,5,6,7,8,9,10,11,12,20,42]:
print(count, plural(count, 'ar', 'v4'), plural(count, 'ar', 'v3'))
except Exception as e:
print("probe unavailable:", type(e).__name__, e)
PY
echo
echo "== translation strings mentioning one/zero/two/few/many =="
rg -n "notifications\\..*_one|notifications\\..*_zero|notification\\..*_one|notification\\..*_zero|notifications\\..*_two|notification\\�*\\._two|notifications\\.*_few|notification\\.*_few|notifications\\.*_many|notification\\.*_many" src/translations/ar.json src/translations/en.jsonRepository: Resgrid/IC
Length of output: 2721
🌐 Web query:
i18next compatibilityJSON v3 plural rules Arabic one other counts 0 2 3-10
💡 Result:
The compatibilityJSON v3 option was a legacy feature used in i18next versions 21 through 23 to maintain backward compatibility with older JSON plural formats [1][2][3]. As of i18next version 24, this option has been removed entirely; i18next now mandates the use of the Intl.PluralRules API [1][2][4]. In the legacy v3 format, languages with multiple plural forms—such as Arabic—used specific numeric suffixes in the JSON keys to map to different plural categories [1][5][6]. Under that older system, Arabic resources typically utilized suffixes like _0, _1, _2, _3, _11, and _100 [7]. In contrast, modern i18next (v4+ format) uses suffixes aligned with the Intl.PluralRules categories (zero, one, two, few, many, other) [8][9][3]. For Arabic, these correspond to the following count mappings [8][9]: { "key_zero": "zero", "key_one": "singular", "key_two": "two", "key_few": "few", "key_many": "many", "key_other": "other" } If you are using a modern version of i18next (v24 or later) and encounter issues with pluralization, you must ensure the Intl.PluralRules API is available in your environment [2][4]. If it is not supported (for example, in certain React Native/Hermes environments), you must include an Intl polyfill (e.g., intl-pluralrules) to restore full plural support [8][2][4]. Without this, i18next will default to a minimal English-style rule supporting only _one and _other [2][9][4].
Citations:
- 1: https://www.i18next.com/misc/json-format
- 2: https://www.i18next.com/how-to/faq
- 3: https://www.i18next.com/misc/migration-guide
- 4: https://github.com/i18next/i18next-gitbook/blob/master/how-to/faq.md
- 5: https://www.i18next.com/misc/json-format.md
- 6: https://github.com/i18next/i18next-gitbook/blob/master/misc/json-format.md
- 7: https://i18next.github.io/i18next/pages/doc_features.html
- 8: https://www.i18next.com/translation-function/plurals
- 9: https://www.i18next.com/translation-function/plurals.md
Keep Arabic count keys in sync with the configured plural mode.
The app runs compatibilityJSON: 'v3', where Arabic still needs count-specific suffixes beyond one/other. Translate the Arabic count variants (0, 1, 2, 3-10, 11-100, 100+) for the notification delete strings, or switch the i18next plural mode and add Intl plural-rule support for Arabic.
🤖 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 1025 - 1043, Update the Arabic
notification count translations used by confirm_delete_message and
delete_success to include the configured v3 plural suffixes: zero, one, two,
few, many, and other, with Arabic text appropriate to each count range. Keep the
existing notification keys and interpolation intact, and ensure both
count-dependent message groups use the complete set.
|
|
||
| /** Sends a free-form message directly to the incident's commander (and optionally deputies) via their notification channels. */ | ||
| export const sendMessageToCommand = async (input: SendMessageToCommandInput) => { | ||
| const response = await createApiEndpoint('/IncidentCommand/SendMessageToCommand').post<SendMessageToCommandResult>({ ...input }); |
There was a problem hiding this comment.
Missing error handling violates Rule [28] by exposing the network POST call without contextual wrapping. Enclose the external request in a try/catch block, log contextual data such as endpoint names and input identifiers, and map the result to an application-level domain error.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/api/incidentCommand/incidentCommand.ts:
Line 68:
Missing error handling violates Rule [28] by exposing the network POST call without contextual wrapping. Enclose the external request in a try/catch block, log contextual data such as endpoint names and input identifiers, and map the result to an application-level domain error.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /** Sends a free-form message directly to the incident's commander (and optionally deputies) via their notification channels. */ | ||
| export const sendMessageToCommand = async (input: SendMessageToCommandInput) => { |
There was a problem hiding this comment.
Missing return type documentation violates Rule [23] for async functions. Update the JSDoc to include @returns {Promise<SendMessageToCommandResult>} and document possible rejection scenarios.
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File src/api/incidentCommand/incidentCommand.ts:
Line 66 to 67:
Missing return type documentation violates Rule [23] for async functions. Update the JSDoc to include `@returns {Promise<SendMessageToCommandResult>}` and document possible rejection scenarios.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <ButtonIcon as={UserCog} /> | ||
| </Button> | ||
| <Button | ||
| onPress={() => setIsMessageSheetOpen(true)} |
There was a problem hiding this comment.
Render performance degradation caused by inline arrow functions and .bind() usage in JSX props across multiple components. Move these function definitions outside the render method to prevent unnecessary function recreation during every render cycle.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/(app)/command.tsx:
Line 608:
Render performance degradation caused by inline arrow functions and `.bind()` usage in JSX props across multiple components. Move these function definitions outside the render method to prevent unnecessary function recreation during every render cycle.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return; | ||
| } | ||
| const laneAssignments = (boards[activeBoardCallId]?.board?.Assignments ?? []).filter((a) => !a.ReleasedOn && a.CommandStructureNodeId === nodeId); | ||
| for (const assignment of laneAssignments) { |
There was a problem hiding this comment.
Network performance bottleneck identified as an N+1 pattern where the loop issues sequential awaited calls to moveResourceAssignment and releaseResourceAssignment. Batch these independent operations using Promise.all or Promise.allSettled before calling deleteNode to reduce round-trips.
Kody rule violation: Detect N+1 style queries and suggest batching
Prompt for LLM
File src/app/(app)/command.tsx:
Line 484:
Network performance bottleneck identified as an N+1 pattern where the loop issues sequential awaited calls to `moveResourceAssignment` and `releaseResourceAssignment`. Batch these independent operations using `Promise.all` or `Promise.allSettled` before calling `deleteNode` to reduce round-trips.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| maps={boardState.board?.Maps ?? []} | ||
| users={users} | ||
| onSave={(nodeId, patch) => updateNodeDetails(boardState.callId, nodeId, patch)} | ||
| resourceCount={(boardState.board?.Assignments ?? []).filter((a) => !a.ReleasedOn && a.CommandStructureNodeId === editLaneNodeId).length} |
There was a problem hiding this comment.
Logic duplication in the active-assignment predicate risks divergence from the identical logic at line 483 in handleDeleteLane. Extract a shared helper like countActiveLaneAssignments(assignments, nodeId) to centralize the filtering criteria for both the handler and the JSX prop.
Kody rule violation: Extract common query logic
Prompt for LLM
File src/app/(app)/command.tsx:
Line 920:
Logic duplication in the active-assignment predicate risks divergence from the identical logic at line 483 in `handleDeleteLane`. Extract a shared helper like `countActiveLaneAssignments(assignments, nodeId)` to centralize the filtering criteria for both the handler and the JSX prop.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!query) { | ||
| return entries; | ||
| } | ||
| return entries.filter((entry) => entry.Description.toLowerCase().includes(query)); |
There was a problem hiding this comment.
Null pointer dereference crashes the screen when the filter callback invokes .toLowerCase() on a potentially undefined entry.Description. Apply optional chaining via entry.Description?.toLowerCase().includes(query) ?? false to guard against nullish values.
Kody rule violation: Add null checks before accessing properties
Prompt for LLM
File src/app/command-log/[callId].tsx:
Line 44:
Null pointer dereference crashes the screen when the filter callback invokes `.toLowerCase()` on a potentially undefined `entry.Description`. Apply optional chaining via `entry.Description?.toLowerCase().includes(query) ?? false` to guard against nullish values.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }; | ||
|
|
||
| /** Fullscreen incident log — every server-logged entry with text search. */ | ||
| export default function CommandLogScreen() { |
There was a problem hiding this comment.
Default export reduces refactor safety and IDE auto-import reliability compared to named exports. Change to a named export like export function CommandLogScreen() unless Expo Router strictly requires a default export for route screens.
Kody rule violation: Avoid default exports
Prompt for LLM
File src/app/command-log/[callId].tsx:
Line 22:
Default export reduces refactor safety and IDE auto-import reliability compared to named exports. Change to a named export like `export function CommandLogScreen()` unless Expo Router strictly requires a default export for route screens.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| return ( | ||
| <HStack key={`${d.Id || index}-${d.Name}`} style={{ backgroundColor: 'rgba(255,255,255,0.25)', borderRadius: 6, overflow: 'hidden' }} className="items-center"> | ||
| <HStack key={`${d.Id || 'noid'}-${d.Name}-${index}`} style={{ backgroundColor: 'rgba(255,255,255,0.25)', borderRadius: 6, overflow: 'hidden' }} className="items-center"> |
There was a problem hiding this comment.
React reconciliation bug triggered by embedding the array index alongside d.Id and d.Name in list keys. Derive a stable unique key from a genuine identifier without mixing in the positional array index.
Kody rule violation: Avoid array indexes as keys in React lists
Prompt for LLM
File src/components/calls/call-card.tsx:
Line 167:
React reconciliation bug triggered by embedding the array `index` alongside `d.Id` and `d.Name` in list keys. Derive a stable unique key from a genuine identifier without mixing in the positional array index.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <Button size="xs" variant="outline" onPress={() => setIsAddOpen(true)} testID="incident-maps-add"> | ||
| <ButtonIcon as={Plus} /> | ||
| <ButtonText>{t('command.add')}</ButtonText> | ||
| </Button> |
There was a problem hiding this comment.
Code duplication violates Rule [15] by repeating the add-button JSX block in both the embedded and non-embedded branches. Extract the block into a named utility component like <IncidentMapAddButton onPress=... /> to prevent synchronization drift.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File src/components/command/incident-maps-section.tsx:
Line 89 to 92:
Code duplication violates Rule [15] by repeating the add-button JSX block in both the `embedded` and non-embedded branches. Extract the block into a named utility component like `<IncidentMapAddButton onPress=... />` to prevent synchronization drift.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /** Active resources sitting in this lane — drives the delete confirmation options. */ | ||
| resourceCount?: number; | ||
| /** Delete the lane; disposition decides what happens to its resources first. */ | ||
| onDelete?: (commandStructureNodeId: string, disposition: 'pool' | 'release') => void; |
There was a problem hiding this comment.
Type drift risk identified as the 'pool' | 'release' union type duplicates string literals used at runtime across multiple lines. Declare a const tuple like const LANE_DISPOSITIONS = ['pool', 'release'] as const; and derive the type from it to ensure single-source-of-truth maintenance.
Kody rule violation: Derive TypeScript types from validation schemas
Prompt for LLM
File src/components/command/lane-details-sheet.tsx:
Line 46:
Type drift risk identified as the `'pool' | 'release'` union type duplicates string literals used at runtime across multiple lines. Declare a const tuple like `const LANE_DISPOSITIONS = ['pool', 'release'] as const;` and derive the type from it to ensure single-source-of-truth maintenance.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return ( | ||
| <Box className="rounded-xl bg-white p-4 shadow-sm dark:bg-gray-800" testID="command-maps-pane"> | ||
| <HStack space="sm" className="mb-3"> | ||
| <Button size="xs" variant={tab === 'incident' ? 'solid' : 'outline'} onPress={() => setTab('incident')} testID="maps-tab-incident"> |
There was a problem hiding this comment.
Maintenance risk identified from raw string literals like 'incident' replacing a strongly typed MapsTab union. Derive the type from a const tuple and reference named constants to eliminate magic strings.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/components/command/maps-tabbed-card.tsx:
Line 34:
Maintenance risk identified from raw string literals like `'incident'` replacing a strongly typed `MapsTab` union. Derive the type from a const tuple and reference named constants to eliminate magic strings.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return; | ||
| } | ||
| setIsSending(true); | ||
| const ok = await onSend(title.trim() || null, trimmedBody, includeDeputies); |
There was a problem hiding this comment.
Unhandled promise rejection from the awaited onSend(...) call on line 51 leaves isSending at true and the UI in a perpetual loading state. Wrap the await in a try/catch block, reset isSending in a finally block, and log the error context.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/components/command/message-commander-sheet.tsx:
Line 51:
Unhandled promise rejection from the awaited `onSend(...)` call on line 51 leaves `isSending` at `true` and the UI in a perpetual loading state. Wrap the await in a try/catch block, reset `isSending` in a `finally` block, and log the error context.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }, [body, title, includeDeputies, isSending, onSend, handleClose]); | ||
|
|
||
| return ( | ||
| <CustomBottomSheet isOpen={isOpen} onClose={handleClose} snapPoints={[75]}> |
There was a problem hiding this comment.
Magic number obfuscation occurs in the snapPoints={[75]} prop, making maintenance error-prone. Extract the viewport height percentage to a named constant like const SHEET_SNAP_POINT_PERCENT = 75;.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/components/command/message-commander-sheet.tsx:
Line 59:
Magic number obfuscation occurs in the `snapPoints={[75]}` prop, making maintenance error-prone. Extract the viewport height percentage to a named constant like `const SHEET_SNAP_POINT_PERCENT = 75;`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <Button size="xs" variant="outline" onPress={() => setVisibleCount((c) => c + VISIBLE_BATCH)} testID="command-timeline-more"> | ||
| <ButtonText>{t('command.show_more')}</ButtonText> | ||
| {callId && entries.length > VISIBLE_BATCH ? ( | ||
| <Button size="xs" variant="outline" onPress={() => router.push(`/command-log/${callId}` as never)} testID="command-timeline-view-log"> |
There was a problem hiding this comment.
Type safety bypass occurs because the route path uses the unsafe as never assertion to bypass expo-router's typed-routes checking. Register the route correctly or use a more specific assertion like as \/command-log/${string}`` to preserve compile-time route validation.
Kody rule violation: Use safe type casting with as operator
Prompt for LLM
File src/components/command/timeline-section.tsx:
Line 58:
Type safety bypass occurs because the route path uses the unsafe `as never` assertion to bypass expo-router's typed-routes checking. Register the route correctly or use a more specific assertion like `as \`/command-log/${string}\`` to preserve compile-time route validation.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| @@ -224,40 +251,41 @@ export const NotificationInbox = ({ isOpen, onClose }: NotificationInboxProps) = | |||
| const deletePromises = Array.from(selectedNotificationIds).map((id) => deleteMessage(id)); | |||
| await Promise.all(deletePromises); | |||
There was a problem hiding this comment.
Partial-failure mishandling occurs because Promise.all short-circuits during a bulk-delete operation, silently abandoning remaining deletions. Use Promise.allSettled to process all deletions and report per-item successes and failures to the user.
Kody rule violation: Use Promise.allSettled for batch operations with partial failures
Prompt for LLM
File src/components/notifications/NotificationInbox.tsx:
Line 252:
Partial-failure mishandling occurs because `Promise.all` short-circuits during a bulk-delete operation, silently abandoning remaining deletions. Use `Promise.allSettled` to process all deletions and report per-item successes and failures to the user.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Platform: Platform.OS === 'ios' ? 1 : 2, | ||
| DeviceUuid: getDeviceUuid() || '', | ||
| Prefix: departmentCode, | ||
| Source: 'IC', |
There was a problem hiding this comment.
Maintenance fragility from the inline string literal 'IC' acting as a source-type identifier across multiple call sites. Define a centralized enum or constant object like export const NOTIFICATION_SOURCE = { IC: 'IC', Responder: 'Responder' } as const; and reference it to prevent duplication.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/services/push-notification.ts:
Line 370:
Maintenance fragility from the inline string literal `'IC'` acting as a source-type identifier across multiple call sites. Define a centralized enum or constant object like `export const NOTIFICATION_SOURCE = { IC: 'IC', Responder: 'Responder' } as const;` and reference it to prevent duplication.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| signalRService.on('incidentCommandUpdated', (message) => { | ||
| set({ lastUpdateMessage: JSON.stringify(message), lastUpdateTimestamp: Date.now() }); | ||
| const callId = extractCommandCallId(message); | ||
| const commandState = useCommandStore.getState(); | ||
| if (callId && commandState.boards[callId]) { | ||
| commandState.refreshBoard(callId).catch((error) => { | ||
| logger.warn({ message: 'incidentCommandUpdated: failed to refresh board', context: { callId, error } }); | ||
| }); | ||
| } else { | ||
| // Unknown or untracked incident — resync the full bundle. | ||
| commandState.syncFromServer().catch((error) => { | ||
| logger.warn({ message: 'incidentCommandUpdated: failed to sync from server', context: { message, error } }); | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Unthrottled network traffic and redundant reprocessing occur because the incidentCommandUpdated handler invokes syncFromServer() for every department-wide event. Debounce the syncFromServer() call or limit its execution to when the user actively opens or lists incidents.
// Use a trailing debounce so a burst of department-wide incidentCommandUpdated
// events collapses into a single full sync instead of one per event.
let incidentCommandResyncTimer: ReturnType<typeof setTimeout> | null = null;
const debouncedFullSync = () => {
if (incidentCommandResyncTimer) {
clearTimeout(incidentCommandResyncTimer);
}
incidentCommandResyncTimer = setTimeout(() => {
incidentCommandResyncTimer = null;
useCommandStore.getState().syncFromServer().catch((error) => {
logger.warn({ message: 'incidentCommandUpdated: failed to sync from server', context: { error } });
});
}, 2000);
};
signalRService.on('incidentCommandUpdated', (message) => {
set({ lastUpdateMessage: JSON.stringify(message), lastUpdateTimestamp: Date.now() });
const callId = extractCommandCallId(message);
const commandState = useCommandStore.getState();
if (callId && commandState.boards[callId]) {
commandState.refreshBoard(callId).catch((error) => {
logger.warn({ message: 'incidentCommandUpdated: failed to refresh board', context: { callId, error } });
});
} else {
debouncedFullSync();
}
});Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 182 to 196:
Unthrottled network traffic and redundant reprocessing occur because the `incidentCommandUpdated` handler invokes `syncFromServer()` for every department-wide event. Debounce the `syncFromServer()` call or limit its execution to when the user actively opens or lists incidents.
Suggested Code:
// Use a trailing debounce so a burst of department-wide incidentCommandUpdated
// events collapses into a single full sync instead of one per event.
let incidentCommandResyncTimer: ReturnType<typeof setTimeout> | null = null;
const debouncedFullSync = () => {
if (incidentCommandResyncTimer) {
clearTimeout(incidentCommandResyncTimer);
}
incidentCommandResyncTimer = setTimeout(() => {
incidentCommandResyncTimer = null;
useCommandStore.getState().syncFromServer().catch((error) => {
logger.warn({ message: 'incidentCommandUpdated: failed to sync from server', context: { error } });
});
}, 2000);
};
signalRService.on('incidentCommandUpdated', (message) => {
set({ lastUpdateMessage: JSON.stringify(message), lastUpdateTimestamp: Date.now() });
const callId = extractCommandCallId(message);
const commandState = useCommandStore.getState();
if (callId && commandState.boards[callId]) {
commandState.refreshBoard(callId).catch((error) => {
logger.warn({ message: 'incidentCommandUpdated: failed to refresh board', context: { callId, error } });
});
} else {
debouncedFullSync();
}
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } else { | ||
| // Unknown or untracked incident — resync the full bundle. | ||
| commandState.syncFromServer().catch((error) => { | ||
| logger.warn({ message: 'incidentCommandUpdated: failed to sync from server', context: { message, error } }); |
There was a problem hiding this comment.
GDPR data minimization violation identified by logging the raw SignalR message payload, which may contain unredacted PII, directly into the warn context. Log only the extracted callId or pass the full payload through a PII-redaction utility before logging.
Kody rule violation: Redact PII in logs and metrics by default
Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 193:
GDPR data minimization violation identified by logging the raw SignalR `message` payload, which may contain unredacted PII, directly into the warn context. Log only the extracted `callId` or pass the full payload through a PII-redaction utility before logging.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/(app)/command.tsx (2)
812-818: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRetry the personnel roster as well as the unit roster.
AddResourceSheetreceivespersonnel={users}, but this handler only retriesfetchUnits(). If the initial user fetch failed, opening the resource picker still leaves personnel unavailable. Add ausers.length === 0retry forfetchUsers().Proposed fix
if (units.length === 0) { fetchUnits(); } + if (users.length === 0) { + fetchUsers(); + } setIsResourceSheetOpen(true);🤖 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)/command.tsx around lines 812 - 818, Update the AddResourceSheet onAdd handler in command.tsx to also call fetchUsers() when users.length === 0, alongside the existing fetchUnits() retry, before opening the resource sheet.
834-858: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve lane context when opening resource details.
Both resource-card handlers always set
context: 'pool', even whenCommandStructureNodeIdis populated. An assigned resource therefore shows “Release from incident” and can be released entirely instead of being moved back to the pool.Proposed fix
- onView={() => setViewResource({ assignment, context: 'pool' })} + onView={() => setViewResource({ assignment, context: assignment.CommandStructureNodeId ? 'lane' : 'pool' })}Apply this to both the unit and personnel cards.
🤖 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)/command.tsx around lines 834 - 858, Update both resource-card onView handlers in the filteredDeptAssignments map to derive context from assignment.CommandStructureNodeId instead of always using 'pool'. Preserve 'pool' for unassigned resources and use the incident/lane context when the node ID is populated, for both UnitResourceCard and PersonnelResourceCard.
♻️ Duplicate comments (1)
src/app/(app)/command.tsx (1)
495-498:⚠️ Potential issue | 🟠 MajorStill abort lane deletion when a release disposition fails.
The
poolbranch checks outcomes, but thereleasebranch discardsPromise.all(...)results and proceeds todeleteNode. Apply the same failure check and user feedback before deleting the lane.🤖 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)/command.tsx around lines 495 - 498, Update the release branch in the lane-deletion flow around releaseResourceAssignment and deleteNode to inspect the Promise.all results, matching the pool branch’s failure handling. If any release disposition fails, provide the same user feedback and return before calling deleteNode; only delete the lane when all assignments release successfully.
🤖 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)/command.tsx:
- Around line 607-612: Add localized accessibilityLabel values using t() to the
icon-only buttons invoking handleOpenCommandDetails and handleOpenTransfer.
Match the existing localized labeling pattern used by the message and
end-command buttons, with labels that clearly identify editing details and
transferring the command.
In `@src/stores/signalr/signalr-store.ts`:
- Around line 79-91: Update debouncedFullSync and its surrounding state to
serialize syncFromServer calls: track whether a full sync is in flight and
whether another event arrived while it was pending, then run one follow-up sync
after completion when dirty. Preserve the existing debounce behavior and
logger.warn handling while ensuring concurrent requests cannot overwrite newer
state with an older response.
---
Outside diff comments:
In `@src/app/`(app)/command.tsx:
- Around line 812-818: Update the AddResourceSheet onAdd handler in command.tsx
to also call fetchUsers() when users.length === 0, alongside the existing
fetchUnits() retry, before opening the resource sheet.
- Around line 834-858: Update both resource-card onView handlers in the
filteredDeptAssignments map to derive context from
assignment.CommandStructureNodeId instead of always using 'pool'. Preserve
'pool' for unassigned resources and use the incident/lane context when the node
ID is populated, for both UnitResourceCard and PersonnelResourceCard.
---
Duplicate comments:
In `@src/app/`(app)/command.tsx:
- Around line 495-498: Update the release branch in the lane-deletion flow
around releaseResourceAssignment and deleteNode to inspect the Promise.all
results, matching the pool branch’s failure handling. If any release disposition
fails, provide the same user feedback and return before calling deleteNode; only
delete the lane when all assignments release successfully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e32a968f-4f6e-49f7-9317-895acbed0dc2
📒 Files selected for processing (18)
src/app/(app)/command.tsxsrc/app/command-log/[callId].tsxsrc/app/resource-map/index.tsxsrc/components/command/message-commander-sheet.tsxsrc/components/command/resource-details-sheet.tsxsrc/components/notifications/NotificationInbox.tsxsrc/stores/command/store.tssrc/stores/signalr/signalr-store.tssrc/stores/units/store.tssrc/translations/ar.jsonsrc/translations/de.jsonsrc/translations/en.jsonsrc/translations/es.jsonsrc/translations/fr.jsonsrc/translations/it.jsonsrc/translations/pl.jsonsrc/translations/sv.jsonsrc/translations/uk.json
🚧 Files skipped from review as they are similar to previous changes (16)
- src/app/resource-map/index.tsx
- src/translations/uk.json
- src/translations/pl.json
- src/components/command/resource-details-sheet.tsx
- src/translations/sv.json
- src/app/command-log/[callId].tsx
- src/stores/units/store.ts
- src/stores/command/store.ts
- src/components/command/message-commander-sheet.tsx
- src/translations/es.json
- src/translations/en.json
- src/translations/fr.json
- src/translations/it.json
- src/translations/ar.json
- src/translations/de.json
- src/components/notifications/NotificationInbox.tsx
| } | ||
| const laneAssignments = (boards[activeBoardCallId]?.board?.Assignments ?? []).filter((a) => !a.ReleasedOn && a.CommandStructureNodeId === nodeId); | ||
| if (disposition === 'pool') { | ||
| const outcomes = await Promise.all(laneAssignments.map((assignment) => moveResourceAssignment(activeBoardCallId, assignment.ResourceAssignmentId, ''))); |
There was a problem hiding this comment.
Unhandled promise rejection risk: the awaited Promise.all at line 496 has no surrounding try/catch or .catch handler, so if any moveResourceAssignment rejects, the error propagates uncaught through the callback. Wrap the await in try/catch, log structured context (callId, nodeId, disposition), and surface a user-facing notification.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/app/(app)/command.tsx:
Line 490:
Unhandled promise rejection risk: the awaited Promise.all at line 496 has no surrounding try/catch or .catch handler, so if any moveResourceAssignment rejects, the error propagates uncaught through the callback. Wrap the await in try/catch, log structured context (callId, nodeId, disposition), and surface a user-facing notification.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| const laneAssignments = (boards[activeBoardCallId]?.board?.Assignments ?? []).filter((a) => !a.ReleasedOn && a.CommandStructureNodeId === nodeId); | ||
| if (disposition === 'pool') { | ||
| const outcomes = await Promise.all(laneAssignments.map((assignment) => moveResourceAssignment(activeBoardCallId, assignment.ResourceAssignmentId, ''))); |
There was a problem hiding this comment.
moveResourceAssignment is an external service call invoked without a try/catch boundary, so network or service-layer failures during batch moves bubble up without contextual error mapping. Wrap the batch in try/catch with structured context (activeBoardCallId, nodeId, count) and map the exception to an application-level error or user notification.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/app/(app)/command.tsx:
Line 490:
moveResourceAssignment is an external service call invoked without a try/catch boundary, so network or service-layer failures during batch moves bubble up without contextual error mapping. Wrap the batch in try/catch with structured context (activeBoardCallId, nodeId, count) and map the exception to an application-level error or user notification.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| const laneAssignments = (boards[activeBoardCallId]?.board?.Assignments ?? []).filter((a) => !a.ReleasedOn && a.CommandStructureNodeId === nodeId); | ||
| if (disposition === 'pool') { | ||
| const outcomes = await Promise.all(laneAssignments.map((assignment) => moveResourceAssignment(activeBoardCallId, assignment.ResourceAssignmentId, ''))); |
There was a problem hiding this comment.
Promise.all short-circuits on the first rejection, causing remaining assignments in the batch of moveResourceAssignment operations to be unhandled and losing per-item failure context. Replace with Promise.allSettled and inspect each settled status individually rather than relying on a single rejection.
Kody rule violation: Use Promise.allSettled for batch operations with partial failures
Prompt for LLM
File src/app/(app)/command.tsx:
Line 490:
Promise.all short-circuits on the first rejection, causing remaining assignments in the batch of moveResourceAssignment operations to be unhandled and losing per-item failure context. Replace with Promise.allSettled and inspect each settled status individually rather than relying on a single rejection.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const rawColor = first(params.color); | ||
| const color = rawColor && /^#[0-9a-fA-F]{3,8}$/.test(rawColor) ? rawColor : '#E53E3E'; | ||
|
|
||
| const isValid = Number.isFinite(latitude) && Number.isFinite(longitude) && latitude >= -90 && latitude <= 90 && longitude >= -180 && longitude <= 180 && (latitude !== 0 || longitude !== 0); |
There was a problem hiding this comment.
Inline magic numbers representing geographic coordinate bounds (-90, 90, -180, 180) reduce readability and make bounds changes error-prone. Extract named constants (MIN_LATITUDE, MAX_LATITUDE, MIN_LONGITUDE, MAX_LONGITUDE) at module level and reference them in the validation expression.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/app/resource-map/index.tsx:
Line 28:
Inline magic numbers representing geographic coordinate bounds (-90, 90, -180, 180) reduce readability and make bounds changes error-prone. Extract named constants (MIN_LATITUDE, MAX_LATITUDE, MIN_LONGITUDE, MAX_LONGITUDE) at module level and reference them in the validation expression.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
2 participants
This PR introduces several major enhancements to the Incident Command board, notifications system, and overall app resilience.
Command Board Enhancements
Resource Details Sheet — Replaces direct remove/release buttons on resource cards with a comprehensive details inspector showing unit/personnel roster info, live status, staffing, assigned roles, and last known GPS coordinates. From the sheet, the IC can remove a resource from its lane, release it from the incident entirely, or open a fullscreen map of the resource's position. A new
/resource-mapscreen renders a single resource marker on a fullscreen map.Resource List Filtering — Added All / Unassigned / Assigned filter chips above the resources list to quickly narrow the view.
Lane Delete with Resource Disposition — Deleting a lane now prompts for what to do with its assigned resources: move them back to the unassigned pool or release them from the incident. The delete action is accessed from within the lane details editor.
Maps Tabbed Card — Consolidated the incident map and named tactical maps into a single tabbed pane, reducing vertical scroll space on the command board.
Message Commander — New feature allowing users to send free-form messages directly to the incident commander (and optionally deputy commanders) via their notification channels. Accessible from both the command board and call detail screen.
Fullscreen Command Log — The timeline section now links to a dedicated
/command-log/[callId]screen with full-text search over all server-logged incident entries, replacing the inline "show more" pagination.Real-Time Updates
Added SignalR handling for
incidentCommandUpdatedevents, automatically refreshing the affected command board when the server reports changes (e.g., from another user's action).Notifications
NotificationPayloadtype; Novu inbox items are now properly mapped to the app's payload shape with reference-type inference from event codes{DepartmentCode}_IC_User_{userId}and addedSource: 'IC'to push registration, keeping the IC app's notification inbox separate from the Responder appResilience & Bug Fixes
Promise.allSettledso a failing status endpoint no longer breaks the unit roster; stale empty rosters are cleared from cache to force a server re-fetchOther
Summary by CodeRabbit
New Features
Bug Fixes
Localization