Conversation
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:
|
📝 WalkthroughWalkthroughAdded API-backed feature flags with persisted Zustand state. The ChangesFeature flag chat gating
Audio migration guidance
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TabLayout
participant FeatureFlagStore
participant FeatureFlagAPI
participant ChatHub
participant ChatScreen
TabLayout->>FeatureFlagStore: fetchFlags()
FeatureFlagStore->>FeatureFlagAPI: getAllFeatureFlags()
FeatureFlagAPI-->>FeatureFlagStore: ChatSystem state
alt ChatSystem enabled
TabLayout->>ChatHub: connect
ChatScreen->>FeatureFlagStore: useIsChatEnabled()
ChatScreen->>ChatHub: fetch chat data
else ChatSystem disabled
TabLayout-->>TabLayout: skip chat connection
ChatScreen-->>ChatScreen: redirect to /home
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/chat/[channelId].tsx (1)
69-78: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate the remaining channel requests.
Lines 69-78 skip channel activation, but the presence request at Lines 82-94 and
markChannelReadat Lines 97-101 still run when chat is disabled. A disabled deep link can therefore read presence or update chat state before navigation completes.Add
isChatEnabledguards and dependencies to both effects.Proposed guard changes
useEffect(() => { + if (!isChatEnabled) return; const ids = (members ?? []).map((m) => m.UserId).filter((id): id is string => !!id && id !== currentUserId); if (ids.length === 0) return; // ... -}, [members, currentUserId]); +}, [members, currentUserId, isChatEnabled]); useEffect(() => { - if (channelId && inverted.length > 0) { + if (isChatEnabled && channelId && inverted.length > 0) { void useChatStore.getState().markChannelRead(channelId); } -}, [channelId, inverted.length]); +}, [channelId, inverted.length, isChatEnabled]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/chat/`[channelId].tsx around lines 69 - 78, Update the presence-request effect and the markChannelRead effect in the chat channel component to return early when isChatEnabled is false, matching the existing guard around setActiveChannel and channel loading. Add isChatEnabled to both effects’ dependency arrays while preserving their current behavior when chat is enabled.
🤖 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 `@docs/audio-stream-refactoring.md`:
- Around line 9-10: Update the later Overview, Key Changes, and Installation
sections in the audio-stream refactoring document to consistently describe the
expo-audio SDK 56 migration using createAudioPlayer, setAudioModeAsync,
AudioPlayer, and playbackStatusUpdate; remove or explicitly label any remaining
expo-av guidance as an SDK 54 baseline.
- Around line 3-5: Update the “Expo SDK 56 migration requirement” guidance to
sequence the expo-audio dependency update during the SDK 56 upgrade rather than
before it, and describe alignment through the SDK upgrade process. If specifying
a package version, use the tested SDK 56-compatible expo-audio version;
otherwise remove the unsupported prerequisite-version wording while retaining
the expo-av replacement requirement.
In `@src/stores/feature-flags/store.ts`:
- Around line 62-66: Separate unresolved feature flags from disabled flags by
adding non-persisted current-department resolution state alongside the enabled
value in useFeatureFlag within src/stores/feature-flags/store.ts#L62-L66. Update
the redirect/rendering logic in src/app/(app)/chat.tsx#L109-L112,
src/app/(app)/chatbot.tsx#L68-L71, src/app/chat/[channelId].tsx#L239-L242, and
src/app/chat/thread/[messageId].tsx#L101-L104 to defer redirects while
unresolved, using ternary rendering for pending, enabled, and disabled states;
the store change is the root fix and all listed route sites require
corresponding updates.
- Around line 55-58: Scope feature-flag persistence to the current
DepartmentCode, or clear the feature-flags store before logout and department
changes. Update the feature-flags store around the feature-flags-storage
configuration and ensure stale flags cannot survive identity changes or fetch
failures for a new department.
---
Outside diff comments:
In `@src/app/chat/`[channelId].tsx:
- Around line 69-78: Update the presence-request effect and the markChannelRead
effect in the chat channel component to return early when isChatEnabled is
false, matching the existing guard around setActiveChannel and channel loading.
Add isChatEnabled to both effects’ dependency arrays while preserving their
current behavior when chat is enabled.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0aaa6653-a6c7-4828-bf10-32dd7eb6935c
📒 Files selected for processing (9)
docs/audio-stream-refactoring.mdsrc/api/feature-flags/feature-flags.tssrc/app/(app)/_layout.tsxsrc/app/(app)/chat.tsxsrc/app/(app)/chatbot.tsxsrc/app/chat/[channelId].tsxsrc/app/chat/thread/[messageId].tsxsrc/components/sidebar/side-menu.tsxsrc/stores/feature-flags/store.ts
| ## Expo SDK 56 migration requirement | ||
|
|
||
| Before upgrading Dispatch to Expo SDK 56, upgrade `expo-audio` to the SDK 56-compatible version and replace all remaining `expo-av` audio usage with `expo-audio`. SDK 56 no longer provides the legacy Expo Modules Core header required by `expo-av` 16, so leaving `expo-av` installed can break the iOS archive build. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate document and relevant docs/config:"
fd -a 'audio-stream-refactoring\.md|app\.json|app\.config\.(js|ts)|package\.json|expo\.config\.(js|ts)|babel\.config' . | sed 's#^\./##' | head -200
echo
echo "Show audio-stream-reffactoring.md context:"
if [ -f docs/audio-stream-refactoring.md ]; then
nl -ba docs/audio-stream-refactoring.md | sed -n '1,220p'
fi
echo
echo "Find expo versions and audio packages:"
rg -n '"expo"|"react-native"|"expo-audio"|"expo-av"|"babel-preset-expo"|SDK 56|SDK 54' -S --glob '!node_modules' --glob '!dist' --glob '!build' .Repository: Resgrid/Dispatch
Length of output: 433
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Audio-refactoring context:"
awk '{ printf "%6d %s\n", NR, $0 }' docs/audio-stream-refactoring.md | sed -n '1,220p'
echo
echo "Package/Expo config snippets:"
for f in app.config.ts babel.config.js package.json; do
echo "--- $f ---"
awk '{ printf "%6d %s\n", NR, $0 }' "$f" | sed -n '1,220p'
done
echo
echo "Search relevant strings:"
rg -n '"expo"|"react-native"|"expo-audio"|"expo-av"|"babel-preset-expo"|SDK 56|SDK 54' -S --glob '!node_modules' --glob '!dist' --glob '!build' .Repository: Resgrid/Dispatch
Length of output: 27844
🌐 Web query:
Expo SDK 54 expo-audio 1.1 installable expo-audio 16 expo-audio SDK 56 compatibility expo doc
💡 Result:
The expo-audio library is fully compatible with and designed for modern Expo SDKs, including SDK 54 and SDK 56. 1. Expo SDK 54: expo-audio 1.1 is compatible and was the standard for audio in SDK 54 [1][2]. During the SDK 54 lifecycle, users were encouraged to migrate to expo-audio from the deprecated expo-av [3][4]. You can install the version appropriate for your SDK using the command: npx expo install expo-audio [5]. 2. Expo SDK 56: expo-audio version 56.x is the version associated with SDK 56 [6][7]. The library received significant updates in SDK 56, including a new useAudioStream hook and various live-streaming improvements [8][9]. Important Note on Versions: In the Expo ecosystem, you should not manually choose a version number like 16 for expo-audio unless it specifically aligns with your SDK. Instead, always use the command npx expo install expo-audio [5]. This command ensures that the Expo CLI automatically installs the specific version of the package that is tested and compatible with your current Expo SDK version [5]. Using version 16 with SDK 56 is likely incorrect, as the package versions for expo-audio follow the SDK release numbering (e.g., version 56.x for SDK 56) [6][7]. Always verify your dependencies using npx expo-doctor after installation [5].
Citations:
- 1: https://docs.expo.dev/versions/v54.0.0/sdk/audio
- 2: https://github.com/expo/expo/blob/sdk-54/packages/expo-audio/CHANGELOG.md
- 3: https://expo.dev/changelog/sdk-54?gad_campaignid=23330869816
- 4: https://docs.expo.dev/versions/v54.0.0/sdk/audio-av/
- 5: https://github.com/expo/fyi/blob/main/resolving-dependency-issues.md
- 6: https://github.com/expo/expo/blob/main/packages/expo-audio/CHANGELOG.md
- 7: https://github.com/expo/expo/blob/HEAD/packages/expo-audio/CHANGELOG.md
- 8: https://expo.dev/changelog/sdk-56-beta
- 9: https://dev.to/expo/expo-sdk-56-5eb5
🌐 Web query:
docs.expo.dev "expo-audio" "Expo SDK 56"
💡 Result:
In Expo SDK 56, the expo-audio library is officially supported and includes several key updates [1][2]. Key features and improvements for expo-audio in SDK 56 include: - New useAudioStream Hook: This hook provides real-time access to microphone buffer data, enabling new possibilities for audio processing [1][3]. - Live-Stream Enhancements: - An isLiveStream option is now available for the lock-screen on iOS [1][2]. - A playsInSilentMode option has been added for Android [1][3]. - The AudioStatus object has been updated to include new fields: isLive, currentOffsetFromLive, and error [1][2]. You can access the official documentation for this version at https://docs.expo.dev/versions/v56.0.0/sdk/audio/ [1][2].
Citations:
- 1: https://expo.dev/changelog/sdk-56
- 2: https://dev.to/expo/expo-sdk-56-5eb5
- 3: https://medium.com/@onix_react/whats-new-in-expo-sdk-56-63f704fc8426
🌐 Web query:
Expo docs "Before upgrading" "During the upgrade" package compatibility SDK 56 expo install
💡 Result:
To upgrade your project to Expo SDK 56, follow these recommended practices regarding preparation, package compatibility, and the installation process [1][2][3]. Preparation and Best Practices Before starting the upgrade, it is strongly recommended that you read the official SDK 56 changelog and breaking changes documentation [1][4]. This allows you to identify necessary configuration adjustments specific to your application before beginning the actual upgrade process [1]. Additionally, use a dedicated git branch for the upgrade to isolate changes [2][3]. If you are still using Expo Go for development, it is recommended to migrate to development builds first, as this provides a more stable baseline for testing and debugging during and after the upgrade [5]. It is also advised to upgrade one major change at a time (e.g., upgrading the SDK version first, then enabling the New Architecture separately) to make it easier to isolate and troubleshoot any issues [5][4][6]. Package Compatibility and Installation During the upgrade, you should prioritize ensuring that your dependencies are compatible with the new SDK version [2][3]. The primary command for updating your Expo-related packages is [2][3]: npx expo install expo@^56.0.0 After updating the core expo package, run the package-aware installer to automatically align your remaining dependencies with the requirements of SDK 56 [2][3]: npx expo install --fix Post-Upgrade Verification Once the installation is complete, it is crucial to verify your project's health. The recommended next step is to run Expo Doctor, which is designed to detect mismatched versions, incompatible dependencies, and configuration issues [2][3]: npx expo-doctor If your project utilizes Expo Router, be aware that SDK 56 includes internal migration work; if you encounter issues, you may need to use the provided codemod to handle transitions between file-based routing and React Navigation imports [2][3]: npx expo-codemod sdk-56-expo-router-react-navigation-replace src Finally, rebuild your development builds and test thoroughly before deploying to production [2][3].
Citations:
- 1: https://expo.dev/blog/upgrading-to-sdk-56
- 2: https://paddyb.com/tutorials/expo-56-upgrade.html
- 3: https://signalandsyntax.netlify.app/blog/expo-sdk-56-beta-react-native
- 4: https://expo.dev/blog/upgrading-to-sdk-55
- 5: https://github.com/expo/fyi/blob/main/troubleshooting-sdk-upgrades.md
- 6: https://expo.dev/blog/upgrading-to-expo-sdk-52
Sequence the SDK 56 package update with the SDK 56 upgrade.
Expo’s SDK upgrade process updates Expo packages with the SDK and uses npx expo install --fix to align dependencies. Keep this guidance focused on the SDK upgrade, or change “Before upgrading” to “During the SDK 56 upgrade” and add the tested expo-audio package version.
🤖 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 `@docs/audio-stream-refactoring.md` around lines 3 - 5, Update the “Expo SDK 56
migration requirement” guidance to sequence the expo-audio dependency update
during the SDK 56 upgrade rather than before it, and describe alignment through
the SDK upgrade process. If specifying a package version, use the tested SDK
56-compatible expo-audio version; otherwise remove the unsupported
prerequisite-version wording while retaining the expo-av replacement
requirement.
Source: MCP tools
| - Migrate `src/hooks/use-ptt.ts`, `src/components/calls/call-audio-modal.tsx`, `src/stores/app/audio-stream-store.ts`, and `src/services/audio.service.ts` to `createAudioPlayer`, `setAudioModeAsync`, `AudioPlayer`, and `playbackStatusUpdate`. | ||
| - Remove `expo-av` from `package.json`, the lockfile, tests/mocks, and the Expo Doctor exclusion after no imports remain. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
if [ -f docs/audio-stream-refactoring.md ]; then
wc -l docs/audio-stream-refactoring.md
sed -n '1,220p' docs/audio-stream-refactoring.md
else
echo "docs/audio-stream-refactoring.md not found"
fi
echo
echo "== expo-av mentions in docs/audio-stream-refactoring.md =="
rg -n "expo-av|expo-audio|audio-av|createAudioPlayer|setAudioModeAsync|AudioPlayer|playbackStatusUpdate|Migrate|Remove" docs/audio-stream-refactoring.md || trueRepository: Resgrid/Dispatch
Length of output: 7247
🌐 Web query:
Expo SDK 56 expo-av expo-audio expo-audio SDK 56 createAudioPlayer AudioPlayer
💡 Result:
In Expo SDK 56, the expo-audio library is the standard tool for audio playback and recording, having replaced the deprecated expo-av package [1][2]. To work with audio, you primarily use the useAudioPlayer hook or, in advanced scenarios where the player must persist beyond a component's lifecycle, the createAudioPlayer function [3][4]. 1. useAudioPlayer(source, options): This is the recommended approach for most use cases [3]. It is a React hook that automatically manages the lifecycle of the AudioPlayer instance, ensuring that resources are properly disposed of when the component unmounts [5][6]. 2. createAudioPlayer(source, options): This function allows you to create an AudioPlayer instance manually [3][5]. Because this instance is not automatically managed by a component's lifecycle, it is your responsibility to call the release method on the player when it is no longer needed to prevent memory leaks [3][4]. Key differences when migrating from expo-av include: - Time is measured in seconds rather than milliseconds [7]. - Audio loads immediately upon creation [7]. - There is no automatic reset to the beginning when playback finishes; if you wish to replay, you must manually call player.seekTo(0) before calling play [7][8]. For additional functionality, expo-audio also provides the useAudioRecorder hook for recording [7][3].
Citations:
- 1: https://expo.dev/blog/upgrading-to-sdk-55
- 2: https://github.com/expo/skills/blob/main/plugins/expo/skills/upgrading-expo/SKILL.md
- 3: https://docs.expo.dev/versions/latest/sdk/audio/
- 4: https://github.com/expo/expo/blob/main/docs/pages/versions/unversioned/sdk/audio.mdx
- 5: https://github.com/expo/expo/blob/073ace8d/packages/expo-audio/src/ExpoAudio.ts
- 6: https://github.com/expo/expo/blob/97172ac0/packages/expo-audio/build/ExpoAudio.d.ts
- 7: https://github.com/expo/skills/blob/main/plugins/expo/skills/expo-upgrade/references/expo-av-to-audio.md
- 8: https://docs.expo.dev/versions/v53.0.0/sdk/audio/
Align expo-av references with the migration goal.
Lines 9-10 remove expo-av, but later sections describe expo-av, the Overview, the Key Changes, and the Installation steps still instruct using expo-av. Rewrite those sections to describe the expo-audio SDK 56 implementation, or label them as an SDK 54 baseline only.
🤖 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 `@docs/audio-stream-refactoring.md` around lines 9 - 10, Update the later
Overview, Key Changes, and Installation sections in the audio-stream refactoring
document to consistently describe the expo-audio SDK 56 migration using
createAudioPlayer, setAudioModeAsync, AudioPlayer, and playbackStatusUpdate;
remove or explicitly label any remaining expo-av guidance as an SDK 54 baseline.
Source: MCP tools
| { | ||
| name: 'feature-flags-storage', | ||
| storage: createJSONStorage(() => zustandStorage), | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate persistence reset or rehydration calls for this store.
rg -n -C 3 'feature-flags-storage|featureFlagsStore\.(persist\.)?(clearStorage|rehydrate)|featureFlagsStore\.setState' src
# Inspect authentication and department-transition paths for a feature-flag reset.
rg -n -C 3 '\b(signOut|logout|signedOut|DepartmentCode)\b' srcRepository: Resgrid/Dispatch
Length of output: 24927
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== feature-flags store outline =="
ast-grep outline src/stores/feature-flags/store.ts --view expanded || true
echo "== feature-flags store contents =="
cat -n src/stores/feature-flags/store.ts
echo "== feature-flags imports/usages =="
rg -n -C 4 'featureFlagsStore|feature.*flags|Flags|Chat\.System' src --glob '!src/translations/**'
echo "== auth store relevant logout path =="
sed -n '120,175p' src/stores/auth/store.tsx | cat -n
echo "== any clearStorage/rehydrate / storage reset =="
rg -n 'clearStorage|rehydrate\(\)|setPersist|persist\.clearStorage|name:\s*feature-flags-storage' src --glob '!src/translations/**'Repository: Resgrid/Dispatch
Length of output: 21012
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== app/_layout.tsx relevant initialization and gating =="
sed -n '330,410p' src/app/(' '(app))/_layout.tsx | cat -n
sed -n '520,555p' src/app/(' '(app))/_layout.tsx | cat -n
echo "== security/store identity change helpers and DepartmentCode state =="
cat -n src/stores/security/store.ts
echo "== all DepartmentCode identifiers =="
rg -n -C 3 'DepartmentCode|departmentCode|setDepartment|Department' src/stores src/services src/app src/components \
--glob '!src/translations/**' --glob '!src/__tests__/**' \
| head -n 220
echo "== imports/features around auth/store and security =="
rg -n -C 3 'useSecurityStore|securityStore|useAuthStore|useAuth|logout\(|signOut' src/app src/hooks src/stores src/components \
--glob '!src/translations/**' --glob '!src/__tests__/**' \
| head -n 260
echo "== targeted reset calls =="
rg -n 'clearStorage|featureFlagsStore\.(persist\.)?(clearStorage|rehydrate)|featureFlagsStore\.setState\(|logout\(\)\s*$|signOut\(\)\s*$' src --glob '!src/translations/**'Repository: Resgrid/Dispatch
Length of output: 264
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== app/_layout.tsx relevant initialization and gating =="
sed -n '330,410p' 'src/app/(app)/_layout.tsx' | cat -n
sed -n '520,555p' 'src/app/(app)/_layout.tsx' | cat -n
echo "== security/store identity change helpers and DepartmentCode state =="
cat -n src/stores/security/store.ts
echo "== all DepartmentCode identifiers =="
rg -n -C 3 'DepartmentCode|departmentCode|setDepartment|Department' src/stores src/services src/app src/components \
--glob '!src/translations/**' --glob '!src/__tests__/**' \
| head -n 220
echo "== imports/features around auth/store and security =="
rg -n -C 3 'useSecurityStore|securityStore|useAuthStore|useAuth|logout\(|signOut' src/app src/hooks src/stores src/components \
--glob '!src/translations/**' --glob '!src/__tests__/**' \
| head -n 260
echo "== targeted reset calls =="
rg -n 'clearStorage|featureFlagsStore\.(persist\.)?(clearStorage|rehydrate)|featureFlagsStore\.setState\(|logout\(\)\s*$|signOut\(\)\s*$' src --glob '!src/translations/**'Repository: Resgrid/Dispatch
Length of output: 44059
Reset or scope feature-flag persistence on sign-out and department changes.
feature-flags-storage is device-wide, while getAllFeatureFlags() evaluates flags for the current department. src/app/(app)/_layout.tsx fetches flags after rights, but saveFlags preserves the previous flags on fetch failure and src/stores/auth/store.tsx logout does not clear them. A prior department can leave Chat.System: true for the next department. Use a DepartmentCode-scoped storage key or clear this store before identity changes.
🤖 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/feature-flags/store.ts` around lines 55 - 58, Scope feature-flag
persistence to the current DepartmentCode, or clear the feature-flags store
before logout and department changes. Update the feature-flags store around the
feature-flags-storage configuration and ensure stale flags cannot survive
identity changes or fetch failures for a new department.
| // Reactive hook; components re-render when the flag changes. Unknown flags default to disabled | ||
| // so gated features stay hidden until the server confirms them. | ||
| export const useFeatureFlag = (key: string, defaultValue = false) => featureFlagsStore((state) => state.flags[key]?.enabled ?? defaultValue); | ||
|
|
||
| export const useIsChatEnabled = () => useFeatureFlag(FeatureFlagKeys.ChatSystem); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Separate unresolved flags from disabled flags.
useFeatureFlag returns false before the current department flag request completes. Each route then treats an unresolved flag as a confirmed disabled flag and redirects enabled deep links.
src/stores/feature-flags/store.ts#L62-L66: expose a non-persisted current-department resolution state with the enabled value.src/app/(app)/chat.tsx#L109-L112: defer the redirect until feature flags resolve.src/app/(app)/chatbot.tsx#L68-L71: defer the redirect until feature flags resolve.src/app/chat/[channelId].tsx#L239-L242: defer the redirect until feature flags resolve.src/app/chat/thread/[messageId].tsx#L101-L104: defer the redirect until feature flags resolve.
Use a ternary to render the pending, enabled, and disabled states.
As per coding guidelines, use ternary operator (? :) for conditional rendering and not && operator.
📍 Affects 5 files
src/stores/feature-flags/store.ts#L62-L66(this comment)src/app/(app)/chat.tsx#L109-L112src/app/(app)/chatbot.tsx#L68-L71src/app/chat/[channelId].tsx#L239-L242src/app/chat/thread/[messageId].tsx#L101-L104
🤖 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/feature-flags/store.ts` around lines 62 - 66, Separate unresolved
feature flags from disabled flags by adding non-persisted current-department
resolution state alongside the enabled value in useFeatureFlag within
src/stores/feature-flags/store.ts#L62-L66. Update the redirect/rendering logic
in src/app/(app)/chat.tsx#L109-L112, src/app/(app)/chatbot.tsx#L68-L71,
src/app/chat/[channelId].tsx#L239-L242, and
src/app/chat/thread/[messageId].tsx#L101-L104 to defer redirects while
unresolved, using ternary rendering for pending, enabled, and disabled states;
the store change is the root fix and all listed route sites require
corresponding updates.
Source: Coding guidelines
|
|
||
| /** Evaluates every active flag for the caller's department. */ | ||
| export const getAllFeatureFlags = async (signal?: AbortSignal) => { | ||
| const response = await api.get<FeatureTogglesResult>(`${FEATURE_TOGGLES}/GetAll`, { signal }); |
There was a problem hiding this comment.
Unguarded external HTTP call via api.get violates Rule [27] by lacking contextual error mapping. Wrap the call in a try/catch block, attach the operation name and endpoint, and map errors to a feature-flags domain error or safe default.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/api/feature-flags/feature-flags.ts:
Line 29:
Unguarded external HTTP call via `api.get` violates Rule [27] by lacking contextual error mapping. Wrap the call in a try/catch block, attach the operation name and endpoint, and map errors to a feature-flags domain error or safe default.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // Connect the realtime chat hub (best-effort; chat may be disabled per department) | ||
| try { | ||
| await useSignalRStore.getState().connectChatHub(); | ||
| // Connect the realtime chat hub only when the Chat.System feature flag is on for |
There was a problem hiding this comment.
Feature flag bypass occurs in useSignalRLifecycle, which unconditionally reconnects the chat hub on app resume via signalRStore.connectChatHub() at use-signalr-lifecycle.ts:121, defeating the Chat.System gate at _layout.tsx:182. Thread the feature flag into the resume reconnect path in handleAppResume to skip connectChatHub() when featureFlagsStore.getState().isEnabled(FeatureFlagKeys.ChatSystem) is false.
// Gate must also be enforced in use-signalr-lifecycle.ts handleAppResume:
// const hubs = [signalRStore.connectUpdateHub(), signalRStore.connectGeolocationHub()];
// if (featureFlagsStore.getState().isEnabled(FeatureFlagKeys.ChatSystem)) {
// hubs.push(signalRStore.connectChatHub());
// }
// const results = await Promise.allSettled(hubs);Prompt for LLM
File src/app/(app)/_layout.tsx:
Line 180:
Feature flag bypass occurs in `useSignalRLifecycle`, which unconditionally reconnects the chat hub on app resume via `signalRStore.connectChatHub()` at `use-signalr-lifecycle.ts:121`, defeating the `Chat.System` gate at `_layout.tsx:182`. Thread the feature flag into the resume reconnect path in `handleAppResume` to skip `connectChatHub()` when `featureFlagsStore.getState().isEnabled(FeatureFlagKeys.ChatSystem)` is false.
Suggested Code:
// Gate must also be enforced in use-signalr-lifecycle.ts handleAppResume:
// const hubs = [signalRStore.connectUpdateHub(), signalRStore.connectGeolocationHub()];
// if (featureFlagsStore.getState().isEnabled(FeatureFlagKeys.ChatSystem)) {
// hubs.push(signalRStore.connectChatHub());
// }
// const results = await Promise.allSettled(hubs);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| context: { platform: Platform.OS }, | ||
| }); | ||
|
|
||
| await featureFlagsStore.getState().fetchFlags(); |
There was a problem hiding this comment.
Unguarded awaited fetchFlags() call risks unhandled rejections that fail app initialization. Wrap the call in a try/catch block and log the error context to comply with Rule [1].
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/app/(app)/_layout.tsx:
Line 158:
Unguarded awaited `fetchFlags()` call risks unhandled rejections that fail app initialization. Wrap the call in a try/catch block and log the error context to comply with Rule [1].
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| // Chat.System feature flag off: no chat for this department. | ||
| if (!isChatEnabled) { | ||
| return <Redirect href={'/home' as Href} />; |
There was a problem hiding this comment.
Magic string '/home' for a finite application route reduces maintainability and risks typos. Define route names as a Route enum or const tuple instead of using raw strings.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/app/(app)/chat.tsx:
Line 111:
Magic string '/home' for a finite application route reduces maintainability and risks typos. Define route names as a Route enum or const tuple instead of using raw strings.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const menuItems = getMenuItems(t); | ||
| const isChatEnabled = useIsChatEnabled(); | ||
| // Chat and the assistant are gated by the Chat.System feature flag. | ||
| const menuItems = getMenuItems(t).filter((item) => (item.id === 'chat' || item.id === 'assistant' ? isChatEnabled : true)); |
There was a problem hiding this comment.
Duplicated inline string literals 'chat' and 'assistant' as menu-item identifiers risk drift across getMenuItems and other components. Define these shared keys as centralized constants, such as MenuItemIds.Chat and MenuItemIds.Assistant, to ensure consistency.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/components/sidebar/side-menu.tsx:
Line 100:
Duplicated inline string literals 'chat' and 'assistant' as menu-item identifiers risk drift across `getMenuItems` and other components. Define these shared keys as centralized constants, such as `MenuItemIds.Chat` and `MenuItemIds.Assistant`, to ensure consistency.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| flags: {}, | ||
| isLoaded: false, | ||
| error: null, | ||
| fetchFlags: async () => { |
There was a problem hiding this comment.
Missing JSDoc on the async fetchFlags method hides its failure semantics, specifically that it swallows errors and sets the error state rather than rejecting. Add a JSDoc block to document the resolve value, rejection conditions, and await usage to satisfy Rule [22].
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File src/stores/feature-flags/store.ts:
Line 34:
Missing JSDoc on the async `fetchFlags` method hides its failure semantics, specifically that it swallows errors and sets the `error` state rather than rejecting. Add a JSDoc block to document the resolve value, rejection conditions, and await usage to satisfy Rule [22].
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
2 participants
Pull Request Description
This PR introduces a department-scoped feature flag system to the Resgrid Dispatch mobile app, with the Chat system as the first feature gated behind it.
What was added
Feature Flags API Client (
src/api/feature-flags/feature-flags.ts): New API integration to evaluate feature toggles from the v4 FeatureToggles API, supporting both bulk retrieval (GetAll) and single-flag state checks (GetState).Feature Flags Store (
src/stores/feature-flags/store.ts): A persisted Zustand store that fetches and caches feature flags. It retains flags on fetch failure to keep gating stable while offline, and unknown flags default to disabled until the server confirms them.Chat Feature Gating: The
Chat.Systemflag controls all chat surfaces:_layout.tsx): Feature flags are fetched after security rights load; the SignalR chat hub only connects when chat is enabled.chat.tsx,chatbot.tsx,chat/[channelId].tsx,chat/thread/[messageId].tsx): Redirect to home when chat is disabled, and skip data-fetching effects.side-menu.tsx): Chat and Assistant menu items are hidden when the flag is off.Documentation: Updated audio stream refactoring docs with Expo SDK 56 migration guidance for
expo-audio/expo-av.