Implement multi-layer variant fingerprinting for more robust app upda…#411
Implement multi-layer variant fingerprinting for more robust app upda…#411rainxchzed merged 3 commits intomainfrom
Conversation
…te resolution.
- **Core Logic**: Introduce `AssetVariant` with three layers of identity: token-set fingerprints (primary), glob patterns (secondary), and legacy substring-tail extracts (fallback).
- **Database**: Bump schema to version 12, adding `preferredAssetTokens`, `assetGlobPattern`, `pickedAssetIndex`, and `pickedAssetSiblingCount` to the `installed_apps` table.
- **Resolver**: Enhance update resolution to match assets using the new multi-layer fingerprints and a "same-position" fallback for identical asset counts.
- **UI/Presentation**:
- Add an "Unpin" affordance to the asset picker sheet.
- Introduce a "Pick variant" entry point in the apps list with status-aware tinting (pinned/stale).
- Display a "Pinned" badge on selected assets in the picker.
- Show a one-time toast notification when a user first pins a variant.
- Clarify the relationship between asset filters and variant pins in the advanced settings.
- **Portability**: Update export/import schema to version 4 to include multi-layer fingerprint metadata.
- **Localization**: Add strings for the new variant picker UX in Arabic, Bengali, French, Hindi, Italian, Japanese, Korean, Polish, Russian, Spanish, Turkish, and Simplified Chinese.
WalkthroughAdds multi-layer preferred-asset fingerprinting (tokens, glob, positional) across domain, data, and UI; bumps Room schema to v12 with a migration adding four columns to Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Picker UI
participant VM as DetailsViewModel
participant Repo as InstalledAppsRepository
participant DB as Room DB
User->>UI: Pick asset to pin
UI->>VM: SelectDownloadAsset(picked)
VM->>VM: fingerprintFromPickedAsset(picked.name, siblingCount)
VM->>Repo: setPreferredVariant(pkg, variant, tokens, glob, index, count)
Repo->>DB: updatePreferredVariant(pkg, variant, tokens, glob, index, count)
DB-->>Repo: OK
Repo-->>VM: OK
VM-->>UI: show pinned toast
sequenceDiagram
participant Updater as Update Check
participant Repo as InstalledAppsRepository
participant Resolver as AssetVariant
participant Installer as Installer
Updater->>Repo: resolveTrackedRelease(release)
Repo->>Resolver: resolvePreferredAsset(assets, pinnedVariant, pinnedTokens, pinnedGlob)
alt Fingerprint match
Resolver-->>Repo: matched asset
else No fingerprint match
Repo->>Resolver: resolveBySamePosition(assets, pickedIndex, siblingCount)
alt Position match
Resolver-->>Repo: matched asset
else
Repo->>Installer: choosePrimaryAsset(assets)
Installer-->>Repo: auto-selected asset
end
end
Repo-->>Updater: resolved asset
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.kt (1)
72-85:⚠️ Potential issue | 🟠 MajorPinned-state UI only understands the legacy tail label.
The persistence model now allows a valid pin to exist with
preferredAssetTokens/assetGlobPatternwhilepreferredAssetVariantisnull, but this sheet only getspinnedVariantand gates both the hint row and badge on that string. In those cases the asset will still be pinned for update resolution, yet this UI shows nothing pinned and removes the new unpin affordance entirely. Pass the full persisted fingerprint into the picker and match againstfingerprintFromPickedAsset(...)instead ofAssetVariant.extract(...).Also applies to: 135-140, 178-218
🤖 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/components/ReleaseAssetsPicker.kt` around lines 72 - 85, The picker UI only checks pinnedVariant (legacy tail label) so it misses pins where preferredAssetTokens/assetGlobPattern exist and preferredAssetVariant is null; update ReleaseAssetsPicker and the calls to ReleaseAssetsItemsPicker to accept the full persisted fingerprint (the complete persisted pin object) instead of just pinnedVariant, then change the matching logic inside the picker to use fingerprintFromPickedAsset(...) when comparing a picked asset to the persisted pin rather than AssetVariant.extract(...); ensure all usages that previously passed pinnedVariant (including the other occurrences mentioned around the ReleaseAssetsPicker/ReleaseAssetsItemsPicker invocation and the selection/unpin handlers) are updated to pass and use the fingerprint so the hint row and unpin affordance render correctly.feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt (1)
719-727:⚠️ Potential issue | 🟡 MinorUpdate the stale-variant copy to match the new interaction.
Line 720 still tells users to “tap Update to reselect”, but this label now opens the picker directly via Line 724. The warning text is now misleading and should mention tapping the warning itself or the variant button instead.
🤖 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 719 - 727, The stale-variant warning text (string resource variant_stale_hint) is now misleading because the Text composable (Text(..., text = stringResource(Res.string.variant_stale_hint), modifier = Modifier.clickable(enabled = !isBusy, onClick = onPickVariantClick))) directly opens the picker via onPickVariantClick; update the copy in Res.string.variant_stale_hint (or swap to a new string key) to instruct users to "tap this warning or the variant button to reselect" (or equivalent), so the message matches the new interaction.feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.kt (1)
301-310:⚠️ Potential issue | 🔴 CriticalThis function call is missing the required
pickedAssetNameparameter and will not compile.The
linkAppToRepo()function requirespickedAssetName: String?(line 157) beforepickedAssetSiblingCount, but the call at line 306 skips it entirely when using named arguments. In Kotlin, you cannot skip required parameters even when using named arguments.Add
pickedAssetName = nullbeforepickedAssetSiblingCount:Fix
linkAppToRepo( deviceApp = deviceApp, repoInfo = repoInfo, assetFilterRegex = exportedApp.assetFilterRegex, fallbackToOlderReleases = exportedApp.fallbackToOlderReleases, + pickedAssetName = null, pickedAssetSiblingCount = exportedApp.pickedAssetSiblingCount ?: 0, preferredAssetVariant = exportedApp.preferredAssetVariant, preferredAssetTokens = exportedApp.preferredAssetTokens, assetGlobPattern = exportedApp.assetGlobPattern, pickedAssetIndex = exportedApp.pickedAssetIndex,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.kt` around lines 301 - 310, The call to linkAppToRepo(...) is missing the required pickedAssetName parameter; update the invocation in AppsRepositoryImpl (the block calling linkAppToRepo with deviceApp, repoInfo, assetFilterRegex, fallbackToOlderReleases, pickedAssetSiblingCount, ...) to include pickedAssetName = null (or the appropriate String?) before pickedAssetSiblingCount so the named argument ordering matches linkAppToRepo(deviceApp, repoInfo, assetFilterRegex, fallbackToOlderReleases, pickedAssetName: String?, pickedAssetSiblingCount, ...).
🧹 Nitpick comments (1)
feature/apps/domain/src/commonMain/kotlin/zed/rainxch/apps/domain/repository/AppsRepository.kt (1)
65-65: Consider documentingassetGlobPatternlike the other new parameters.Keeping KDoc parity here will make the API contract easier to consume.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/apps/domain/src/commonMain/kotlin/zed/rainxch/apps/domain/repository/AppsRepository.kt` at line 65, Add KDoc for the assetGlobPattern parameter in the AppsRepository API to match the other newly added parameters: update the KDoc block for the function/class that declares assetGlobPattern (referencing the parameter name assetGlobPattern) to include an `@param` entry explaining what glob pattern matches, accepted format, behavior when null (default), and how it affects asset selection; ensure wording and style mirror the existing KDoc entries for the other new parameters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 419-445: The equality check in isSameFingerprint is incomplete:
change the variant comparison to be null-safe (treat two null variants as equal,
e.g. fingerprint.variant?.equals(currentVariant, ignoreCase = true) ?:
(currentVariant == null)) and extend isSameFingerprint to also compare
positional metadata (pickedIndex vs current pickedAssetIndex and
installable.size vs current pickedAssetSiblingCount) so a re-pick that only
affects asset ordering still triggers saving; then keep the existing shouldSave
logic that also respects installedApp.preferredVariantStale and call
installedAppsRepository.setPreferredVariant with the updated values when needed.
---
Outside diff comments:
In
`@feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.kt`:
- Around line 301-310: The call to linkAppToRepo(...) is missing the required
pickedAssetName parameter; update the invocation in AppsRepositoryImpl (the
block calling linkAppToRepo with deviceApp, repoInfo, assetFilterRegex,
fallbackToOlderReleases, pickedAssetSiblingCount, ...) to include
pickedAssetName = null (or the appropriate String?) before
pickedAssetSiblingCount so the named argument ordering matches
linkAppToRepo(deviceApp, repoInfo, assetFilterRegex, fallbackToOlderReleases,
pickedAssetName: String?, pickedAssetSiblingCount, ...).
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt`:
- Around line 719-727: The stale-variant warning text (string resource
variant_stale_hint) is now misleading because the Text composable (Text(...,
text = stringResource(Res.string.variant_stale_hint), modifier =
Modifier.clickable(enabled = !isBusy, onClick = onPickVariantClick))) directly
opens the picker via onPickVariantClick; update the copy in
Res.string.variant_stale_hint (or swap to a new string key) to instruct users to
"tap this warning or the variant button to reselect" (or equivalent), so the
message matches the new interaction.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.kt`:
- Around line 72-85: The picker UI only checks pinnedVariant (legacy tail label)
so it misses pins where preferredAssetTokens/assetGlobPattern exist and
preferredAssetVariant is null; update ReleaseAssetsPicker and the calls to
ReleaseAssetsItemsPicker to accept the full persisted fingerprint (the complete
persisted pin object) instead of just pinnedVariant, then change the matching
logic inside the picker to use fingerprintFromPickedAsset(...) when comparing a
picked asset to the persisted pin rather than AssetVariant.extract(...); ensure
all usages that previously passed pinnedVariant (including the other occurrences
mentioned around the ReleaseAssetsPicker/ReleaseAssetsItemsPicker invocation and
the selection/unpin handlers) are updated to pass and use the fingerprint so the
hint row and unpin affordance render correctly.
---
Nitpick comments:
In
`@feature/apps/domain/src/commonMain/kotlin/zed/rainxch/apps/domain/repository/AppsRepository.kt`:
- Line 65: Add KDoc for the assetGlobPattern parameter in the AppsRepository API
to match the other newly added parameters: update the KDoc block for the
function/class that declares assetGlobPattern (referencing the parameter name
assetGlobPattern) to include an `@param` entry explaining what glob pattern
matches, accepted format, behavior when null (default), and how it affects asset
selection; ensure wording and style mirror the existing KDoc entries for the
other new parameters.
🪄 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: b565ca08-2c5c-4c03-a34c-a097ca855360
📒 Files selected for processing (38)
.claude/memory/feedback_coding_boundaries.mdcore/data/schemas/zed.rainxch.core.data.local.db.AppDatabase/12.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_11_12.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/AppsRoot.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/DetailsAction.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleaseAssetsPicker.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/Header.kt
💤 Files with no reviewable changes (1)
- .claude/memory/feedback_coding_boundaries.md
...ails/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt
Show resolved
Hide resolved
…/details/presentation/DetailsViewModel.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt (1)
486-493: Consider adding user feedback on unpin failure.Currently, if
clearPreferredVariantfails, the user sees no toast—only a log entry. Since the user explicitly tapped "Unpin," they might expect confirmation or an error message. This is a minor UX polish.💡 Optional: Add error toast
} catch (e: Exception) { logger.error( "Failed to clear preferred variant for " + "${installedApp.packageName}: ${e.message}", ) + _events.send( + DetailsEvent.OnMessage("Failed to unpin variant"), + ) }🤖 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 486 - 493, The catch block in DetailsViewModel where clearPreferredVariant is called currently only logs the error; update it to also emit a user-facing error event (e.g., call the ViewModel's existing UI event/notification mechanism such as showToast, postUiEvent, or update a MutableStateFlow for toasts) so the user sees a brief error message when unpin fails; preserve the CancellationException rethrow and include the error message text (e.message) in the user-visible string for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 486-493: The catch block in DetailsViewModel where
clearPreferredVariant is called currently only logs the error; update it to also
emit a user-facing error event (e.g., call the ViewModel's existing UI
event/notification mechanism such as showToast, postUiEvent, or update a
MutableStateFlow for toasts) so the user sees a brief error message when unpin
fails; preserve the CancellationException rethrow and include the error message
text (e.message) in the user-visible string for clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5c5b79c6-09c8-4508-9bd8-5dfd87596243
📒 Files selected for processing (1)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt
…te resolution.
AssetVariantwith three layers of identity: token-set fingerprints (primary), glob patterns (secondary), and legacy substring-tail extracts (fallback).preferredAssetTokens,assetGlobPattern,pickedAssetIndex, andpickedAssetSiblingCountto theinstalled_appstable.Summary by CodeRabbit
New Features
Documentation