Sprite Mobile Code Review — Bugs, Dead Code & Quality Assessment
Systematic code review of the Knocklace Collection app: 3 confirmed bugs, dead code inventory, code quality assessment, and improvement recommendations.
Sprite Mobile Code Review — Bugs, Dead Code & Quality Assessment
Source: Reverse-engineering of
/Users/alefita/workdir/sprite-mobile/Version: 4.24.0 (JS) / 4.24.1 (native module) Severity scale: CRITICAL > HIGH > MEDIUM > LOW > INFO
1. Confirmed Bugs
Bug 1: setNotificationPermission Updates Wrong Slice State
Severity: HIGH
File: src/store/permissions/reducer.ts (lines 37-43)
Impact: Notification permission state is never correctly tracked; bluetooth permission state is silently overwritten when notification permission is checked.
The Bug:
// CURRENT (BUGGY)
setNotificationPermission: (state, action) => {
state.bluetooth.granted = action.payload.granted; // WRONG
state.bluetooth.canAskAgain = action.payload.canAskAgain; // WRONG
}
The Fix:
// CORRECT
setNotificationPermission: (state, action) => {
state.notification.granted = action.payload.granted;
state.notification.canAskAgain = action.payload.canAskAgain;
}
Root Cause: Copy-paste error from the adjacent setBluetoothPermission reducer, which correctly writes to state.bluetooth. The notification permission thunk dispatches this action, so every time the app checks notification permissions, it silently corrupts the bluetooth permission state.
Downstream Effects:
state.notificationis always{ granted: false, canAskAgain: true }(initial state) regardless of actual permissionstate.bluetoothgets overwritten with notification permission data — if the user grants notifications but denies bluetooth, the app thinks bluetooth is granted- Any UI component reading
permissions.notification.grantedwill display incorrect state - The
requestPermissionsorchestrator thunk may skip requesting bluetooth if it reads the corrupted notification state as bluetooth-granted
Workaround: The app still functions because BLE scanning proceeds regardless of the permission slice state on iOS (permissions are auto-granted). On Android, this could cause the bluetooth permission prompt to never appear if notification permission is granted first.
Bug 2: Hardcoded Portuguese in Unpair Alert
Severity: LOW
File: src/components/ConnectionStateManagerView/ConnectionStateManagerView.tsx (lines 48-49)
Impact: Non-Portuguese users see Portuguese button labels in the unpair confirmation dialog.
The Bug:
Alert.alert(
t('unpair-device'), // correctly internationalized
t('unpair-device-message'), // correctly internationalized
[
{ text: 'Cancelar', style: 'cancel' }, // HARDCODED PORTUGUESE
{ text: 'Continuar', onPress: () => ... }, // HARDCODED PORTUGUESE
]
);
The Fix:
Alert.alert(
t('unpair-device'),
t('unpair-device-message'),
[
{ text: t('cancel', 'Cancel'), style: 'cancel' },
{ text: t('continue', 'Continue'), onPress: () => ... },
]
);
Context: The i18n system has 13 translation keys but is missing cancel and continue. The developer likely wrote this quickly in Portuguese and forgot to add the keys. The rest of the component correctly uses t().
Severity rationale: LOW because the primary market is Brazilian (Sprite brand campaign), and the en_BR locale maps to Portuguese content anyway. But it violates the i18n contract established elsewhere in the codebase.
Bug 3: BLE Toggle onValueChange Is a No-Op
Severity: MEDIUM
File: src/components/ConnectionStateManagerView/ConnectionStateManagerView.tsx
Impact: The bluetooth toggle switch in the device connection view does nothing when tapped.
The Bug:
<Switch
value={false} // always off
onValueChange={() => {}} // empty handler
// ... styling props
/>
Analysis: The switch:
- Always renders as "off" (
value={false}) regardless of actual bluetooth state - Has an empty
onValueChangehandler — tapping it triggers no action - Does not dispatch
setBluetoothEnabled()or any BLE state action
Possible Intentions:
- Abandoned feature: The toggle was meant to enable/disable BLE scanning but was never completed
- UI placeholder: Left in during development for visual layout purposes
- Permission gate: Meant to trigger bluetooth enable/disable flow but the native bridge was never wired
Context: The showBluetothState prop (note: typo "Bluetoth" in the prop name) controls whether this switch renders at all. It is currently passed as true in RadarScreen, meaning users see and can interact with a completely non-functional toggle.
Recommendation: Either wire the toggle to BleManager.enable() / disable scanning, or remove it entirely. A non-functional toggle in a production app erodes user trust.
2. Dead Code Inventory
2.1 Unused npm Dependencies
| Package | Status | Evidence |
|---|---|---|
@react-native-masked-view/masked-view | Unused | Not imported in any .ts, .tsx, .js, or .jsx file |
its-fine | Unused | React internals hook library; zero imports found in source |
@react-native-masked-view/masked-view is typically used for gradient text effects or partial image reveals. It may have been used in a previous iteration of the UI and was left in package.json after the feature was removed.
its-fine is a niche library by Poimandres (pmndrs) that provides useFiber() and useContextBridge() hooks for accessing React fiber internals. Its presence suggests someone planned to do advanced React tree manipulation (possibly for the Skia integration) but never followed through.
2.2 Unused Babel Module Aliases
The babel.config.js defines 7 module aliases via babel-plugin-module-resolver:
| Alias | Target | Status |
|---|---|---|
@screens | ./src/screens/index | Active |
@components | ./src/components/index | Active |
@assets | ./assets/index | Active |
@store | ./src/store/index | Active |
@theme | ./src/theme/index | DEAD — directory does not exist |
@api | ./src/api/index | DEAD — directory does not exist |
@hooks | ./src/hooks/index | DEAD — directory does not exist |
Three of seven aliases point to nonexistent directories. If any import uses @theme, @api, or @hooks, it will fail at module resolution time with a confusing error. These are likely remnants from a project scaffold or boilerplate.
2.3 Dormant Kalman Filter (iOS)
Files:
modules/app-module/ios/KalmanFilter.swiftmodules/app-module/ios/Matrix.swiftmodules/app-module/ios/KalmanFilterType.swiftmodules/app-module/ios/DoubleExtension.swift
A complete matrix-based Kalman filter implementation using Apple's Accelerate framework (vDSP, cblas_dgemm, LAPACK dgetrf_/dgetri_). The implementation includes:
- State prediction and update matrices
- Covariance tracking
- Matrix inversion via LU decomposition
- Full Swift type system integration
However, the AppModule.filter() method in AppModule.swift uses a simple sliding window mean:
func filter(rssi: Int, variation: Int) -> Int {
positionsArray.append(rssi)
if positionsArray.count > variation {
positionsArray.removeFirst()
}
let uniqueValues = Set(positionsArray)
return uniqueValues.reduce(0, +) / uniqueValues.count
}
The Kalman filter is never called from the active code path. It appears to be a more sophisticated implementation that was prototyped but replaced with the simpler algorithm — possibly because the sliding window mean performed adequately for the 3-7 meter range, or because the Kalman filter's matrix inversion overhead was not justified given the sampling rate.
Impact: ~200 lines of dead Swift code, including a dependency on the Accelerate framework that is compiled but never executed.
2.4 Video Component Legacy
The react-native.config.js marks a Video component as unstable_reactLegacyComponentNames for both platforms:
module.exports = {
unstable_reactLegacyComponentNames: ['Video'],
};
This suggests the app previously used react-native-video for animated backgrounds before migrating to Skia's useAnimatedImageValue() for Android and static PNGs for iOS. The legacy component registration is a leftover.
3. Code Quality Assessment
3.1 Architecture Quality: B+
Strengths:
- Clean separation between Redux state, BLE service layer (
ApplicationService), and UI components - Event-driven architecture via
eventemitter3decouples BLE events from React rendering - Singleton
ApplicationServicepattern avoids prop drilling for BLE state - Platform-specific file resolution (
.android.tsxsuffix) for Skia vs. ImageBackground
Weaknesses:
ApplicationServiceis 798 lines — a god object handling BLE lifecycle, distance calculation, alert logic, remote config, and Redux coordination- No dependency injection —
ApplicationServiceis a singleton with directBleManagerandstorereferences - The store subscription pattern (mirroring Redux state into instance variables) bypasses React's rendering model
3.2 State Management: B
Strengths:
- Redux Toolkit with proper slice separation (application, bluetoothAdapter, permissions)
redux-persistwith sensible blacklist/whitelist configuration- Serializable check middleware correctly ignoring redux-persist actions
Weaknesses:
- The store subscription anti-pattern:
ApplicationServicesubscribes to the entire store and copies values into instance variables. This creates two sources of truth (Redux store and service instance variables) isAlertAlreadyShown,isAlertActive, andnotificationAlreadyShownForRangeare all boolean/array flags managing a state machine that would be clearer as an explicit state machine pattern- The permissions slice bug (Bug 1) indicates the slice was not integration-tested
3.3 i18n: C+
Strengths:
- Proper i18next setup with native locale detection
- Fallback language chain (
en_BR->pt->en) - Module-level initialization (runs before React renders)
Weaknesses:
- Only 13 translation keys — the notification system has its own inline locale detection (
isbrcheck) outside the i18n system - Missing keys for "Cancel" and "Continue" (Bug 2)
- The
en_BRlocale convention is unusual and may confuse future developers - No pluralization support, no interpolation, no namespaces — barely using i18next's capabilities
3.4 BLE Implementation: B+
Strengths:
- Correct use of
allowDuplicates: truefor continuous RSSI - Per-device calibration via remote config (
rssiAt1Meter,rssiPow) - Platform-aware device identification (name on iOS, MAC on Android)
- Background scanning on both platforms with proper service declarations
- Remote config with bundled fallback
Weaknesses:
- The locally forked
react-native-ble-plxwill not receive upstream updates — maintenance burden - No connection retry logic (the app is RSSI-only, so this is less critical)
- The BLE toggle no-op (Bug 3) suggests incomplete feature integration
3.5 Build & CI/CD: A-
Strengths:
- 5 GitHub Actions workflows covering the full distribution matrix
- Proper signing for both platforms (keystore for Android, provisioning for iOS)
- Firebase App Distribution for alpha, Play Store beta track, TestFlight for iOS beta
- Node 22, Yarn with corepack, dependency caching
Weaknesses:
releaseLocalvariant (release with debug keystore) could be a security risk if accidentally used for distribution- No automated testing workflow visible (no Jest/Detox step in CI)
3.6 Component Quality: B
Strengths:
- Consistent use of
React.memofor expensive renders (BackgroundVideo, RadarBaseAndEffect) react-native-reanimatedfor smooth 60fps animations (radar sweep, button springs)- Proper
forwardRefusage where needed (UrlButton)
Weaknesses:
- Mixed animation libraries:
react-native-reanimated(spring animations) alongsidereact-native's built-inAnimated(toggle thumb). Should standardize on one - No error boundaries
- No accessibility props (
accessibilityLabel,accessibilityRole) on interactive elements
4. Improvement Recommendations
4.1 Immediate Fixes (P0)
| # | Action | Effort |
|---|---|---|
| 1 | Fix setNotificationPermission reducer to update state.notification | 5 min |
| 2 | Add cancel/continue i18n keys and use in unpair alert | 15 min |
| 3 | Either implement BLE toggle functionality or remove the Switch component | 30 min |
4.2 Cleanup (P1)
| # | Action | Effort |
|---|---|---|
| 4 | Remove @react-native-masked-view/masked-view from dependencies | 5 min |
| 5 | Remove its-fine from dependencies | 5 min |
| 6 | Remove dead babel aliases (@theme, @api, @hooks) | 10 min |
| 7 | Decide on Kalman filter: activate it or delete the dead Swift files | 30 min |
| 8 | Clean up unstable_reactLegacyComponentNames in react-native config | 5 min |
4.3 Architecture Improvements (P2)
| # | Action | Effort |
|---|---|---|
| 9 | Refactor ApplicationService into smaller, focused services (BLE, Distance, Alert, Config) | 2-4 hours |
| 10 | Replace store subscription pattern with React hooks (useSelector) | 1-2 hours |
| 11 | Consolidate animation libraries to react-native-reanimated only | 1-2 hours |
| 12 | Move notification text into the i18n system | 30 min |
| 13 | Add error boundaries around screen components | 1 hour |
| 14 | Add accessibility props to all interactive elements | 1-2 hours |
5. Security Observations
5.1 API Key Exposure
The .env file contains API_URL and API_KEY for the Google Cloud Run remote config endpoint. These are baked into the app bundle via react-native-dotenv. While the API likely serves only read-only configuration, the key is extractable from the APK/IPA.
5.2 Remote Config Trust
The app trusts the remote config payload completely — device names, calibration values, and feature flags are all server-controlled. A compromised Cloud Run endpoint could:
- Redirect all proximity alerts to a phishing URL via
defaultVideo - Disable alerts entirely via
enableDot: false - Poison distance calibration to make devices appear closer/farther than reality
5.3 Emergency URL Injection
The emergencyUrl field accepts any string and is opened via Linking.openURL(). While the user sets this value, a malicious remote config could set defaultEmergencyUrl to a javascript: or custom scheme URL. React Native's Linking.openURL does perform some scheme validation, but this is implementation-dependent.
Cross-References
- sprite-mobile-knocklace — Full Knocklace product context and BLE protocol
- sprite-mobile-redux — Redux store architecture where Bug 1 lives
- preferencias-tecnicas — Alefita's technical preferences (React Native, uv, quality standards)