Conversation
|
Warning Review limit reached
Next review available in: 19 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 ignored due to path filters (12)
📒 Files selected for processing (90)
✨ 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 |
Code Review Could Not Complete
|
| Options | Enabled |
|---|---|
| Bug | ✅ |
| Performance | ✅ |
| Security | ✅ |
| Business Logic | ✅ |
| <View> | ||
| <Button | ||
| title="Load Feature" | ||
| onPress={() => setShowFeature(true)} |
There was a problem hiding this comment.
Performance regression caused by inline arrow functions in JSX props creating new functions on every render. Move the function definitions outside the render method.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File .agents/skills/react-native-best-practices/references/bundle-code-splitting.md:
Line 150:
Performance regression caused by inline arrow functions in JSX props creating new functions on every render. Move the function definitions outside the render method.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }); | ||
|
|
||
| ScriptManager.shared.on('error', (scriptId, error) => { | ||
| console.error(`Failed: ${scriptId}`, error); |
There was a problem hiding this comment.
Unstructured error logging in console.error prevents searchable, parseable logs. Rule [3] requires using a structured logger with the operation name and relevant identifiers, such as logger.error('chunk_load_failed', { scriptId, error }).
Kody rule violation: Include error context in structured logs
Prompt for LLM
File .agents/skills/react-native-best-practices/references/bundle-code-splitting.md:
Line 231:
Unstructured error logging in `console.error` prevents searchable, parseable logs. Rule [3] requires using a structured logger with the operation name and relevant identifiers, such as `logger.error('chunk_load_failed', { scriptId, error })`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| ```tsx | ||
| // Check if chunk loaded correctly | ||
| ScriptManager.shared.on('loading', (scriptId) => { |
There was a problem hiding this comment.
Event listener leak on ScriptManager.shared.on('loading') lacks a deterministic cleanup path. Rule [4] requires calling .off() or removeEventListener during component teardown to prevent dangling handlers.
Kody rule violation: Provide error handlers to subscription/listener APIs
Prompt for LLM
File .agents/skills/react-native-best-practices/references/bundle-code-splitting.md:
Line 222:
Event listener leak on `ScriptManager.shared.on('loading')` lacks a deterministic cleanup path. Rule [4] requires calling `.off()` or `removeEventListener` during component teardown to prevent dangling handlers.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| ```tsx | ||
| // Check if chunk loaded correctly | ||
| ScriptManager.shared.on('loading', (scriptId) => { |
There was a problem hiding this comment.
Memory leak caused by an unremoved event listener on ScriptManager. Rule [57] requires capturing the handler reference and calling ScriptManager.shared.off('loading', handler) during component teardown to prevent resource consumption.
Kody rule violation: Proper memory management in event listeners
Prompt for LLM
File .agents/skills/react-native-best-practices/references/bundle-code-splitting.md:
Line 222:
Memory leak caused by an unremoved event listener on `ScriptManager`. Rule [57] requires capturing the handler reference and calling `ScriptManager.shared.off('loading', handler)` during component teardown to prevent resource consumption.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const TodoProvider = ({ children }) => { | ||
| const [state, setState] = useState({ filter: 'all', todos: [] }); | ||
| return ( | ||
| <TodoContext.Provider value={{ state, setState }}> |
There was a problem hiding this comment.
Unstable context value from the inline object literal {{ state, setState }} triggers unnecessary re-renders of all consumers on every render. Rule [55] requires wrapping the value with useMemo, such as const value = useMemo(() => ({ state, setState }), [state]).
Kody rule violation: React Context values should have stable identities
Prompt for LLM
File .agents/skills/react-native-best-practices/references/js-atomic-state.md:
Line 189:
Unstable context value from the inline object literal `{{ state, setState }}` triggers unnecessary re-renders of all consumers on every render. Rule [55] requires wrapping the value with `useMemo`, such as `const value = useMemo(() => ({ state, setState }), [state])`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const filter = get(filterAtom); | ||
| const todos = get(todosAtom); | ||
|
|
||
| if (filter === 'active') return todos.filter(t => !t.completed); |
There was a problem hiding this comment.
Magic strings used in comparisons risk silent failures from typos. Rule [9] requires defining a constants tuple, such as const FILTERS = ['all', 'active', 'completed'] as const; type Filter = typeof FILTERS[number];, to replace finite-set string literals.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File .agents/skills/react-native-best-practices/references/js-atomic-state.md:
Line 84:
Magic strings used in comparisons risk silent failures from typos. Rule [9] requires defining a constants tuple, such as `const FILTERS = ['all', 'active', 'completed'] as const; type Filter = typeof FILTERS[number];`, to replace finite-set string literals.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const BadList = ({ items }) => ( | ||
| <ScrollView> | ||
| {items.map((item, index) => ( | ||
| <View key={index}> |
There was a problem hiding this comment.
Anti-pattern identified using array index as a React key (<View key={index}>), normalizing a practice the pitfalls section (line 237) warns against. Replace it with a stable unique identifier like key={item.id}.
Kody rule violation: Avoid array indexes as keys in React lists
Prompt for LLM
File .agents/skills/react-native-best-practices/references/js-lists-flatlist-flashlist.md:
Line 65:
Anti-pattern identified using array index as a React key (`<View key={index}>`), normalizing a practice the pitfalls section (line 237) warns against. Replace it with a stable unique identifier like `key={item.id}`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ```tsx | ||
| import performance from 'react-native-performance'; | ||
|
|
||
| export default function HomeScreen() { |
There was a problem hiding this comment.
Default export for HomeScreen violates Rule [75], which requires named exports for clarity, maintainability, and refactoring safety. Change to export function HomeScreen() and update imports to import { HomeScreen } from './HomeScreen'.
Kody rule violation: Avoid default exports
Prompt for LLM
File .agents/skills/react-native-best-practices/references/native-measure-tti.md:
Line 173:
Default export for `HomeScreen` violates Rule [75], which requires named exports for clarity, maintainability, and refactoring safety. Change to `export function HomeScreen()` and update imports to `import { HomeScreen } from './HomeScreen'`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| let result = self.compute() | ||
| resolve(result) |
There was a problem hiding this comment.
Unhandled rejection risk where self.compute() lacks error handling, causing the JavaScript caller to hang indefinitely if it throws. Invoke the existing reject: RCTPromiseRejectBlock parameter (line 151) within a do/catch block to ensure the promise settles cleanly.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File .agents/skills/react-native-best-practices/references/native-threading-model.md:
Line 155 to 156:
Unhandled rejection risk where `self.compute()` lacks error handling, causing the JavaScript caller to hang indefinitely if it throws. Invoke the existing `reject: RCTPromiseRejectBlock` parameter (line 151) within a `do/catch` block to ensure the promise settles cleanly.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const getEndpoint = (path: string): MockEndpoint => { | ||
| const callIndex = mockCreateApiEndpoint.mock.calls.findIndex(([endpoint]) => endpoint === path); | ||
| return mockCreateApiEndpoint.mock.results[callIndex].value as unknown as MockEndpoint; |
There was a problem hiding this comment.
Opaque TypeError risk when dereferencing mock.results[callIndex].value without a null guard. findIndex (line 24) returns -1 when the path is not found, making the element undefined; add a bounds check or use optional chaining like mock.results[callIndex]?.value.
Kody rule violation: Add null checks before accessing properties
Prompt for LLM
File src/api/check-in-timers/__tests__/check-in-timers.test.ts:
Line 25:
Opaque `TypeError` risk when dereferencing `mock.results[callIndex].value` without a null guard. `findIndex` (line 24) returns -1 when the path is not found, making the element `undefined`; add a bounds check or use optional chaining like `mock.results[callIndex]?.value`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const getEndpoint = (path: string): MockEndpoint => { | ||
| const callIndex = mockCreateApiEndpoint.mock.calls.findIndex(([endpoint]) => endpoint === path); | ||
| return mockCreateApiEndpoint.mock.results[callIndex].value as unknown as MockEndpoint; |
There was a problem hiding this comment.
Null pointer dereference risk on mock.results[callIndex].value when callIndex is -1 from findIndex. Guard the access with a null check or throw an explicit Error before accessing .value.
Kody rule violation: Add null checks to prevent NullReferenceException
Prompt for LLM
File src/api/check-in-timers/__tests__/check-in-timers.test.ts:
Line 25:
Null pointer dereference risk on `mock.results[callIndex].value` when `callIndex` is -1 from `findIndex`. Guard the access with a null check or throw an explicit `Error` before accessing `.value`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const getEndpoint = (path: string): MockEndpoint => { | ||
| const callIndex = mockCreateApiEndpoint.mock.calls.findIndex(([endpoint]) => endpoint === path); | ||
| return mockCreateApiEndpoint.mock.results[callIndex].value as unknown as MockEndpoint; |
There was a problem hiding this comment.
Unvalidated array bounds when using callIndex (from findIndex) to index mock.results, yielding undefined when the endpoint path is not found. Validate the index before use, e.g., if (callIndex < 0 || callIndex >= mock.results.length) throw new Error(...).
Kody rule violation: Check query results before accessing indices
Prompt for LLM
File src/api/check-in-timers/__tests__/check-in-timers.test.ts:
Line 25:
Unvalidated array bounds when using `callIndex` (from `findIndex`) to index `mock.results`, yielding `undefined` when the endpoint path is not found. Validate the index before use, e.g., `if (callIndex < 0 || callIndex >= mock.results.length) throw new Error(...)`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| CallId: callId, | ||
| Enabled: enabled, | ||
| export const getCallPersonnelCheckInStatuses = async (callId: number) => { | ||
| const response = await getCallPersonnelCheckInStatusesApi.get<CallPersonnelCheckInStatusResult>({ |
There was a problem hiding this comment.
Unhandled external HTTP call failure in getCallPersonnelCheckInStatusesApi.get violates Rule 28. Wrap the call in a try/catch block, include the callId and endpoint name in the error context, and map it to an application-level error.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/api/check-in-timers/check-in-timers.ts:
Line 60:
Unhandled external HTTP call failure in `getCallPersonnelCheckInStatusesApi.get` violates Rule 28. Wrap the call in a `try/catch` block, include the `callId` and endpoint name in the error context, and map it to an application-level error.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <Pressable | ||
| className="p-3" | ||
| className="p-2" | ||
| hitSlop={4} |
There was a problem hiding this comment.
Magic number 4 used directly as hitSlop degrades readability and maintainability across multiple components. Extract it into a named constant like const HIT_SLOP = 4; or a shared PressableHitSlop object.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/app/(app)/_layout.tsx:
Line 546:
Magic number `4` used directly as `hitSlop` degrades readability and maintainability across multiple components. Extract it into a named constant like `const HIT_SLOP = 4;` or a shared `PressableHitSlop` object.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| </VStack> | ||
| <VStack space="xs" className="items-end"> | ||
| <Badge action={getParBadgeAction(entry.Status)} variant="solid"> | ||
| <BadgeText className="text-white">{entry.Status === 'Green' ? t('command.par_green') : entry.Status === 'Warning' ? t('command.par_warning') : t('command.par_critical')}</BadgeText> |
There was a problem hiding this comment.
Duplicated magic strings for status values like 'Green' and 'Warning' introduce drift and typos. Centralize them into constants or an enum, such as const PAR_STATUS = { GREEN: 'Green', WARNING: 'Warning', CRITICAL: 'Critical' } as const, in a shared module.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/components/command/accountability-section.tsx:
Line 191:
Duplicated magic strings for status values like `'Green'` and `'Warning'` introduce drift and typos. Centralize them into constants or an enum, such as `const PAR_STATUS = { GREEN: 'Green', WARNING: 'Warning', CRITICAL: 'Critical' } as const`, in a shared module.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const accountabilityCount = visiblePersonnelStatuses.length + visibleTimers.length; | ||
|
|
||
| const refresh = useCallback(async () => { | ||
| await Promise.all([fetchTimerStatuses(callId), fetchPersonnelStatuses(callId), fetchResolvedTimers(callId)]); |
There was a problem hiding this comment.
Batch failure vulnerability where Promise.all discards all fetched data if a single request fails. Use Promise.allSettled to inspect each result's status independently, allowing the UI to populate with successful data while targeting specific failures.
Kody rule violation: Use Promise.allSettled for batch operations with partial failures
Prompt for LLM
File src/components/command/accountability-section.tsx:
Line 62:
Batch failure vulnerability where `Promise.all` discards all fetched data if a single request fails. Use `Promise.allSettled` to inspect each result's status independently, allowing the UI to populate with successful data while targeting specific failures.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| setTimeout(() => { | ||
| set((state) => ({ | ||
| toasts: state.toasts.filter((toast) => toast.id !== id), | ||
| })); | ||
| }, duration); |
There was a problem hiding this comment.
Orphaned callback leak caused by discarding the setTimeout return value, triggering unnecessary state updates via set() if the toast is manually dismissed. Capture the timer ID and call clearTimeout(timerId) inside removeToast to ensure a deterministic cleanup path.
Kody rule violation: Clear timers on teardown/unmount
Prompt for LLM
File src/stores/toast/store.ts:
Line 34 to 38:
Orphaned callback leak caused by discarding the `setTimeout` return value, triggering unnecessary state updates via `set()` if the toast is manually dismissed. Capture the timer ID and call `clearTimeout(timerId)` inside `removeToast` to ensure a deterministic cleanup path.
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:
|
| <View> | ||
| <Button | ||
| title="Load Feature" | ||
| onPress={() => setShowFeature(true)} |
There was a problem hiding this comment.
Render performance degradation caused by inline arrow functions in JSX props, such as onPress={() => setShowFeature(true)}. These allocations create new function instances on every render; move function definitions outside the render method or use standard callbacks.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File .agents/skills/react-native-best-practices/references/bundle-code-splitting.md:
Line 150:
Render performance degradation caused by inline arrow functions in JSX props, such as `onPress={() => setShowFeature(true)}`. These allocations create new function instances on every render; move function definitions outside the render method or use standard callbacks.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }); | ||
|
|
||
| ScriptManager.shared.on('error', (scriptId, error) => { | ||
| console.error(`Failed: ${scriptId}`, error); |
There was a problem hiding this comment.
Unstructured logging: console.error uses template-string interpolation instead of structured fields, violating Rule [3] for observability searchability. Replace this with a structured logger call, such as logger.error('chunk_load_failed', { scriptId, err: error }).
Kody rule violation: Include error context in structured logs
Prompt for LLM
File .agents/skills/react-native-best-practices/references/bundle-code-splitting.md:
Line 231:
Unstructured logging: `console.error` uses template-string interpolation instead of structured fields, violating Rule [3] for observability searchability. Replace this with a structured logger call, such as `logger.error('chunk_load_failed', { scriptId, err: error })`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <TextInput | ||
| ref={inputRef} | ||
| defaultValue="" | ||
| onChangeText={(text) => console.log('Current:', text)} |
There was a problem hiding this comment.
PII exposure risk: Raw TextInput values are logged to the console without redaction, violating Rule [48] against default raw PII logging. Hash or tokenize sensitive values before logging, or log only non-identifying metadata such as input length.
Kody rule violation: Redact PII in logs and metrics by default
Prompt for LLM
File .agents/skills/react-native-best-practices/references/js-uncontrolled-components.md:
Line 109:
PII exposure risk: Raw `TextInput` values are logged to the console without redaction, violating Rule [48] against default raw PII logging. Hash or tokenize sensitive values before logging, or log only non-identifying metadata such as input length.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const handleChange = (text) => { | ||
| setQuery(text); | ||
| fetchResults(text).then(setResults); |
There was a problem hiding this comment.
Network flooding: onChangeText triggers fetchResults(text) on every keystroke without a debounce, violating Rule [53] and degrading UX with excessive requests. Wrap the fetch call in a debounce utility, such as lodash.debounce with a 250–400ms delay.
Kody rule violation: Debounce or throttle user input that triggers work
Prompt for LLM
File .agents/skills/react-native-best-practices/references/js-uncontrolled-components.md:
Line 128:
Network flooding: `onChangeText` triggers `fetchResults(text)` on every keystroke without a debounce, violating Rule [53] and degrading UX with excessive requests. Wrap the fetch call in a debounce utility, such as `lodash.debounce` with a 250–400ms delay.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| NotificationCenter.default.addObserver( | ||
| self, | ||
| selector: #selector(onJSLoad), | ||
| name: NSNotification.Name("RCTJavaScriptDidLoadNotification"), | ||
| object: nil | ||
| ) |
There was a problem hiding this comment.
Memory leak risk: The NotificationCenter observer lacks a deterministic removal path, which can cause leaks or fire on deallocated objects. Store the observer token and remove it within deinit using NotificationCenter.default.removeObserver(observer).
Kody rule violation: Provide error handlers to subscription/listener APIs
Prompt for LLM
File .agents/skills/react-native-best-practices/references/native-measure-tti.md:
Line 221 to 226:
Memory leak risk: The `NotificationCenter` observer lacks a deterministic removal path, which can cause leaks or fire on deallocated objects. Store the observer token and remove it within `deinit` using `NotificationCenter.default.removeObserver(observer)`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ```tsx | ||
| import performance from 'react-native-performance'; | ||
|
|
||
| export default function HomeScreen() { |
There was a problem hiding this comment.
Maintainability constraint: The HomeScreen component uses a default export, which reduces refactoring clarity and auto-import capabilities. Replace it with a named export like export function HomeScreen().
Kody rule violation: Avoid default exports
Prompt for LLM
File .agents/skills/react-native-best-practices/references/native-measure-tti.md:
Line 173:
Maintainability constraint: The `HomeScreen` component uses a default export, which reduces refactoring clarity and auto-import capabilities. Replace it with a named export like `export function HomeScreen()`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ReactMarker.addListener { name -> | ||
| when (name) { | ||
| RUN_JS_BUNDLE_START -> { /* mark start */ } | ||
| RUN_JS_BUNDLE_END -> { /* mark end */ } | ||
| CONTENT_APPEARED -> { /* mark content */ } | ||
| } | ||
| } |
There was a problem hiding this comment.
Resource leak: The ReactMarker listener is never removed during teardown, causing unnecessary resource consumption. Store the listener reference and remove it using the appropriate remove API during application or module teardown.
Kody rule violation: Proper memory management in event listeners
Prompt for LLM
File .agents/skills/react-native-best-practices/references/native-measure-tti.md:
Line 232 to 238:
Resource leak: The `ReactMarker` listener is never removed during teardown, causing unnecessary resource consumption. Store the listener reference and remove it using the appropriate remove API during application or module teardown.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const getEndpoint = (path: string): MockEndpoint => { | ||
| const callIndex = mockCreateApiEndpoint.mock.calls.findIndex(([endpoint]) => endpoint === path); | ||
| return mockCreateApiEndpoint.mock.results[callIndex].value as unknown as MockEndpoint; |
There was a problem hiding this comment.
TypeError vulnerability: mock.results[callIndex] is indexed without verifying validity, meaning a -1 return from findIndex will throw a TypeError when accessing .value. Add a guard throwing an Error for invalid indices, or use optional chaining mock.results[callIndex]?.value.
Kody rule violation: Add null checks before accessing properties
Prompt for LLM
File src/api/check-in-timers/__tests__/check-in-timers.test.ts:
Line 33:
TypeError vulnerability: `mock.results[callIndex]` is indexed without verifying validity, meaning a `-1` return from `findIndex` will throw a `TypeError` when accessing `.value`. Add a guard throwing an `Error` for invalid indices, or use optional chaining `mock.results[callIndex]?.value`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const getEndpoint = (path: string): MockEndpoint => { | ||
| const callIndex = mockCreateApiEndpoint.mock.calls.findIndex(([endpoint]) => endpoint === path); | ||
| return mockCreateApiEndpoint.mock.results[callIndex].value as unknown as MockEndpoint; |
There was a problem hiding this comment.
TypeError vulnerability: Indexing mock.results with an unchecked callIndex from findIndex crashes when accessing .value on mock.results[-1]. Validate that callIndex >= 0 before indexing, or use mock.results.at(callIndex) with a null check.
Kody rule violation: Check query results before accessing indices
Prompt for LLM
File src/api/check-in-timers/__tests__/check-in-timers.test.ts:
Line 33:
TypeError vulnerability: Indexing `mock.results` with an unchecked `callIndex` from `findIndex` crashes when accessing `.value` on `mock.results[-1]`. Validate that `callIndex >= 0` before indexing, or use `mock.results.at(callIndex)` with a null check.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Enabled: enabled, | ||
| }); | ||
| const endpoint = createApiEndpoint(`/CheckInTimers/ToggleCallTimers?callId=${encodeURIComponent(callId)}&enabled=${encodeURIComponent(enabled)}`); | ||
| const response = await endpoint.put<PerformCheckInResult>({}); |
There was a problem hiding this comment.
Unhandled rejection risk: The awaited HTTP call lacks a try/catch guard, allowing PUT request failures to propagate without context. Wrap the await in a try/catch, log with structured context (callId, enabled, endpoint), and throw a typed error consistent with the CallPersonnelCheckInStatusesFetchError pattern in getCallPersonnelCheckInStatuses (lines 75-92).
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/api/check-in-timers/check-in-timers.ts:
Line 97:
Unhandled rejection risk: The awaited HTTP call lacks a `try/catch` guard, allowing `PUT` request failures to propagate without context. Wrap the `await` in a `try/catch`, log with structured context (`callId`, `enabled`, `endpoint`), and throw a typed error consistent with the `CallPersonnelCheckInStatusesFetchError` pattern in `getCallPersonnelCheckInStatuses` (lines 75-92).
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Enabled: enabled, | ||
| }); | ||
| const endpoint = createApiEndpoint(`/CheckInTimers/ToggleCallTimers?callId=${encodeURIComponent(callId)}&enabled=${encodeURIComponent(enabled)}`); | ||
| const response = await endpoint.put<PerformCheckInResult>({}); |
There was a problem hiding this comment.
Unhandled rejection risk: The endpoint.put call lacks a try/catch wrapper, violating Rule [28] for network error mapping. Wrap the await call in a try/catch to log structured context (callId, enabled, endpoint) and throw an application-level error, matching the getCallPersonnelCheckInStatuses pattern.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/api/check-in-timers/check-in-timers.ts:
Line 97:
Unhandled rejection risk: The `endpoint.put` call lacks a `try/catch` wrapper, violating Rule [28] for network error mapping. Wrap the `await` call in a `try/catch` to log structured context (`callId`, `enabled`, `endpoint`) and throw an application-level error, matching the `getCallPersonnelCheckInStatuses` pattern.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| CallId: callId, | ||
| Enabled: enabled, | ||
| }); | ||
| const endpoint = createApiEndpoint(`/CheckInTimers/ToggleCallTimers?callId=${encodeURIComponent(callId)}&enabled=${encodeURIComponent(enabled)}`); |
There was a problem hiding this comment.
Architectural inconsistency: The route /CheckInTimers/ToggleCallTimers is embedded inline in a template literal, breaking the centralized module-level constant pattern established by CALL_PERSONNEL_CHECK_IN_STATUSES_ENDPOINT. Extract the base path to a module-level constant like const TOGGLE_CALL_TIMERS_ENDPOINT = '/CheckInTimers/ToggleCallTimers'; and append query parameters at call time.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/api/check-in-timers/check-in-timers.ts:
Line 96:
Architectural inconsistency: The route `/CheckInTimers/ToggleCallTimers` is embedded inline in a template literal, breaking the centralized module-level constant pattern established by `CALL_PERSONNEL_CHECK_IN_STATUSES_ENDPOINT`. Extract the base path to a module-level constant like `const TOGGLE_CALL_TIMERS_ENDPOINT = '/CheckInTimers/ToggleCallTimers';` and append query parameters at call time.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <Pressable | ||
| className="p-3" | ||
| className="p-2" | ||
| hitSlop={4} |
There was a problem hiding this comment.
Magic number duplication: The numeric literal 4 for hitSlop is repeated across multiple components without a named constant. Extract this to a shared module-level constant like HIT_SLOP = 4 to ensure touch-target padding synchronization across CreateDrawerMenuButton and CreateHeaderBackButton.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/app/(app)/_layout.tsx:
Line 546:
Magic number duplication: The numeric literal `4` for `hitSlop` is repeated across multiple components without a named constant. Extract this to a shared module-level constant like `HIT_SLOP = 4` to ensure touch-target padding synchronization across `CreateDrawerMenuButton` and `CreateHeaderBackButton`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| useEffect(() => { | ||
| setTimersEnabled(initialTimersEnabled); | ||
| setLoadedCallId(null); | ||
|
|
||
| if (!initialTimersEnabled) { | ||
| stopPolling(); | ||
| return; | ||
| } | ||
|
|
||
| refresh(); | ||
| startPolling(callId); | ||
| return stopPolling; | ||
| }, [callId, initialTimersEnabled, refresh, startPolling, stopPolling]); |
There was a problem hiding this comment.
Stale state regression: The AccountabilitySection's useEffect (lines 87-99) fails to re-run after src/app/call/[id].tsx (lines 239-241) invokes reset() on the shared useCheckInTimerStore during cleanup, leaving accountability data empty and polling stopped. Since the effect's dependencies [callId, initialTimersEnabled, refresh, startPolling, stopPolling] are stable references, subscribe to store data staleness or add a separate useEffect to trigger refresh when timerStatuses is unexpectedly empty.
Prompt for LLM
File src/components/command/accountability-section.tsx:
Line 87 to 99:
Stale state regression: The `AccountabilitySection`'s `useEffect` (lines 87-99) fails to re-run after `src/app/call/[id].tsx` (lines 239-241) invokes `reset()` on the shared `useCheckInTimerStore` during cleanup, leaving accountability data empty and polling stopped. Since the effect's dependencies `[callId, initialTimersEnabled, refresh, startPolling, stopPolling]` are stable references, subscribe to store data staleness or add a separate `useEffect` to trigger `refresh` when `timerStatuses` is unexpectedly empty.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
|
|
||
| if (isOffline) { | ||
| activeToastIdRef.current = showToast( |
There was a problem hiding this comment.
Type safety violation: The string literal 'warning' represents a finite toast severity set without a strict type definition. Define a ToastType enum (e.g., enum ToastType { Warning='warning' }) and replace inline literals with enum references.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/components/common/offline-status-toast.tsx:
Line 37:
Type safety violation: The string literal `'warning'` represents a finite toast severity set without a strict type definition. Define a `ToastType` enum (e.g., `enum ToastType { Warning='warning' }`) and replace inline literals with enum references.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| try { | ||
| const [lat, lng] = centerStr.split(',').map(Number); | ||
| if (isNaN(lat) || isNaN(lng)) return null; | ||
| if (!Number.isFinite(lat) || !Number.isFinite(lng) || lat < -90 || lat > 90 || lng < -180 || lng > 180) return null; |
There was a problem hiding this comment.
Logic duplication: The coordinate validity check (isFinite + range) is duplicated in parseCenterLocation (line 101) and getPolygonBounds (line 128), risking implementation drift. Extract a shared isValidCoordinate(lat, lng): boolean helper to enforce single-source validation.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File src/lib/weather-alert-utils.ts:
Line 101:
Logic duplication: The coordinate validity check (`isFinite` + range) is duplicated in `parseCenterLocation` (line 101) and `getPolygonBounds` (line 128), risking implementation drift. Extract a shared `isValidCoordinate(lat, lng): boolean` helper to enforce single-source validation.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }; | ||
|
|
||
| export interface WeatherAlertMapBounds { | ||
| ne: [number, number]; |
There was a problem hiding this comment.
Type ambiguity: The northeast bound uses an unnamed tuple [number, number], obscuring longitude and latitude element order. Replace it with a named interface like { longitude: number; latitude: number } or explicitly document the GeoJSON [lng, lat] convention.
Kody rule violation: Prefer Named Classes Over Tuples
Prompt for LLM
File src/lib/weather-alert-utils.ts:
Line 109:
Type ambiguity: The northeast bound uses an unnamed tuple `[number, number]`, obscuring longitude and latitude element order. Replace it with a named interface like `{ longitude: number; latitude: number }` or explicitly document the GeoJSON `[lng, lat]` convention.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
2 participants
PR Description
This PR delivers a set of fixes and improvements across the command board accountability system, offline status handling, toast notifications, and weather alert rendering.
Accountability & Check-in Timers
toggleCallTimersAPI call (query-string based) andsetCallTimersEnabledstore action so ICs can enable timers on demand; the section shows an activation prompt when timers are inactive.getCallPersonnelCheckInStatusesendpoint and store state with typed error handling (CallPersonnelCheckInStatusesFetchError), severity sorting, and polling integration.performCheckInsupports aUserIdfield; these check-ins require a connection and cannot be queued offline.UnitIdchanged from string tonumber | null;LastCheckInmade nullable.Offline Status & Toast System
Weather Alerts
parseWeatherAlertDate/formatWeatherAlertDateto correctly handle department-local timestamps (both 12-hour and 24-hour formats), eliminating "Invalid Date" display.getPolygonBounds, validates center coordinates, and renders nothing when location data is unavailable instead of showing a default map.App Shell & UI
getAppHeaderHeightutility for consistent, safe-area-aware header sizing across portrait/landscape.hitSlopand reduced padding for better tap accessibility.Project & Tooling
/docsto.gitignore.