chore: KMP audit — commonize code, centralize utilities, eliminate dead abstractions#5133
Merged
jamesarich merged 21 commits intomainfrom Apr 15, 2026
Merged
chore: KMP audit — commonize code, centralize utilities, eliminate dead abstractions#5133jamesarich merged 21 commits intomainfrom
jamesarich merged 21 commits intomainfrom
Conversation
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
jamesarich
added a commit
that referenced
this pull request
Apr 14, 2026
- testing-ci: remove ./gradlew clean, consolidate to single invocation - project-overview: add step 3 (init secrets) to workspace bootstrap - kmp-common: add safeCatching and CancellationException import rules - code-review: add coroutine safety, HttpClientDefaults, Room patterns - kmp-architecture: add CoroutineDispatchers injection, safeCatching, HttpClientDefaults, Room @Upsert/LIMIT 1/N+1 batching patterns - compose-ui: add @Preview in commonMain, rememberSaveable for dialogs, expand NumberFormatter float formatting example - implement-feature: add spotlessApply to baseline verification Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3 tasks
## Compose Multiplatform - KmpFeatureConventionPlugin: move compose-multiplatform-ui-tooling-preview to commonMain (CMP 1.11 makes @Preview available in all source sets); remove duplicate compose-multiplatform-material3 from androidMain - Move messaging preview files (MessageItemPreviews, ReactionPreviews, QuickChatPreviews) from androidMain → commonMain — all imports are KMP-safe and use the canonical androidx.compose.ui.tooling.preview.* imports ## Coil 3.4.0 - Add DeDupeConcurrentRequestStrategy to KtorNetworkFetcherFactory on both Android and Desktop — deduplicates concurrent requests for the same URL (important for device hardware images shown in multiple list items) - Add memoryCacheMaxSizePercentWhileInBackground(0.1) to Android ImageLoader to proactively free memory when the app is in the background ## Ktor 3.4.2 - Add DefaultRequest plugin to both Android and Desktop HttpClient with API_BASE_URL from HttpClientDefaults — centralises the base URL - Update ApiServiceImpl to use relative paths (resource/deviceHardware, github/firmware/list) instead of full hardcoded URLs - Standardise debug logging level: Desktop was using LogLevel.HEADERS, now matches Android at LogLevel.BODY ## Room KMP - MeshLogDao: fix deprecated LIMIT 0,:maxItem syntax → LIMIT :maxItem in all three affected queries (getAllLogs, getAllLogsInReceiveOrder, getLogsFrom) - PacketDao: replace in-memory getQueuedPackets() filter with a direct @query using json_extract(data, '$.status') = 'QUEUED' — eliminates full table scan; update PacketRepository/PacketRepositoryImpl signatures and remove redundant ?: emptyList() at call site ## Koin / Desktop - DesktopPlatformModule: store DesktopProcessLifecycleOwner in a val before returning its lifecycle — guards against potential GC if LifecycleRegistry holds only a weak reference to its owner ## Lifecycle / ViewModel - FirmwareUpdateViewModel.onCleared(): wrap NonCancellable cleanup in CoroutineScope(SupervisorJob()) so the short-lived scope has a proper Job for exception handling/tracking Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ance Add indexes to PacketEntity for received_time, filtered, and read columns that are frequently used in WHERE/ORDER BY clauses across PacketDao queries. Add index to NodeEntity for public_key used in findNodeByPublicKey lookups. Bump database version 37 → 38 with AutoMigration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace per-node getVerifiedNodeForUpsert() calls with a batch getVerifiedNodesForUpsert() that pre-fetches all existing nodes and public-key conflicts in two queries (chunked at 999 bind params), then processes verification logic in-memory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rvices - Replace read-then-write race in setMuteUntil with INSERT OR IGNORE + atomic UPDATE — eliminates concurrent data corruption risk. - Replace Dispatchers.IO with injected CoroutineDispatchers in AndroidFileService, JvmFileService, and JvmServiceDiscovery for KMP consistency. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove duplicate SearchMatch/SearchState data classes in DebugViewModel
- Replace unnecessary mutableStateOf wrapping in remember(key) blocks
(ChannelConfigScreen, LoRaConfigItemList)
- Replace SideEffect with LaunchedEffect(destNum) in SettingsNavigation
- Refactor CoTXml.toXml() to use buildString instead of manual StringBuilder
- Replace string concatenation with string templates across 6 files
- Replace regex.find()?.let { _ -> } with if(containsMatchIn()) pattern
- Simplify with(list) { if (size > i) get(i) else default } to getOrNull()
- Remove unused androidx-lifecycle-testing from version catalog
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace runCatching with safeCatching in the coroutine context and remove the now-redundant manual CancellationException re-throw in onFailure. Consistent with the safeCatching migration across the rest of the audit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- testing-ci: remove ./gradlew clean, consolidate to single invocation - project-overview: add step 3 (init secrets) to workspace bootstrap - kmp-common: add safeCatching and CancellationException import rules - code-review: add coroutine safety, HttpClientDefaults, Room patterns - kmp-architecture: add CoroutineDispatchers injection, safeCatching, HttpClientDefaults, Room @Upsert/LIMIT 1/N+1 batching patterns - compose-ui: add @Preview in commonMain, rememberSaveable for dialogs, expand NumberFormatter float formatting example - implement-feature: add spotlessApply to baseline verification Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Create MetricFormatter in core:common with temperature, voltage, current, percent, humidity, pressure, snr, rssi helpers - Fix 3 fragile %.1f format specifiers in strings.xml → %s - Remove %SECS% placeholder hack in TracerouteLog - Replace ~20 scattered formatString calls in Node.kt, NodeItem.kt, MaterialBatteryInfo.kt, SignalInfo.kt, LoraSignalIndicator.kt with MetricFormatter methods - Pre-format float values in DeviceMetrics.kt before passing to templates that now use %s - Add MetricFormatterTest with 14 test cases - Document string formatting decision tree in .skills/compose-ui - Update kmp-common instructions with MetricFormatter guidance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Delete dead ByteUtils.kt (exact duplicate of core/model CommonUtils.kt) - Move ByteUtilsTest to core/model as CommonUtilsTest - Extract normalizeAddress() to core:common AddressUtils.kt, remove duplicates from DatabaseConstants.kt and MeshPrefsImpl.kt - Migrate PowerMetrics.kt (6 calls), SignalMetrics.kt (6 calls), and NodeDetailsSection.kt (2 calls) from formatString to MetricFormatter - Net: -90 lines, 3 fewer formatString imports Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace hand-rolled CommonUri expect/actual with com.eygraber:uri-kmp, a battle-tested AOSP-derived KMP URI library. This eliminates: - Broken iOS no-op stub that returned null/empty for all URI operations - Unsafe `toPlatformUri(): Any` requiring `as android.net.Uri` casts - Manual JVM query parameter parser (parseQueryParameters) - 3 platform-specific actual implementations (android, jvm, ios) CommonUri is now a typealias to com.eygraber.uri.Uri, providing: - Identical API surface (host, fragment, pathSegments, getQueryParameter) - Type-safe Android bridging via toAndroidUri()/toKmpUri() - Working iOS implementation (no more broken stubs) - Pure-Kotlin URI parsing in commonMain Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace MeshtasticUri data class with deprecated typealias to CommonUri (which itself is a typealias to uri-kmp's Uri). This eliminates the redundant string-wrapping URI type and unifies all URI handling on a single type backed by uri-kmp. Changes: - MeshtasticUri.kt: rewrite as deprecated typealias to CommonUri - Delete MeshtasticUriExt.kt (redundant Android bridge) - Migrate all MeshtasticUri params/returns to CommonUri across 20+ files - Replace MeshtasticUri(string) constructors with CommonUri.parse(string) - Replace toMeshtasticUri() with toKmpUri() in Android source sets - Expand CommonUriTest with query parameter and file URI tests - All spotless, detekt, and compile checks pass Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
All references to MeshtasticUri have been migrated to CommonUri. The deprecated typealias is no longer imported anywhere and can be safely removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move platform-specific implementations to pure Kotlin in commonMain: - formatString: Move pure-Kotlin format parser from iosMain to commonMain, eliminating expect/actual and jvmAndroid String.format() wrapper. All 60+ call sites now use a single implementation. - SfppHasher: Replace ByteBuffer/ByteOrder (JVM) with pure Kotlin little-endian byte encoding. Use Okio ByteString.sha256() for cross-platform hashing. Eliminates expect object entirely. - CryptoCodec: Replace MessageDigest (JVM) and CoreCrypto (iOS) with Okio ByteString.sha256(). ZlibCodec remains as expect/actual (no pure-Kotlin zlib available). Renamed CodecActual.kt → ZlibCodec.kt. Net: -82 lines, 5 platform-specific files eliminated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…istency - Delete AndroidDateTimeUtils.kt: getShortDate() was never called, orphaned KDoc for calculateMuteRemainingTime (already in commonMain) - Fix JVM DateFormatter.formatDateTimeShort to use java.time API consistently instead of mixing java.text.DateFormat - Remove unused java.text.DateFormat import Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…r spacing - Use safeCatching instead of runCatching in suspend cleanupTemporaryFiles() to avoid swallowing CancellationException - Add space before V and mA units in MetricFormatter for consistency with hPa, dB, dBm formatting - Update MetricFormatterTest assertions to match Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Revert DesktopProcessLifecycleOwner from eager module-body construction
back to lazy single{} block — eager init triggered setCurrentState()
outside the main thread, failing DesktopKoinTest.
Guard 'Upload coverage to Codecov' step with inputs.run_coverage so
it's skipped when PR workflow disables coverage (run_coverage: false).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jamesarich
force-pushed
the
chore/audit2
branch
from
b054160 to
f99f911
Compare
jamesarich
changed the title
- compose-ui SKILL: formatString is pure Kotlin, not expect/actual - code-review SKILL: Locale purged from commonMain, add MetricFormatter - core:common README: replace deleted ByteUtils with MetricFormatter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- SfppHasherTest: 8 tests — determinism, output size, byte order, collisions - AddressUtilsTest: 9 tests — null/blank/sentinel, colon stripping, case - FormatStringTest: 8 new hex/edge tests — %x, %X, %08x, node IDs, bounds - MetricFormatterTest: 9 new edge tests — freezing/boiling, zero values, negatives Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Expect/actual count: 10+ eliminated (was 7), ~20 retained - Add URI unification and utility commonization to architecture decisions - Add MetricFormatter and commonize utilities to roadmap health table - Update test maturity notes with new SfppHasher/AddressUtils coverage - Remove stale 'desktop navigation graphs' from test gaps - Update last-modified dates Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JvmServiceDiscoveryTest and AndroidFileServiceTest were missing the required dispatchers parameter added during the audit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
StandardTestDispatcher requires explicit advancement. When passed to classes that use flowOn(dispatchers.io), the flow never completes because no one advances the standalone dispatcher — causing CI hangs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive KMP codebase audit: commonize platform-specific code into
commonMain, centralize scattered utilities, eliminate dead abstractions, and fix CI issues.Changes
Commonization & Architecture
expect/actualCommonUriwithtypealiastouri-kmp(com.eygraber.uri.Uri). Delete 3 redundant actual implementations.MeshtasticUri→CommonUriacross 26 files, then delete the deprecated wrapper entirely.actualto pure-Kotlin parser incommonMain— handles%s,%d,%f,%x, positional args.Okio ByteString.sha256()(was JVM-onlyMessageDigest).Okio ByteString.sha256()(was JVM-only).Centralized Utilities
formatString("%.1f°C", val)calls.°C/°F/%, space beforeV/mA/hPa/dB/dBm.Dead Code & Dedup Cleanup
ByteUtils.kt,AndroidDateTimeUtils.kt(unusedgetShortDate).normalizeAddress()(was in bothbleandcommon).DateFormatterto usejava.timeconsistently (was mixingjava.text).Best Practices
safeCatching{}in suspend contexts (FirmwareUpdateViewModel,cleanupTemporaryFiles).AGENTS.md,CLAUDE.md,GEMINI.md, skills, instructions.CI Fix
inputs.run_coveragecondition — skip when PR workflow disables coverage.DesktopKoinTestfailure: revert eagerLifecycleOwnerinit back to lazysingle{}block.Dependency Audit (base commit)
@Indexannotations for query performance.NodeInfoDao.installConfigto eliminate N+1 queries.setMuteUntil+ injectedCoroutineDispatchers.Testing
MetricFormatterTestcases covering all metric types and edge cases.CommonUriTestwith query parameter and file URI tests.spotlessCheck,detekt,assembleDebug,test,allTests.