From 39fbd943ffef821bbeac6738ef6434e72becf4b2 Mon Sep 17 00:00:00 2001 From: null Date: Mon, 6 Jul 2026 21:36:59 -0500 Subject: [PATCH] docs: standardization & modernization review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Grounded review of the Android app / functions / build with measured inconsistencies, what was standardized this session (collectAsStateWithLifecycle; Firebase Tasks→await subset), and risk-assessed recommendations for the larger items (finish Tasks→await, version catalog, ViewModel Firebase extraction, Functions v2, color tokens, TS types). Co-Authored-By: Claude Fable 5 --- docs/standardization-review.md | 65 ++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 docs/standardization-review.md diff --git a/docs/standardization-review.md b/docs/standardization-review.md new file mode 100644 index 00000000..a1758c91 --- /dev/null +++ b/docs/standardization-review.md @@ -0,0 +1,65 @@ +# Code standardization & modernization review (2026-07-06) + +A grounded pass over the Android app (313 Kotlin files), Cloud Functions (TypeScript), and build. Each +item is a *real, measured* inconsistency (with counts), not generic advice. Applied = done + verified +this session; the rest are prioritized by value ÷ risk so the riskier ones can be greenlit deliberately. + +## Applied (done + verified) + +1. **Compose state collection unified on `collectAsStateWithLifecycle`** — was 55 files on + `collectAsState()` vs 2 on the lifecycle-aware variant; now all 57. Added + `androidx.lifecycle:lifecycle-runtime-compose`. Upstream flow collection pauses when the UI isn't + visible. Commit `61a8609`. +2. **Firebase Tasks → `.await()` (safe subset)** — `FirestoreOutcomeDataSource` (mixed both styles in + one file), `FirestoreCoupleDataSource`, `PlayIntegrityChecker`. Commit `7507ca6`. + +## Recommended next — LOW risk, high value + +3. **Finish Firebase Tasks → `.await()`** across the remaining ~11 data sources + (`FirestoreAnswerDataSource` 11 blocks, `FirestoreUserDataSource` 13, `FirebaseAuthDataSource` 13, + `FirestoreConversationDataSource`, `…DateSwipe/DatePlan/DateReflection/DateMemory/QuestionThread/ + BucketList/DateMatch`, `FirebaseStorageDataSource`). ~72 blocks remain. **Per-block review required** + — these files mix `callbackFlow` snapshot listeners (leave as-is) and one transaction + (`FirestoreDateMatchDataSource`) with the convertible one-shot Tasks. Watch for the failure→null + idiom (see `PlayIntegrityChecker.verifyWithServer`: use `runCatching{ …await() }.getOrNull()`, not a + bare `await()`). Not unit-tested (live Firebase I/O), so do it incrementally with a smoke per file, + not a bulk sed. + +## Recommended — MEDIUM risk / churn + +4. **Gradle version catalog (`gradle/libs.versions.toml`)** — 57 dependency versions are hardcoded + across the build files. The modern Gradle standard centralizes versions + enables the `libs.*` + accessors. Build-only, fully verifiable (build must pass), but pure mechanical churn with typo risk. +5. **`FirebaseFirestore`/`FirebaseFunctions` injected directly into 9 ViewModels/screens** + (`HomeViewModel`, `DailyQuestionViewModel`, `QuestionThreadViewModel`, `BucketListViewModel`, + `DateBuilderViewModel`, `MessagesInboxViewModel`, `ConversationViewModel`, `PartnerHomeScreen`, + `DeleteAccountScreen`) — an architectural leak past the repository/data-source layer that the rest of + the app respects. `HomeViewModel` even runs raw Firestore `addSnapshotListener`s. Worth extracting to + data sources, but higher-risk (touches live listeners) and best done one ViewModel at a time. + +## Recommended — HIGHER risk, do deliberately + +6. **~175 hardcoded `Color(0xFF…)` literals in `ui/`** vs. theme tokens. A theme-token migration is the + right modern Compose practice, but carries visual-regression risk; the repo already has + `scripts/theme-scan.sh` and prior brand work, so pair any migration with that scanner + screenshot + checks. Do per-screen, not en masse. +7. **Cloud Functions on the v1 API** (`functions.https.onCall`, `functions.pubsub.schedule`, all 30 + imports `from 'firebase-functions'`). Firebase Functions **v2** (`firebase-functions/v2/*`) is the + modern target (better cold-start, concurrency, typed params). Migration changes signatures + deploy + config and touches the billing webhook / callables — meaningful risk; do as its own project with + sandbox verification, not bundled. +8. **11 `: any` types in TypeScript** (e.g. `onCall(async (data: any, …))`) — tighten to typed request + shapes. Low risk, low urgency; fold into the v2 migration if/when it happens. + +## Healthy already (no action) + +- **Material3 only** (86 files, zero Material2 imports). +- **No `GlobalScope`**; scoping via `viewModelScope`/`lifecycleScope` throughout. +- Only **9 `!!`** non-null assertions app-wide. +- Analytics hashing single-sourced (`AnalyticsHashing`, this session); DI split of `@Binds`/`@Provides` + modules is idiomatic (not an inconsistency). + +## Suggested order + +3 (finish `.await()`) → 4 (version catalog) → 5 (ViewModel Firebase extraction, per-VM) → 7 (Functions +v2, standalone) → 6 (colors, per-screen) → 8 (TS types, with #7).