---
name: sprite-mobile-code-review
type: analysis
title: "Sprite Mobile Code Review — Bugs, Dead Code & Quality Assessment"
description: "Systematic code review of the Knocklace Collection app: 3 confirmed bugs, dead code inventory, code quality assessment, and improvement recommendations."
tags: [sprite, code-review, bugs, dead-code, react-native, quality, redux, ble]
timestamp: 2026-07-22
---

# 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:**

```typescript
// CURRENT (BUGGY)
setNotificationPermission: (state, action) => {
  state.bluetooth.granted = action.payload.granted;      // WRONG
  state.bluetooth.canAskAgain = action.payload.canAskAgain;  // WRONG
}
```

**The Fix:**

```typescript
// 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.notification` is always `{ granted: false, canAskAgain: true }` (initial state) regardless of actual permission
- `state.bluetooth` gets 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.granted` will display incorrect state
- The `requestPermissions` orchestrator 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:**

```typescript
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:**

```typescript
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:**

```typescript
<Switch
  value={false}                    // always off
  onValueChange={() => {}}         // empty handler
  // ... styling props
/>
```

**Analysis:** The switch:
1. Always renders as "off" (`value={false}`) regardless of actual bluetooth state
2. Has an empty `onValueChange` handler — tapping it triggers no action
3. 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.swift`
- `modules/app-module/ios/Matrix.swift`
- `modules/app-module/ios/KalmanFilterType.swift`
- `modules/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:

```swift
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:

```javascript
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 `eventemitter3` decouples BLE events from React rendering
- Singleton `ApplicationService` pattern avoids prop drilling for BLE state
- Platform-specific file resolution (`.android.tsx` suffix) for Skia vs. ImageBackground

**Weaknesses:**
- `ApplicationService` is 798 lines — a god object handling BLE lifecycle, distance calculation, alert logic, remote config, and Redux coordination
- No dependency injection — `ApplicationService` is a singleton with direct `BleManager` and `store` references
- 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-persist` with sensible blacklist/whitelist configuration
- Serializable check middleware correctly ignoring redux-persist actions

**Weaknesses:**
- The store subscription anti-pattern: `ApplicationService` subscribes to the entire store and copies values into instance variables. This creates two sources of truth (Redux store and service instance variables)
- `isAlertAlreadyShown`, `isAlertActive`, and `notificationAlreadyShownForRange` are 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 (`isbr` check) outside the i18n system
- Missing keys for "Cancel" and "Continue" (Bug 2)
- The `en_BR` locale 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: true` for 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-plx` will 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:**
- `releaseLocal` variant (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.memo` for expensive renders (BackgroundVideo, RadarBaseAndEffect)
- `react-native-reanimated` for smooth 60fps animations (radar sweep, button springs)
- Proper `forwardRef` usage where needed (UrlButton)

**Weaknesses:**
- Mixed animation libraries: `react-native-reanimated` (spring animations) alongside `react-native`'s built-in `Animated` (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)