update mobile voice quality guardrails
Document package-specific React Native best practices and add lint warnings so state, effect, and complexity issues surface earlier during mobile-voice work.
This commit is contained in:
Ryan Vogel
@@ -11,7 +11,7 @@ This file defines package-specific guidance for agents working in `packages/mobi
|
||||
## Project Overview
|
||||
|
||||
- Expo + React Native app for voice dictation and OpenCode session monitoring.
|
||||
- Uses native modules (`react-native-executorch`, `react-native-audio-api`, `expo-notifications`, `expo-camera`).
|
||||
- Uses native/device-heavy modules such as `whisper.rn`, `react-native-audio-api`, `expo-notifications`, and `expo-camera`.
|
||||
- Development builds are required for native module changes.
|
||||
|
||||
## Commands
|
||||
@@ -32,7 +32,7 @@ Run all commands from `packages/mobile-voice`.
|
||||
|
||||
- For JS-only changes: run `bun run lint` and verify app behavior via dev client.
|
||||
- For native dependency/config/plugin changes: rebuild dev client via EAS before validation.
|
||||
- If notifications/camera/audio behavior changes, verify on a physical iOS device.
|
||||
- If notifications, camera, microphone, or audio-session behavior changes, verify on a physical iOS device.
|
||||
- Do not claim a fix unless you validated in Metro logs and app runtime behavior.
|
||||
|
||||
## Single-Test Guidance
|
||||
@@ -43,55 +43,86 @@ Run all commands from `packages/mobile-voice`.
|
||||
- `bunx expo export --platform ios --clear`
|
||||
- manual runtime test in dev client
|
||||
|
||||
## Architecture Priorities
|
||||
|
||||
- Keep screens focused on composition and orchestration. Once a screen owns multiple workflows, extract hooks/components before adding more local state.
|
||||
- Prefer extracting pure helpers and config objects before introducing new stores or abstractions.
|
||||
- Treat `src/app/index.tsx` as a composition root, not as the permanent home for onboarding, dictation, monitoring, pairing, persistence, and all UI details.
|
||||
- Avoid mirrored `state + ref` pairs unless they are needed for imperative native APIs, race cancellation, or subscription callbacks.
|
||||
|
||||
## Code Style And Patterns
|
||||
|
||||
### Formatting / Structure
|
||||
|
||||
- Preserve existing style (`semi: false`, concise JSX, stable import grouping).
|
||||
- Keep UI changes localized; avoid large architectural rewrites.
|
||||
- Avoid unrelated formatting churn.
|
||||
- Keep UI changes localized and behavior-preserving; avoid unrelated formatting churn.
|
||||
- Prefer feature-adjacent hooks/components over growing a single screen file.
|
||||
|
||||
### React State / Effects
|
||||
|
||||
- Effects are for subscriptions, timers, persistence, network I/O, and native bridge setup/cleanup.
|
||||
- Do not add `useEffect` just to derive render data from props or state. Derive during render instead.
|
||||
- Prefer one source of truth. If a value can be computed from existing state, do not store it separately.
|
||||
- Use `useMemo` only when computation is expensive or stable identity actually matters.
|
||||
- Use `useCallback` only when stable function identity matters for dependencies, cleanup, or memoized children.
|
||||
- When UI branches are driven by a small finite state, prefer config tables/objects over long nested ternaries.
|
||||
|
||||
### Types
|
||||
|
||||
- Avoid `any`; prefer local type aliases for component state and network payloads.
|
||||
- Keep exported/shared boundaries typed explicitly.
|
||||
- Parse persisted and network payloads as `unknown` first, then validate before use.
|
||||
- Use discriminated unions for UI modes/status where practical.
|
||||
|
||||
### Naming
|
||||
|
||||
- Prefer short, readable names consistent with nearby code.
|
||||
- Keep naming aligned with existing app state keys (`serverDraftURL`, `monitorStatus`, etc.).
|
||||
- Keep naming aligned with existing app state keys (`monitorStatus`, `activeSessionId`, etc.).
|
||||
|
||||
### Error Handling / Logging
|
||||
|
||||
- Fail gracefully in UI (alerts, disabled actions, fallback text).
|
||||
- Log actionable diagnostics for runtime workflows:
|
||||
- server health checks
|
||||
- relay registration attempts
|
||||
- notification token lifecycle
|
||||
- Avoid bare `catch {}` or `.catch(() => {})` for meaningful work. If failure is intentionally best-effort, leave a short comment or use a helper that makes that explicit.
|
||||
- Log actionable diagnostics for runtime workflows such as server health checks, relay registration, and notification token lifecycle.
|
||||
- Never log secrets or full APNs tokens.
|
||||
- Keep hot-path logging behind `__DEV__` when possible.
|
||||
|
||||
### Network / Relay Integration
|
||||
|
||||
- Normalize and validate URLs before storing server configs.
|
||||
- Use `AbortController` or request IDs for overlapping requests, streams, and polling.
|
||||
- Keep relay registration idempotent.
|
||||
- Guard duplicate scan/add flows to avoid repeated server entries.
|
||||
|
||||
### Notifications / APNs
|
||||
|
||||
- Distinguish sandbox vs production token environments correctly.
|
||||
- This package currently assumes APNs relay registration uses the `production` environment only. Do not add environment switching unless explicitly requested.
|
||||
- On registration changes, ensure old token unregister flow remains intact.
|
||||
- Treat permission failures as non-fatal and degrade to foreground monitoring when needed.
|
||||
|
||||
### Performance / RN
|
||||
|
||||
- Validate performance-sensitive changes in a dev client or release build, not only Metro dev mode.
|
||||
- During recording and monitoring flows, keep JS-thread work light.
|
||||
- Prefer Reanimated/native-thread-friendly animations for motion.
|
||||
- For small menus a `ScrollView` is fine; if a list grows beyond a small bounded menu, move to `FlatList` or `FlashList`.
|
||||
|
||||
## Lint / Quality Bar
|
||||
|
||||
- Keep hooks lint warnings clean before finishing.
|
||||
- Treat `any`, `no-console`, complexity, and max-lines warnings as refactor prompts, not noise to suppress.
|
||||
- Do not disable React Hooks lint rules inline unless there is a documented native-interop reason.
|
||||
- When introducing new persistence or network payloads, add or reuse a parser instead of scattering casts.
|
||||
|
||||
## Native-Module Safety
|
||||
|
||||
- If adding a native module, ensure it is in `package.json` with SDK-compatible version.
|
||||
- Rebuild dev client after native module additions/changes.
|
||||
- If adding a native module, ensure it is in `package.json` with an SDK-compatible version.
|
||||
- Rebuild the dev client after native module additions or changes.
|
||||
- For optional native capability usage, prefer runtime fallback paths instead of hard crashes.
|
||||
|
||||
## Common Pitfalls
|
||||
|
||||
- Black screen + "No script URL provided" often means stale dev client binary.
|
||||
- Black screen + "No script URL provided" often means a stale dev client binary.
|
||||
- `expo-doctor` duplicate module warnings may appear in Bun workspaces; prioritize runtime verification.
|
||||
- `expo lint` may auto-generate `eslint.config.js`; do not commit accidental generated config unless requested.
|
||||
|
||||
|
||||
@@ -1,10 +1,52 @@
|
||||
// https://docs.expo.dev/guides/using-eslint/
|
||||
const { defineConfig } = require('eslint/config');
|
||||
const expoConfig = require("eslint-config-expo/flat");
|
||||
const { defineConfig } = require("eslint/config")
|
||||
const tsGuard = require("@typescript-eslint/eslint-plugin")
|
||||
const expoConfig = require("eslint-config-expo/flat")
|
||||
const reactHooksNext = require("eslint-plugin-react-hooks")
|
||||
|
||||
module.exports = defineConfig([
|
||||
expoConfig,
|
||||
{
|
||||
ignores: ["dist/*"],
|
||||
}
|
||||
]);
|
||||
},
|
||||
{
|
||||
files: ["**/*.{ts,tsx}"],
|
||||
languageOptions: {
|
||||
parserOptions: {
|
||||
projectService: true,
|
||||
tsconfigRootDir: __dirname,
|
||||
},
|
||||
},
|
||||
plugins: {
|
||||
"react-hooks-next": reactHooksNext,
|
||||
"ts-guard": tsGuard,
|
||||
},
|
||||
rules: {
|
||||
"ts-guard/no-explicit-any": "warn",
|
||||
"ts-guard/no-floating-promises": "warn",
|
||||
complexity: ["warn", 20],
|
||||
"max-lines": [
|
||||
"warn",
|
||||
{
|
||||
max: 1200,
|
||||
skipBlankLines: true,
|
||||
skipComments: true,
|
||||
},
|
||||
],
|
||||
"max-lines-per-function": [
|
||||
"warn",
|
||||
{
|
||||
max: 250,
|
||||
skipBlankLines: true,
|
||||
skipComments: true,
|
||||
},
|
||||
],
|
||||
"no-console": ["warn", { allow: ["warn", "error"] }],
|
||||
"no-nested-ternary": "warn",
|
||||
"react-hooks/exhaustive-deps": "error",
|
||||
"react-hooks-next/refs": "warn",
|
||||
"react-hooks-next/set-state-in-effect": "warn",
|
||||
"react-hooks-next/static-components": "warn",
|
||||
},
|
||||
},
|
||||
])
|
||||
|
||||
@@ -51,8 +51,11 @@
|
||||
"whisper.rn": "0.5.5"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@typescript-eslint/eslint-plugin": "^8.57.2",
|
||||
"@typescript-eslint/parser": "^8.57.2",
|
||||
"@types/react": "~19.2.2",
|
||||
"babel-preset-expo": "~55.0.8",
|
||||
"eslint-plugin-react-hooks": "^7.0.1",
|
||||
"typescript": "~5.9.2"
|
||||
},
|
||||
"private": true
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
# Mobile Voice Refactor Plan
|
||||
|
||||
## Goals
|
||||
|
||||
- Reduce the surface area of `src/app/index.tsx` without changing product behavior.
|
||||
- Make device, network, and monitoring flows easier to reason about.
|
||||
- Move toward React Native / Expo best practices for state, effects, and file structure.
|
||||
- Use the new lint warnings as refactor prompts, not as permanent background noise.
|
||||
|
||||
## Current Pain Points
|
||||
|
||||
- `DictationScreen` currently owns onboarding, permissions, Whisper/model lifecycle, dictation, pairing, server/session sync, relay registration, notification handling, and most UI rendering.
|
||||
- The screen mixes render-time derived state, imperative refs, polling, persistence, and native cleanup in one place.
|
||||
- There are many nested conditionals and long derived blocks that are hard to scan.
|
||||
- Best-effort async cleanup and silent catches make failures harder to understand.
|
||||
|
||||
## Target Shape
|
||||
|
||||
- `src/app/index.tsx`
|
||||
- compose hooks and presentational sections
|
||||
- keep only screen-level orchestration
|
||||
- `src/features/onboarding/`
|
||||
- onboarding step config
|
||||
- onboarding UI component
|
||||
- `src/features/dictation/`
|
||||
- `use-whisper-dictation`
|
||||
- transcript helpers
|
||||
- `src/features/servers/`
|
||||
- server/session refresh and pairing helpers
|
||||
- persisted server state helpers
|
||||
- `src/features/monitoring/`
|
||||
- foreground SSE monitoring
|
||||
- notification payload handling
|
||||
- relay registration helpers
|
||||
- `src/lib/`
|
||||
- parser/validation helpers
|
||||
- logger helper for dev-only diagnostics
|
||||
|
||||
## Refactor Order
|
||||
|
||||
### Phase 1: Extract pure helpers first
|
||||
|
||||
- Move onboarding step text/style selection into a config object or array.
|
||||
- Move server/session payload parsing into dedicated helpers.
|
||||
- Keep existing behavior and props the same.
|
||||
|
||||
### Phase 2: Extract onboarding UI
|
||||
|
||||
- Create an `OnboardingFlow` component that receives explicit state and handlers.
|
||||
- Keep onboarding persistence in the screen until the UI extraction is stable.
|
||||
|
||||
### Phase 3: Extract dictation logic
|
||||
|
||||
- Move Whisper loading, recording, bulk/realtime transcription, and waveform state into a `useWhisperDictation` hook.
|
||||
- Expose a small interface: recording state, transcript, actions, and model status.
|
||||
|
||||
### Phase 4: Extract server/session management
|
||||
|
||||
- Move server restore/save, pairing, health refresh, and active server/session selection into a dedicated hook.
|
||||
- Centralize server parsing and dedupe logic.
|
||||
|
||||
### Phase 5: Extract monitoring and notifications
|
||||
|
||||
- Move SSE monitoring, push payload handling, and relay registration into a `useMonitoring` hook.
|
||||
- Keep side effects close to the feature that owns them.
|
||||
|
||||
### Phase 6: Lint burn-down
|
||||
|
||||
- Replace `any` with explicit parsed shapes.
|
||||
- Reduce nested ternaries in favor of config tables.
|
||||
- Replace ad hoc `console.log` calls with a logger helper or `__DEV__`-gated diagnostics.
|
||||
- Audit bare `.catch(() => {})` and convert non-trivial cases to explicit best-effort helpers or real error handling.
|
||||
|
||||
## Guardrails During Refactor
|
||||
|
||||
- Keep one behavior-preserving slice per PR.
|
||||
- Do not introduce more derived state in `useEffect`.
|
||||
- Prefer explicit hook inputs/outputs over hidden cross-hook coupling.
|
||||
- Only use refs for imperative APIs, subscriptions, and race control.
|
||||
- Re-run lint after each slice.
|
||||
- Validate app behavior in the dev client for microphone, notifications, pairing, and monitoring flows.
|
||||
|
||||
## Exit Criteria
|
||||
|
||||
- `src/app/index.tsx` is mostly screen composition and stays under roughly 800-1200 lines.
|
||||
- Feature logic lives in focused hooks/components with clearer ownership.
|
||||
- New payload parsing does not rely on `any`.
|
||||
- Lint warnings trend down instead of growing.
|
||||
Reference in New Issue
Block a user