fix(docs): review pass on mobile SDK best practices — correct 6 bugs found via codebase verification

This commit is contained in:
saravanakumardb1 2026-03-01 07:45:39 -08:00
parent d10b898ba5
commit 137266a284
2 changed files with 99 additions and 48 deletions

View File

@ -34,7 +34,7 @@ This document proposes creating a **Kotlin Platform SDK** (`kotlin-platform-sdk`
| Component | LysnrAI | ChronoMind | MindLyst | Diff Between Copies |
| ---------------------- | --------------------------- | ------------------------------------ | ------------------------------ | --------------------------------- |
| **AuthService** | ✅ 300+ LOC (OkHttp) | ✅ 344 LOC (HttpURLConnection, Hilt) | ✅ 232 LOC (HttpURLConnection) | productId, prefs name, DI style |
| **TelemetryService** | ⚠️ Presumed | ✅ 178 LOC (Hilt @Singleton) | ✅ 171 LOC (plain class) | productId, prefs name, class name |
| **TelemetryService** | ❌ Not yet | ✅ 178 LOC (Hilt @Singleton) | ✅ 171 LOC (plain class) | productId, prefs name, class name |
| **FeatureFlagService** | ✅ 98 LOC (OkHttp, object) | ✅ 89 LOC (Hilt @Singleton) | ✅ similar | HTTP client, DI style |
| **KillSwitchService** | ✅ 64 LOC (OkHttp, object) | ❌ Not yet | ❌ Not yet | — |
| **BlobService** | ✅ 150 LOC (OkHttp, object) | ❌ Not yet | ❌ Not yet | — |
@ -272,10 +272,21 @@ LysnrAI has the most platform files (12) and uses OkHttp throughout.
```kotlin
// packages/kotlin-platform-sdk/build.gradle.kts
plugins {
kotlin("jvm") version "2.1.0"
id("com.android.library")
kotlin("android") version "2.1.0"
kotlin("plugin.serialization") version "2.1.0"
}
android {
namespace = "com.bytelyst.platform"
compileSdk = 35
defaultConfig { minSdk = 26 }
compileOptions {
sourceCompatibility = JavaVersion.VERSION_17
targetCompatibility = JavaVersion.VERSION_17
}
}
group = "com.bytelyst.platform"
version = "0.1.0"
@ -284,16 +295,24 @@ dependencies {
implementation("org.jetbrains.kotlinx:kotlinx-serialization-json:1.7.3")
implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.9.0")
// Android-specific (optional, for SecureStore and BiometricAuth)
compileOnly("androidx.security:security-crypto:1.1.0-alpha06")
compileOnly("androidx.biometric:biometric:1.2.0-alpha05")
// Android-specific (SecureStore needs Keystore-backed prefs, BiometricAuth needs AndroidX)
implementation("androidx.security:security-crypto:1.0.0")
implementation("androidx.biometric:biometric:1.1.0")
implementation("androidx.core:core-ktx:1.15.0")
testImplementation(kotlin("test"))
testImplementation("com.squareup.okhttp3:mockwebserver:4.12.0")
testImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.9.0")
testImplementation("org.robolectric:robolectric:4.14.1")
}
```
> **Why `com.android.library` instead of `kotlin("jvm")`?** Several SDK components require
> `android.content.Context` (SecureStore, TelemetryClient, BiometricAuth). A pure JVM module
> cannot access Android framework classes. If you need a pure-Kotlin subset (e.g., for KMP),
> split into `kotlin-platform-core` (JVM: PlatformConfig, models, HTTP client) and
> `kotlin-platform-android` (Android library: SecureStore, BiometricAuth, TelemetryClient).
### 5.2 Consumer App settings.gradle.kts
```kotlin
@ -382,6 +401,6 @@ Or create a single `BLDesignTokens.kt` with product-specific color palette selec
| **OkHttp version conflict** | SDK declares OkHttp as `api` dependency; consumers use same version |
| **Hilt vs Koin** | SDK uses constructor injection (no DI annotations); each app wires its own way |
| **EncryptedSharedPreferences on API < 23** | `minSdk = 26` across all apps; not an issue |
| **Breaking changes** | Semantic versioning; test in ChronoMind first (smallest Android app) |
| **Breaking changes** | Semantic versioning; test in ChronoMind first (cleanest Hilt DI structure) |
| **Gradle composite build issues** | Document exact `includeBuild()` + `dependencySubstitution` setup |
| **CI builds without sibling repo** | Pack SDK into `.aar` tarball for CI (same pattern as Swift Package + docker-prep.sh) |

View File

@ -38,23 +38,27 @@ The **ByteLystPlatformSDK** is a shared Swift Package that provides 13 platform-
## 3. App Adoption Matrix
| Component | LysnrAI | ChronoMind | PeakPulse | JarvisJr | MindLyst |
| --------------------- | --------------------- | ---------------------- | ---------------------- | ---------------------- | ------------------------ |
| `BLPlatformConfig` | ✅ Config.swift | ✅ (inline) | ✅ Config.swift | ✅ Config.swift | ⚠️ Not yet |
| `BLPlatformClient` | ✅ (internal) | ✅ (internal) | ✅ (internal) | ✅ (internal) | ⚠️ PlatformServiceClient |
| `BLKeychain` | ✅ KeychainHelper | ✅ KeychainHelper | ✅ KeychainHelper | ✅ KeychainHelper | ✅ KeychainHelper |
| `BLTelemetryClient` | ✅ TelemetryService | ✅ TelemetryService | ✅ TelemetryService | ✅ TelemetryService | ✅ TelemetryService |
| `BLAuthClient` | ✅ AuthService | ✅ AuthService | ✅ AuthService | ✅ AuthService | ✅ AuthService |
| `BLFeatureFlagClient` | ✅ FeatureFlagService | ✅ FeatureFlagService | ✅ FeatureFlagService | ✅ FeatureFlagService | ✅ FeatureFlagService |
| `BLSyncEngine` | ✅ CloudSyncService | ✅ PlatformSyncManager | ✅ PlatformSyncManager | ✅ PlatformSyncManager | ⚠️ Not yet |
| `BLBlobClient` | ✅ BlobService | ❌ Not needed | ❌ Phase 2 | ❌ Phase 2 | ❌ Phase 2 |
| `BLKillSwitchClient` | ✅ KillSwitchService | ❌ Not yet | ✅ KillSwitchService | ✅ KillSwitchService | ❌ Not yet |
| `BLLicenseClient` | ✅ LicenseService | ❌ N/A | ❌ N/A | ❌ N/A | ❌ N/A |
| `BLBiometricAuth` | ✅ BiometricAuth | ❌ Not yet | ❌ Phase 3 | ❌ Phase 3 | ❌ Phase 3 |
| `BLCrashReporter` | ❌ Not yet | ✅ CrashReporter | ✅ CrashReporter | ✅ CrashReporter | ❌ Not yet |
| `BLAuditLogger` | ✅ AuditLogger | ❌ Not yet | ❌ Phase 2 | ❌ Phase 2 | ❌ Not yet |
| Component | LysnrAI | ChronoMind | PeakPulse | JarvisJr | MindLyst |
| --------------------- | -------------------------------------------------- | ----------------------- | ---------------------- | ---------------------- | -------------------------- |
| `BLPlatformConfig` | ⚠️ Config.swift (env loader, not BLPlatformConfig) | ✅ (inline in wrappers) | ✅ Config.swift | ✅ Config.swift | ✅ (inline in AuthService) |
| `BLPlatformClient` | ⚠️ Not used (raw URLSession) | ✅ (internal) | ✅ (internal) | ✅ (internal) | ✅ (internal) |
| `BLKeychain` | ✅ KeychainHelper | ✅ KeychainHelper | ✅ KeychainHelper | ✅ KeychainHelper | ✅ KeychainHelper |
| `BLTelemetryClient` | ✅ TelemetryService | ✅ TelemetryService | ✅ TelemetryService | ✅ TelemetryService | ✅ TelemetryService |
| `BLAuthClient` | ✅ AuthService | ✅ AuthService | ✅ AuthService | ✅ AuthService | ✅ AuthService |
| `BLFeatureFlagClient` | ✅ FeatureFlagService | ✅ FeatureFlagService | ✅ FeatureFlagService | ✅ FeatureFlagService | ✅ FeatureFlagService |
| `BLSyncEngine` | ✅ CloudSyncService | ✅ PlatformSyncManager | ✅ PlatformSyncManager | ✅ PlatformSyncManager | ⚠️ Not yet |
| `BLBlobClient` | ✅ BlobService | ❌ Not needed | ❌ Phase 2 | ❌ Phase 2 | ❌ Phase 2 |
| `BLKillSwitchClient` | ✅ KillSwitchService | ❌ Not yet | ✅ KillSwitchService | ✅ KillSwitchService | ❌ Not yet |
| `BLLicenseClient` | ✅ LicenseService | ❌ N/A | ❌ N/A | ❌ N/A | ❌ N/A |
| `BLBiometricAuth` | ✅ BiometricAuth | ❌ Not yet | ❌ Phase 3 | ❌ Phase 3 | ❌ Phase 3 |
| `BLCrashReporter` | ❌ Not yet | ✅ CrashReporter | ✅ CrashReporter | ✅ CrashReporter | ❌ Not yet |
| `BLAuditLogger` | ✅ AuditLogger | ❌ Not yet | ❌ Phase 2 | ❌ Phase 2 | ❌ Not yet |
**Legend:** ✅ = migrated to SDK wrapper | ⚠️ = hand-rolled (needs migration) | ❌ = not used / future phase
**Legend:** ✅ = migrated to SDK wrapper (path shown) | ⚠️ = hand-rolled (needs migration) | ❌ = not used / future phase
> **Note on directory conventions:** PeakPulse and JarvisJr use the recommended `Platform/` directory.
> ChronoMind uses `Shared/Cloud/` + `Shared/Diagnostics/`. LysnrAI uses `Auth/` + `Util/`.
> MindLyst uses `Services/`. All should ideally standardize on `Platform/` (see Section 6).
---
@ -173,40 +177,68 @@ ios/
## 6. Gap Analysis & Remediation
### 6.1 MindLyst iOS — Needs Migration
### 6.1 MindLyst iOS — Directory Rename Only
MindLyst iOS (`iosApp/Services/`) still has hand-rolled platform files not using the SDK:
MindLyst iOS (`iosApp/Services/`) **already uses ByteLystPlatformSDK** — all 4 platform wrappers
(`KeychainHelper`, `TelemetryService`, `AuthService`, `FeatureFlagService`) correctly import and
delegate to SDK components. The only action needed is a directory rename for consistency:
| File | Status | Action |
| --------------------------------------------- | -------------------------- | ----------------------------- |
| `iosApp/Services/KeychainHelper.swift` | ⚠️ Hand-rolled | Migrate to SDK wrapper |
| `iosApp/Services/TelemetryService.swift` | ⚠️ Hand-rolled | Migrate to SDK wrapper |
| `iosApp/Services/AuthService.swift` | ⚠️ Hand-rolled | Migrate to SDK wrapper |
| `iosApp/Services/FeatureFlagService.swift` | ⚠️ Hand-rolled | Migrate to SDK wrapper |
| `iosApp/Services/PlatformServiceClient.swift` | ⚠️ Hand-rolled HTTP client | Replace with BLPlatformClient |
| `iosApp/MindLystTheme.swift` | ✅ Auto-generated | Keep as-is (design tokens) |
| Current Location | Recommended Location | Status |
| --------------------------------------------- | ------------------------------------------ | -------------------------------------------- |
| `iosApp/Services/KeychainHelper.swift` | `iosApp/Platform/KeychainHelper.swift` | ✅ SDK wrapper (move only) |
| `iosApp/Services/TelemetryService.swift` | `iosApp/Platform/TelemetryService.swift` | ✅ SDK wrapper (move only) |
| `iosApp/Services/AuthService.swift` | `iosApp/Platform/AuthService.swift` | ✅ SDK wrapper (move only) |
| `iosApp/Services/FeatureFlagService.swift` | `iosApp/Platform/FeatureFlagService.swift` | ✅ SDK wrapper (move only) |
| `iosApp/Services/PlatformServiceClient.swift` | Keep in `Services/` | App-specific API client (not platform infra) |
**Effort:** ~2 hours. Create `iosApp/Platform/` directory, add 5 thin wrappers, delete hand-rolled files.
`PlatformServiceClient.swift` is an **app-specific** API client for MindLyst memory items and blob
uploads (259 LOC) — it does NOT duplicate SDK functionality and should remain in `Services/`.
### 6.2 LysnrAI iOS — Directory Rename
**Effort:** ~30 minutes. Move 4 files to `Platform/`, update Xcode project references.
LysnrAI wrappers are split across `Auth/` and `Util/` instead of a unified `Platform/` directory:
**Missing SDK wrappers for MindLyst (future phases):**
| Current Location | Should Be |
| --------------------------------------- | ------------------------------------------- |
| `LysnrAI/Auth/KeychainHelper.swift` | `LysnrAI/Platform/KeychainHelper.swift` |
| `LysnrAI/Auth/AuthService.swift` | `LysnrAI/Platform/AuthService.swift` |
| `LysnrAI/Auth/BiometricAuth.swift` | `LysnrAI/Platform/BiometricAuth.swift` |
| `LysnrAI/Util/TelemetryService.swift` | `LysnrAI/Platform/TelemetryService.swift` |
| `LysnrAI/Util/FeatureFlagService.swift` | `LysnrAI/Platform/FeatureFlagService.swift` |
| `LysnrAI/Util/KillSwitchService.swift` | `LysnrAI/Platform/KillSwitchService.swift` |
| `LysnrAI/Util/LicenseService.swift` | `LysnrAI/Platform/LicenseService.swift` |
| `LysnrAI/Util/BlobService.swift` | `LysnrAI/Platform/BlobService.swift` |
| `LysnrAI/Util/AuditLogger.swift` | `LysnrAI/Platform/AuditLogger.swift` |
- `BLPlatformConfig` — currently created inline in `AuthService.init()`, should extract to `Platform/Config.swift`
- `BLKillSwitchClient` — not yet adopted
- `BLCrashReporter` — not yet adopted
- `BLSyncEngine` — not yet adopted
**Effort:** ~1 hour. Move files, update Xcode project references.
### 6.2 LysnrAI iOS — Config.swift + Directory Rename
### 6.3 Missing SDK Features
LysnrAI's `Config.swift` is a hand-rolled `.env` file loader (reads `~/.lysnrai/.env`). It does
**NOT** use `BLPlatformConfig`. The SDK wrappers themselves are correct (all 9 import
`ByteLystPlatformSDK`) but they construct `BLPlatformConfig` inline instead of reading from a
central `AppConfig`.
**Actions:**
1. Create `Platform/Config.swift` using `BLPlatformConfig` (reading baseURL from existing `Config.platformServiceURL`)
2. Move all 9 SDK wrappers from `Auth/` and `Util/` to `Platform/`
3. Keep existing `Config.swift` (rename to `EnvConfig.swift`) for Azure Speech/OpenAI env vars
| Current Location | Recommended Location |
| --------------------------------------- | -------------------------------------------------- |
| `LysnrAI/Config.swift` | `LysnrAI/EnvConfig.swift` (keep, rename) |
| — (new) | `LysnrAI/Platform/Config.swift` (BLPlatformConfig) |
| `LysnrAI/Auth/KeychainHelper.swift` | `LysnrAI/Platform/KeychainHelper.swift` |
| `LysnrAI/Auth/AuthService.swift` | `LysnrAI/Platform/AuthService.swift` |
| `LysnrAI/Auth/BiometricAuth.swift` | `LysnrAI/Platform/BiometricAuth.swift` |
| `LysnrAI/Util/TelemetryService.swift` | `LysnrAI/Platform/TelemetryService.swift` |
| `LysnrAI/Util/FeatureFlagService.swift` | `LysnrAI/Platform/FeatureFlagService.swift` |
| `LysnrAI/Util/KillSwitchService.swift` | `LysnrAI/Platform/KillSwitchService.swift` |
| `LysnrAI/Util/LicenseService.swift` | `LysnrAI/Platform/LicenseService.swift` |
| `LysnrAI/Util/BlobService.swift` | `LysnrAI/Platform/BlobService.swift` |
| `LysnrAI/Util/AuditLogger.swift` | `LysnrAI/Platform/AuditLogger.swift` |
**Effort:** ~1.5 hours. Move 9 files, create Config.swift, rename existing Config, update Xcode project.
### 6.3 ChronoMind iOS — Directory Convention (Optional)
ChronoMind wrappers are in `Shared/Cloud/` and `Shared/Diagnostics/` — all correctly use the SDK.
This structure works because `Shared/` is consumed by watchOS and macOS targets. Moving to `Platform/`
is optional; the cross-platform `Shared/` directory is valid for multi-target apps.
### 6.4 Missing SDK Features
| Feature | Description | Priority |
| ---------------------- | ------------------------------------------------------------- | -------- |