feat: implement preferred APK variant tracking and selection#408
feat: implement preferred APK variant tracking and selection#408rainxchzed merged 3 commits intomainfrom
Conversation
- Introduce `AssetVariant` utility to extract and match stable variant identifiers (e.g., `arm64-v8a`, `universal`) from GitHub asset filenames. - Add `preferredAssetVariant` and `preferredVariantStale` fields to the `installed_apps` database table (Migration 10 to 11). - Implement a `VariantPickerDialog` to allow users to manually select or reset their preferred APK variant. - Update the update-checker logic to prioritize the user's preferred variant and flag it as "stale" if a match is no longer found in new releases. - Surface variant status in the UI, including a "Variant changed" warning and inline variant labels in the apps list. - Update the export/import schema to version 3 to persist preferred variant settings across devices. - Provide localized strings for the variant picker and status hints across all supported languages.
|
Caution Review failedPull request was closed or merged during review WalkthroughAdds pinned APK variant support: DB schema v11 and migration; new domain/util AssetVariant; repository APIs and migrations to persist/track preferred variants; UI picker, viewmodel flows, and localized strings across many languages. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant VM as Apps<br/>ViewModel
participant Repo as InstalledApps<br/>Repository
participant DB as Database
participant Dialog as Variant Picker<br/>Dialog
User->>VM: Tap "Update" (app with preferredVariantStale=true)
VM->>VM: Detect stale → openVariantPicker(resume=true)
VM->>Repo: previewMatchingAssets(packageName)
Repo->>DB: Query latest release assets
DB-->>Repo: Assets list
Repo-->>VM: Assets (UI list)
VM->>Dialog: Show options + current variant
User->>Dialog: Select variant
Dialog->>VM: OnVariantSelected(variant)
VM->>Repo: setPreferredVariant(packageName, variant)
Repo->>DB: UPDATE installed_apps SET preferredAssetVariant=?, preferredVariantStale=0
DB-->>Repo: OK
Repo->>Repo: checkForUpdates(packageName) (variant-aware)
Repo-->>VM: Updated InstalledApp
VM->>VM: resume update → updateSingleApp(...)
VM-->>User: Install update using selected variant
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt (1)
422-439:⚠️ Potential issue | 🟠 MajorDon’t read the preferred variant from
_stateduring initial load.
loadInitial()calls this helper before_state.installedAppis updated, so the first render falls back toinstaller.choosePrimaryAsset(...)even when a pinned variant already exists. Pass the preferred variant in from the caller instead of reading mutable state here.Suggested shape
-private fun recomputeAssetsForRelease(release: GithubRelease?): Pair<List<GithubAsset>, GithubAsset?> { +private fun recomputeAssetsForRelease( + release: GithubRelease?, + preferredVariant: String? = _state.value.installedApp?.preferredAssetVariant, +): Pair<List<GithubAsset>, GithubAsset?> { @@ - val preferredVariant = _state.value.installedApp?.preferredAssetVariant val variantMatch = AssetVariant.resolvePreferredAsset(installable, preferredVariant)Then have
loadInitial()call:val (installable, primary) = recomputeAssetsForRelease(selectedRelease, installedApp?.preferredAssetVariant)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt` around lines 422 - 439, recomputeAssetsForRelease currently reads preferredVariant from mutable _state which causes incorrect fallback during initial load; change recomputeAssetsForRelease signature to accept a preferredVariant: String? (or whatever type used) passed in by the caller, use that parameter in the call to AssetVariant.resolvePreferredAsset(installable, preferredVariant) instead of reading _state.value.installedApp, and leave the fallback to installer.choosePrimaryAsset(installable) unchanged; update callers (notably loadInitial) to call recomputeAssetsForRelease(selectedRelease, installedApp?.preferredAssetVariant) so the pinned variant is respected on first render.
🧹 Nitpick comments (3)
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt (1)
703-706: Comment text is misleading about click behavior.At Line 703, the comment says “tappable hint,” but this branch renders plain
Text(non-clickable). Please reword the comment to avoid future confusion.✏️ Suggested comment-only tweak
- // Tap-to-fix label: route through the same OnUpdateApp - // intercept that would have opened the picker anyway, - // but also surface a tappable hint here for users - // who notice the warning before tapping Update. + // Visual warning only. Users resolve this via the + // existing Update action, which opens the variant picker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt` around lines 703 - 706, The comment incorrectly calls the rendered plain Text a "tappable hint" even though it's non-clickable; update the comment near the branch that renders Text to clarify it's a non-interactive warning that routes through the same OnUpdateApp intercept (referencing OnUpdateApp and the Text render) and remove the word "tappable" so future readers won't expect click behavior.feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/VariantPickerDialog.kt (1)
302-308: Consider locale-aware byte formatting.
String.formatuses the default locale, which may produce unexpected decimal separators (e.g.,1,5 MBinstead of1.5 MB) depending on the user's device settings. For a utility function used in UI, consider using a locale-invariant format or a shared formatting utility.♻️ Use explicit locale for consistent formatting
+import java.util.Locale + private fun formatBytes(bytes: Long): String = when { - bytes >= 1_073_741_824 -> "%.1f GB".format(bytes / 1_073_741_824.0) - bytes >= 1_048_576 -> "%.1f MB".format(bytes / 1_048_576.0) - bytes >= 1_024 -> "%.1f KB".format(bytes / 1_024.0) + bytes >= 1_073_741_824 -> String.format(Locale.US, "%.1f GB", bytes / 1_073_741_824.0) + bytes >= 1_048_576 -> String.format(Locale.US, "%.1f MB", bytes / 1_048_576.0) + bytes >= 1_024 -> String.format(Locale.US, "%.1f KB", bytes / 1_024.0) else -> "$bytes B" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/VariantPickerDialog.kt` around lines 302 - 308, The formatBytes function uses String.format without an explicit locale causing locale-dependent decimal separators; update format calls in formatBytes to use a locale-invariant formatter (e.g., Locale.US) or a shared formatting utility so bytes like 1.5 MB are always rendered with a dot; locate formatBytes and replace "%.1f GB".format(...) / "%.1f MB".format(...) / "%.1f KB".format(...) with the locale-explicit formatting approach (or call the shared helper) to ensure consistent UI output across locales.feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt (1)
623-655: Network failures may show misleading "no_assets" error instead of "load_failed".Per
InstalledAppsRepositoryImpl.previewMatchingAssets()(context snippet 2), network failures returnMatchingPreview(release=null, matchedAssets=emptyList())rather than throwing. This means a network error will show "No installable assets" (Line 641) instead of "Could not load variants" (Line 651).Consider checking
preview.release == nullto distinguish between "no release found" (likely a fetch failure) and "release exists but has no matching assets":♻️ Distinguish fetch failure from empty assets
_state.update { it.copy( variantPickerLoading = false, variantPickerOptions = preview.matchedAssets .map { asset -> asset.toUi() } .toImmutableList(), variantPickerError = - if (preview.matchedAssets.isEmpty()) "no_assets" else null, + when { + preview.release == null -> "load_failed" + preview.matchedAssets.isEmpty() -> "no_assets" + else -> null + }, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt` around lines 623 - 655, The current AppsViewModel coroutine treats an empty preview.matchedAssets as a generic "no_assets" error even when preview.release is null due to network/fetch failures; update the handling after calling installedAppsRepository.previewMatchingAssets (the previewMatchingAssets call) to check preview.release == null and set variantPickerError to "load_failed" (or another fetch-failure key) when release is null, otherwise keep "no_assets" when release is non-null but matchedAssets is empty; adjust the _state.update block that sets variantPickerLoading/variantPickerOptions/variantPickerError accordingly so variantPickerError distinguishes fetch failures from genuine empty asset lists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/entities/InstalledAppEntity.kt`:
- Around line 67-74: The entity field preferredVariantStale in
InstalledAppEntity lacks an explicit Room default value causing metadata
mismatch with MIGRATION_10_11 which adds the column with DEFAULT 0; update the
InstalledAppEntity definition to annotate the preferredVariantStale property
with `@ColumnInfo`(defaultValue = "0") so the Room entity schema matches the
migration (refer to the InstalledAppEntity class and the preferredVariantStale
property and ensure it aligns with MIGRATION_10_11).
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/AssetVariant.kt`:
- Around line 32-33: The VERSION_SEGMENT regex is too permissive and matches
architecture fragments (e.g., "-v8" or "_64"), breaking resolvePreferredAsset
and deriveFromPickedAsset; update the Regex assigned to VERSION_SEGMENT to
require at least one dot in the numeric portion (e.g., require \.\d) and add a
lookahead/word-boundary so the pattern only matches version-like suffixes (for
example: enforce something like "v?\d+(?:\.\d+)+") and ensure it anchors to
token boundaries to avoid mid-token matches; replace the current Regex("[-_
]v?\\d+(?:\\.\\d+)*", ...) with the hardened pattern so variant extraction no
longer treats architecture tokens as versions.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 694-705: The code reads _state.value.apps after
setPreferredVariant finishes, which can race because checkForUpdates updates the
DB asynchronously; instead fetch the canonical/updated InstalledApp directly
from the data layer before calling updateSingleApp — call the repository method
that returns the latest installed app by packageName (or await the Flow from
loadApps filtered for that packageName) and pass that result into
updateSingleApp (referencing setPreferredVariant, checkForUpdates, _state,
loadApps, updateSingleApp) so you never rely on possibly-stale
_state.value.apps.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/VariantPickerDialog.kt`:
- Around line 222-230: The VariantPickerDialog currently shows assets where
AssetVariant.extract(asset.name) returns null but their onClick silently no-ops;
update the rendering so unextractable assets are either filtered out or become
selectable using the full asset name as a fallback: either (A) filter
variantPickerOptions before passing to items by keeping only assets where
AssetVariant.extract(asset.name) != null (update caller openVariantPicker or
local mapping), or (B) change the item logic in VariantPickerDialog (where
VariantRow is created) to compute a non-null identifier = variant ?: asset.name,
use that identifier for isCurrent comparison and for onClick (call onPick with
the identifier), and adjust title/subtitle to indicate fallback when needed.
Ensure you modify AssetVariant.extract usage, VariantRow parameters (isSelected,
onClick), and the selection comparison against current to use the chosen
identifier so rows become selectable or hidden consistently.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 389-407: The current equality guard in DetailsViewModel prevents
calling installedAppsRepository.setPreferredVariant when the user re-selects the
same variant, which leaves preferredVariantStale uncleared; remove or relax that
guard so setPreferredVariant is invoked whenever a non-null variant is chosen
(e.g., change the if from "variant != null && !variant.equals(current,
ignoreCase = true)" to simply "variant != null") so that
installedAppsRepository.setPreferredVariant(...) runs and clears
preferredVariantStale/refreshes cached metadata even for the same variant.
---
Outside diff comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 422-439: recomputeAssetsForRelease currently reads
preferredVariant from mutable _state which causes incorrect fallback during
initial load; change recomputeAssetsForRelease signature to accept a
preferredVariant: String? (or whatever type used) passed in by the caller, use
that parameter in the call to AssetVariant.resolvePreferredAsset(installable,
preferredVariant) instead of reading _state.value.installedApp, and leave the
fallback to installer.choosePrimaryAsset(installable) unchanged; update callers
(notably loadInitial) to call recomputeAssetsForRelease(selectedRelease,
installedApp?.preferredAssetVariant) so the pinned variant is respected on first
render.
---
Nitpick comments:
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt`:
- Around line 703-706: The comment incorrectly calls the rendered plain Text a
"tappable hint" even though it's non-clickable; update the comment near the
branch that renders Text to clarify it's a non-interactive warning that routes
through the same OnUpdateApp intercept (referencing OnUpdateApp and the Text
render) and remove the word "tappable" so future readers won't expect click
behavior.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 623-655: The current AppsViewModel coroutine treats an empty
preview.matchedAssets as a generic "no_assets" error even when preview.release
is null due to network/fetch failures; update the handling after calling
installedAppsRepository.previewMatchingAssets (the previewMatchingAssets call)
to check preview.release == null and set variantPickerError to "load_failed" (or
another fetch-failure key) when release is null, otherwise keep "no_assets" when
release is non-null but matchedAssets is empty; adjust the _state.update block
that sets variantPickerLoading/variantPickerOptions/variantPickerError
accordingly so variantPickerError distinguishes fetch failures from genuine
empty asset lists.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/VariantPickerDialog.kt`:
- Around line 302-308: The formatBytes function uses String.format without an
explicit locale causing locale-dependent decimal separators; update format calls
in formatBytes to use a locale-invariant formatter (e.g., Locale.US) or a shared
formatting utility so bytes like 1.5 MB are always rendered with a dot; locate
formatBytes and replace "%.1f GB".format(...) / "%.1f MB".format(...) / "%.1f
KB".format(...) with the locale-explicit formatting approach (or call the shared
helper) to ensure consistent UI output across locales.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ae434638-6c51-4c09-8fd7-2bf3c5cb226b
📒 Files selected for processing (36)
core/data/schemas/zed.rainxch.core.data.local.db.AppDatabase/11.jsoncore/data/src/androidMain/kotlin/zed/rainxch/core/data/local/db/initDatabase.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/local/db/migrations/MIGRATION_10_11.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/AppDatabase.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/entities/InstalledAppEntity.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/mappers/InstalledAppsMappers.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/ExportedApp.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/InstalledApp.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/InstalledAppsRepository.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/AssetVariant.ktcore/presentation/src/commonMain/composeResources/values-ar/strings-ar.xmlcore/presentation/src/commonMain/composeResources/values-bn/strings-bn.xmlcore/presentation/src/commonMain/composeResources/values-es/strings-es.xmlcore/presentation/src/commonMain/composeResources/values-fr/strings-fr.xmlcore/presentation/src/commonMain/composeResources/values-hi/strings-hi.xmlcore/presentation/src/commonMain/composeResources/values-it/strings-it.xmlcore/presentation/src/commonMain/composeResources/values-ja/strings-ja.xmlcore/presentation/src/commonMain/composeResources/values-ko/strings-ko.xmlcore/presentation/src/commonMain/composeResources/values-pl/strings-pl.xmlcore/presentation/src/commonMain/composeResources/values-ru/strings-ru.xmlcore/presentation/src/commonMain/composeResources/values-tr/strings-tr.xmlcore/presentation/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xmlcore/presentation/src/commonMain/composeResources/values/strings.xmlfeature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.ktfeature/apps/domain/src/commonMain/kotlin/zed/rainxch/apps/domain/repository/AppsRepository.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsAction.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsState.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/AdvancedAppSettingsBottomSheet.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/VariantPickerDialog.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/mappers/InstalledAppMapper.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/model/InstalledAppUi.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt
core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/entities/InstalledAppEntity.kt
Show resolved
Hide resolved
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/AssetVariant.kt
Outdated
Show resolved
Hide resolved
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt
Show resolved
Hide resolved
...tation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/VariantPickerDialog.kt
Show resolved
Hide resolved
...ails/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt
Outdated
Show resolved
Hide resolved
- Enhance `normalizeVersion` to better handle non-standard version tags and prefixes (e.g., `release-1.2.0`, `App-v1.2.0`). - Update logic to strip build metadata (text after `+`) as per SemVer specifications. - Add regex-based extraction to isolate dotted-digit substrings when standard prefix stripping fails. - Fixes an issue where lexicographic comparisons incorrectly flagged updates when version strings contained leading non-numeric characters.
- **Localization**: Added `variant_picker_no_pinnable` string across multiple languages (English, Arabic, Turkish, Chinese, Bengali, Japanese, Korean, Polish, Italian, French, Russian, Hindi, Spanish) to inform users when a release lacks version-tagged variants for pinning.
- **Domain Logic**: Refined the `AssetVariant` extraction regex to require at least two dotted digit groups and a clean token boundary, preventing false matches on architecture tokens (e.g., `-v8a`).
- **Data Layer**:
- Updated `InstalledAppEntity` with `@ColumnInfo(defaultValue = "0")` for `fallbackToOlderReleases` and `preferredVariantStale` to ensure Room schema consistency.
- Incremented database identity hash in schema version 11.
- **Presentation Logic**:
- **AppsViewModel**: Filtered variant picker options to only include "pinnable" assets (those with extractable variant tags).
- **AppsViewModel**: Changed the post-selection update flow to perform a direct DAO read from the repository to prevent race conditions with asynchronous state updates.
- **DetailsViewModel**: Updated logic to allow re-saving the same variant if the `preferredVariantStale` flag is set, ensuring the stale warning can be cleared.
- **VariantPickerDialog**: UI now displays the specific "no pinnable variants" error message and skips rendering invalid assets.
AssetVariantutility to extract and match stable variant identifiers (e.g.,arm64-v8a,universal) from GitHub asset filenames.preferredAssetVariantandpreferredVariantStalefields to theinstalled_appsdatabase table (Migration 10 to 11).VariantPickerDialogto allow users to manually select or reset their preferred APK variant.Summary by CodeRabbit
New Features
Database