Skip to content

android: fix stale update status in PackageEventReceiver on external …#454

Merged
rainxchzed merged 9 commits intomainfrom
app-version-notify
Apr 25, 2026
Merged

android: fix stale update status in PackageEventReceiver on external …#454
rainxchzed merged 9 commits intomainfrom
app-version-notify

Conversation

@rainxchzed
Copy link
Copy Markdown
Member

@rainxchzed rainxchzed commented Apr 24, 2026

…app updates

  • Refactor PackageEventReceiver to detect version changes when an app is updated or reinstalled externally (e.g., via Play Store or sideloading).
  • Ensure installedVersion, installedVersionName, installedVersionCode, and isUpdateAvailable are all synchronized when a version change is detected.
  • Recompute isUpdateAvailable by comparing the new system version code against the latest known version to prevent the UI from incorrectly showing available updates for versions already installed.
  • Add detailed logging to distinguish between actual version changes and broadcast events triggered by reinstalls of the same version.

Summary by CodeRabbit

  • New Features

    • Details screen: release-channel controls (toggle per-app beta inclusion, switch to latest stable) and merged “what’s changed” changelog card; localized strings for these UIs.
    • UI: specific pre-release labels (Beta/RC/etc.) and stalled-stable warning card.
  • Bug Fixes / Improvements

    • More accurate version normalization & comparison for update checks.
    • External install handling now better detects immediate update availability and triggers authoritative re-checks.

…app updates

- Refactor `PackageEventReceiver` to detect version changes when an app is updated or reinstalled externally (e.g., via Play Store or sideloading).
- Ensure `installedVersion`, `installedVersionName`, `installedVersionCode`, and `isUpdateAvailable` are all synchronized when a version change is detected.
- Recompute `isUpdateAvailable` by comparing the new system version code against the latest known version to prevent the UI from incorrectly showing available updates for versions already installed.
- Add detailed logging to distinguish between actual version changes and broadcast events triggered by reinstalls of the same version.
- Introduce `VersionMath` utility to provide a single source of truth for version normalization and semver-compliant comparison.
- Implement `resolveExternalInstallVerdict` to produce a local, best-effort update status after external package changes (e.g., sideloading) using a priority ladder of signals (versionCode, versionName, and release tags).
- Refactor `PackageEventReceiver` to handle external installs by updating local state and triggering an authoritative async update check via `InstalledAppsRepository`.
- Migrate version logic from `InstalledAppsRepositoryImpl` to `VersionMath` to ensure consistency across the app and fix incorrect "update available" flags caused by non-standard version naming.
- Update Claude local settings to allow `rtk grep` bash commands.
- Introduce `GithubRelease.isEffectivelyPreRelease()` as a single source of truth for identifying pre-releases.
- Implement `VersionMath.isPreReleaseTag` using regex to detect common markers (alpha, beta, rc, preview, snapshot, etc.) in tag names and release titles.
- Enhance pre-release filtering to combine the GitHub API `prerelease` flag with the new tag heuristic to catch releases where maintainers forgot to set the API flag.
- Add diagnostic logging in `InstalledAppsRepositoryImpl` to report mismatches between GitHub API flags and tag-based heuristics.
- Update `VersionPicker`, `DetailsViewModel`, and `DetailsState` to use the improved "effectively pre-release" logic for UI badges and release filtering.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

Walkthrough

Adds centralized version utilities and pre-release heuristics, exposes a targeted installed-version update API, updates package-install broadcast handling to compute an immediate install/update verdict and schedule authoritative re-checks, and surfaces pre-release channel controls across details data, viewmodel, and UI.

Changes

Cohort / File(s) Summary
Version utilities
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt, core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/ExternalInstallVerdict.kt
Adds VersionMath (normalize/compare/isPreRelease/preReleaseLabel) and resolveExternalInstallVerdict plus VersionVerdict enum to centralize version comparison and external-install verdict logic.
Release metadata extensions
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/GithubReleaseExt.kt
Adds GithubRelease.isEffectivelyPreRelease() and preReleaseLabel() using VersionMath heuristics.
Repository & DAO
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt, core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/InstalledAppsRepository.kt, core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.kt
Repository delegates version-newer checks to VersionMath, adds updateInstalledVersion(...) API; DAO adds updateInstalledVersion SQL update to persist installed-version fields plus isUpdateAvailable in one statement.
Package event handling
core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt
Detects external-install version changes, computes VersionVerdict via resolveExternalInstallVerdict, persists installed-version fields and isUpdateAvailable, logs path details, and schedules async repo.checkForUpdates(packageName) on a backstop app-scoped coroutine.
Details feature — data & wiring
feature/details/data/.../SharedModule.kt, feature/details/data/.../InstallationManagerImpl.kt
Injects TweaksRepository into InstallationManagerImpl; new installs initialize includePreReleases from global tweaks (safe fallback on failure).
Details feature — presentation & UI
feature/details/presentation/.../DetailsState.kt, .../DetailsViewModel.kt, .../DetailsAction.kt, .../DetailsRoot.kt, .../components/VersionPicker.kt, .../components/sections/ReleaseChannel.kt
Replaces raw isPrerelease usage with isEffectivelyPreRelease(), adds state fields (stalledStableSinceDays, mergedChangelog, mergedChangelogBaseTag, latestStableHasInstallableAsset), new actions (ToggleIncludeBetas, SwitchToStable), merged changelog/stalled-stable logic, and a new release-channel UI section with chips/cards.
Strings / localization
core/presentation/src/commonMain/composeResources/values/strings.xml
Adds string resources supporting pre-release channel chips, switch-to-stable action, stalled-project warnings, and merged changelog headings.
Misc / config
.claude/settings.local.json
Expanded local allowlist to include Bash(rtk grep *) alongside existing WebFetch(domain:blog.jetbrains.com).

Sequence Diagram

sequenceDiagram
    participant PM as Android PM
    participant Receiver as PackageEventReceiver
    participant Repo as InstalledAppsRepository
    participant Verdict as VersionVerdictResolver
    participant DB as InstalledApp DAO

    PM->>Receiver: broadcast external install/update
    Receiver->>DB: load InstalledApp by packageName
    Receiver->>Receiver: extract newVersionCode/newVersionName
    Receiver->>Receiver: detect changed versionCode/name
    alt Version changed
        Receiver->>Verdict: resolveExternalInstallVerdict(app, newVersionName, newVersionCode)
        Verdict-->>Receiver: UP_TO_DATE / UPDATE_AVAILABLE / UNKNOWN
        Receiver->>DB: updateInstalledVersion(packageName, installedVersion, installedVersionName, installedVersionCode, isUpdateAvailable)
        Receiver->>Repo: schedule repo.checkForUpdates(packageName) on backstop scope
        Repo->>Repo: network validation & authoritative update
        Repo->>DB: persist canonical state
    else No change
        Receiver->>DB: no-op / preserve existing flags
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 I hop through tags and semver streams,
I sniff for betas, RCs, and dreams.
A verdict forms, a check set free,
New versions bob across the sea.
I nibble bugs and cheer updates—hooray! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'android: fix stale update status in PackageEventReceiver on external …' is directly related to the main change: fixing stale update status synchronization when apps are updated externally. However, it is truncated with '…' and does not fully convey the core improvement (atomic updates and preventing data races).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch app-version-notify

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
.claude/settings.local.json (1)

12-13: The .claude/settings.local.json file naming is misleading but not problematic.

The .local.json suffix typically indicates environment-specific settings, yet this is the only settings file in the .claude/ directory and is intentionally committed to version control as a shared configuration. Consider renaming it to .claude/settings.json to better reflect that it's a shared, not environment-specific, file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/settings.local.json around lines 12 - 13, The settings file is named
settings.local.json but is a shared, committed configuration; rename
settings.local.json to settings.json, update any code/CI/docs that reference
settings.local.json to use settings.json, and commit the rename preserving the
existing contents (the entries like "WebFetch(domain:blog.jetbrains.com)" and
"Bash(rtk grep *)" can be left unchanged).
core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt (1)

178-183: Nit: import InstalledApp instead of using the FQ name in the signature.

♻️ Proposed import + signature cleanup
 import zed.rainxch.core.domain.repository.InstalledAppsRepository
 import zed.rainxch.core.domain.system.PackageMonitor
+import zed.rainxch.core.domain.model.InstalledApp
 import zed.rainxch.core.domain.util.VersionVerdict
 import zed.rainxch.core.domain.util.resolveExternalInstallVerdict
     private suspend fun handleExternalInstall(
         packageName: String,
-        app: zed.rainxch.core.domain.model.InstalledApp,
+        app: InstalledApp,
         repo: InstalledAppsRepository,
         monitor: PackageMonitor,
     ) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt`
around lines 178 - 183, The function handleExternalInstall currently uses the
fully-qualified type zed.rainxch.core.domain.model.InstalledApp in its parameter
list; add an import for InstalledApp at the top of the file and change the
parameter type to InstalledApp in the handleExternalInstall signature (and
update any other occurrences in this file that reference the FQCN) so the code
uses the simple type name instead of the fully-qualified one.
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/ExternalInstallVerdict.kt (1)

93-107: Optional: the else → UNKNOWN branch in compareAndDecide is unreachable.

After the emptiness guard on line 99 both inputs are non-empty, and compareNormalized (used by both compareVersions and isVersionNewer) is antisymmetric, so cmp < 0isVersionNewer(latest, system) == true. The third arm can't fire. Harmless, but simplifying makes the intent obvious and avoids readers wondering what edge it's guarding.

♻️ Suggested simplification
     val cmp = VersionMath.compareVersions(system, latest)
-    return when {
-        cmp >= 0 -> VersionVerdict.UP_TO_DATE
-        VersionMath.isVersionNewer(latest, system) -> VersionVerdict.UPDATE_AVAILABLE
-        else -> VersionVerdict.UNKNOWN
-    }
+    return if (cmp >= 0) VersionVerdict.UP_TO_DATE else VersionVerdict.UPDATE_AVAILABLE
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/ExternalInstallVerdict.kt`
around lines 93 - 107, The final else branch returning VersionVerdict.UNKNOWN in
compareAndDecide is unreachable because after the emptiness guard
compareVersions and isVersionNewer are mutually exclusive; simplify
compareAndDecide (keeping the emptiness check) to return
VersionVerdict.UP_TO_DATE when VersionMath.compareVersions(system, latest) >= 0
and otherwise return VersionVerdict.UPDATE_AVAILABLE, removing the unreachable
VersionVerdict.UNKNOWN arm; refer to compareAndDecide,
VersionMath.compareVersions, VersionMath.isVersionNewer, and the VersionVerdict
enum (UP_TO_DATE, UPDATE_AVAILABLE, UNKNOWN) when making the change.
🤖 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/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt`:
- Around line 178-219: The current handleExternalInstall captures the stale app
snapshot and calls repo.updateApp(app.copy(...)), which can overwrite concurrent
changes; change this to perform a field-specific update or a transactional
read-modify-write: add/use an InstalledAppsRepository method like
updateInstalledVersion(packageName: String, versionName: String, versionCode:
Long, isUpdateAvailable: Boolean) and call that from handleExternalInstall
(using the computed newIsUpdateAvailable), or wrap the read and
repo.updateApp(...) call in repo.executeInTransaction { ... } so the
installed-version fields are updated atomically without stomping other fields
(refer to resolveExternalInstallVerdict and the app parameter to locate the
logic).

---

Nitpick comments:
In @.claude/settings.local.json:
- Around line 12-13: The settings file is named settings.local.json but is a
shared, committed configuration; rename settings.local.json to settings.json,
update any code/CI/docs that reference settings.local.json to use settings.json,
and commit the rename preserving the existing contents (the entries like
"WebFetch(domain:blog.jetbrains.com)" and "Bash(rtk grep *)" can be left
unchanged).

In
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt`:
- Around line 178-183: The function handleExternalInstall currently uses the
fully-qualified type zed.rainxch.core.domain.model.InstalledApp in its parameter
list; add an import for InstalledApp at the top of the file and change the
parameter type to InstalledApp in the handleExternalInstall signature (and
update any other occurrences in this file that reference the FQCN) so the code
uses the simple type name instead of the fully-qualified one.

In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/ExternalInstallVerdict.kt`:
- Around line 93-107: The final else branch returning VersionVerdict.UNKNOWN in
compareAndDecide is unreachable because after the emptiness guard
compareVersions and isVersionNewer are mutually exclusive; simplify
compareAndDecide (keeping the emptiness check) to return
VersionVerdict.UP_TO_DATE when VersionMath.compareVersions(system, latest) >= 0
and otherwise return VersionVerdict.UPDATE_AVAILABLE, removing the unreachable
VersionVerdict.UNKNOWN arm; refer to compareAndDecide,
VersionMath.compareVersions, VersionMath.isVersionNewer, and the VersionVerdict
enum (UP_TO_DATE, UPDATE_AVAILABLE, UNKNOWN) when making the change.
🪄 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: c49ba7f1-395a-4bf4-96e2-a9dc5d0adaa1

📥 Commits

Reviewing files that changed from the base of the PR and between 3e11fc5 and 40c6e53.

📒 Files selected for processing (9)
  • .claude/settings.local.json
  • core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/GithubReleaseExt.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/ExternalInstallVerdict.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt

- Implement a new `releaseChannel` section for the app details screen to handle GitHub pre-release and stable versioning logic.
- Add a `ChannelChip` component to toggle pre-release enrollment and provide a quick action to switch back to stable versions.
- Implement a "stalled-project" warning card that displays when a stable release is significantly outdated compared to available pre-releases.
- Add a "What's changed" card to display a merged changelog when a user has skipped multiple versions.
- Integrate these components into the `LazyListScope` using `DetailsState` to conditionally render based on app tracking status and release availability.
- Add `ToggleIncludeBetas` and `SwitchToStable` actions to support better release channel management from the Details screen.
- Implement `ReleaseInsights` logic to compute "stalled stable" warnings and merged changelogs for users skipping multiple versions.
- Introduce a "Switch to stable" affordance in `DetailsState` to help users roll back from pre-release versions to the latest stable release.
- Add a new `releaseChannel` UI section to the Details screen for toggling beta updates.
- Update `DetailsViewModel` to re-calculate release insights immediately after an app update or channel toggle.
- Support concatenated release notes in the "What's New" section to show all intermediate changes since the installed version.
- Add `ToggleIncludeBetas` and `SwitchToStable` actions to support better release channel management from the Details screen.
- Implement `ReleaseInsights` logic to compute "stalled stable" warnings and merged changelogs for users skipping multiple versions.
- Introduce a "Switch to stable" affordance in `DetailsState` to help users roll back from pre-release versions to the latest stable release.
- Add a new `releaseChannel` UI section to the Details screen for toggling beta updates.
- Update `DetailsViewModel` to re-calculate release insights immediately after an app update or channel toggle.
- Support concatenated release notes in the "What's New" section to show all intermediate changes since the installed version.
- Enhance `VersionPicker` to display specific pre-release labels (e.g., "Beta", "RC", "Alpha") instead of a generic "Pre-release" badge when a marker is detected in the release tag.
- Implement `preReleaseMarkerLabel` utility in `VersionMath` to parse and canonicalize common pre-release suffixes from version strings.
- Update `InstallationManagerImpl` to inherit the global "include betas" preference from `TweaksRepository` when registering a newly installed app.
- Add various string resources for the pre-release channel UI, including channel toggles and project health warnings for stalled stable releases.
- Inject `TweaksRepository` into `InstallationManagerImpl` via the shared Koin module.
- Add `updateInstalledVersion` to `InstalledAppDao` to allow targeted updates of version-related columns and the `isUpdateAvailable` flag.
- Implement `updateInstalledVersion` in `InstalledAppsRepository` and its implementation to provide a safer alternative to full-row updates.
- Refactor `PackageEventReceiver` to use the new atomic update method, preventing race conditions where concurrent writes to sibling columns (like favorites or pins) could be overwritten by a stale app snapshot.
- Improve data integrity during external app installations and sideloading by avoiding the "read-modify-write" pattern for the entire `InstalledApp` entity.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (1)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt (1)

110-111: Implicit ordering assumption on allReleases.

latestStableRelease uses firstOrNull, so it only yields the most recent stable if allReleases is guaranteed sorted descending by publishedAt. That's the common GitHub-API order, but nothing in this data class enforces it, and downstream uses (stalled-stable warning, "Switch to stable vX.Y.Z") silently degrade if the VM ever hands over unsorted data.

Consider either documenting the ordering contract on allReleases, or making this resilient:

♻️ Suggested resilient variant
     val latestStableRelease: GithubRelease?
-        get() = allReleases.firstOrNull { !it.isEffectivelyPreRelease() }
+        get() =
+            allReleases
+                .asSequence()
+                .filter { !it.isEffectivelyPreRelease() }
+                .maxByOrNull { it.publishedAt }
🤖 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/DetailsState.kt`
around lines 110 - 111, latestStableRelease currently assumes allReleases is
pre-sorted and uses firstOrNull, which can break if order isn't guaranteed;
change its implementation in the DetailsState class to be order-independent by
filtering out pre-releases (use isEffectivelyPreRelease()) and then selecting
the release with the latest timestamp (e.g., use maxByOrNull on
publishedAt/createdAt with a safe null fallback) so it always returns the most
recently published stable release regardless of allReleases ordering.
🤖 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/components/sections/ReleaseChannel.kt`:
- Around line 106-120: The stable quick-action Chip is rendered based only on
showSwitchToStable/state.latestStableRelease but not whether that release
actually has an installable APK; update the render logic to also require an
installable asset before showing the ChannelChip. Concretely, in the block that
references state.latestStableRelease and showSwitchToStable, check the release's
installable asset (e.g. state.latestStableRelease?.installableApk or use an
existing helper like state.canInstall(release) if present) and only create the
ChannelChip when that check succeeds; keep the existing onClick
(DetailsAction.SwitchToStable) but ensure it cannot be triggered for
non-installable releases. This change touches the ChannelChip creation and the
use of state.latestStableRelease / state.canSwitchToStable.
- Around line 84-103: The ChannelChip's contentDescriptionText is static and
doesn't convey the current includeBetas state; update the ChannelChip usage so
contentDescriptionText uses the same conditional label as the visible label
(i.e., reuse the stringResource expression that chooses between
Res.string.channel_chip_include_betas and Res.string.channel_chip_stable_only
based on includeBetas) or build an explicit state-aware string (e.g., "Include
betas, selected"/"Stable only, not selected") so screen readers hear the current
state; modify the ChannelChip call in ReleaseChannel.kt (referencing
ChannelChip, includeBetas, contentDescriptionText, stringResource,
Res.string.channel_chip_include_betas, Res.string.channel_chip_stable_only,
onAction and DetailsAction.ToggleIncludeBetas) to pass that state-aware
description.

In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt`:
- Around line 113-133: The KDoc and implementation disagree on whether tracking
is required; update the canSwitchToStable getter to also require the
DetailsState.isTrackingApp flag (in addition to installedApp and
latestStableRelease) so the affordance only appears for tracked apps; keep the
existing checks (installedIsPreRelease via allReleases lookup and stable.tagName
!= app.installedVersion) but add a short-circuit return false when isTrackingApp
is false to match the documented contract.

In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 530-533: switchToStable() currently selects latestStableRelease
and immediately dispatches InstallPrimary even when that release has no
installable asset (primaryAsset becomes null); change it to first resolve an
installable asset for the current device before calling InstallPrimary.
Specifically, after retrieving _state.value.latestStableRelease (and/or after
dispatching DetailsAction.SelectRelease(stable)), verify that the resolved
primaryAsset (or a helper that finds an installable asset for the release) is
non-null and only then dispatch DetailsAction.InstallPrimary; if no installable
asset exists, bail out (or dispatch a no-op/fallback action) so the quick action
doesn't silently fail.
- Around line 666-676: The resolvedCategory currently falls back to STABLE even
when the selected release is a pre-release; change the fallback logic so
resolvedCategory is derived from the actual selected release instead of blindly
STABLE. Specifically, in the block that computes byPrevCategory, selected and
resolvedCategory, keep using byPrevCategory when non-null, otherwise set
resolvedCategory = PRE_RELEASE if the chosen selected release satisfies
isEffectivelyPreRelease(), or = ALL if not (i.e., no stable or
pre-release-specific match); reference byPrevCategory, selected,
resolvedCategory and isEffectivelyPreRelease() when making the change.

---

Nitpick comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt`:
- Around line 110-111: latestStableRelease currently assumes allReleases is
pre-sorted and uses firstOrNull, which can break if order isn't guaranteed;
change its implementation in the DetailsState class to be order-independent by
filtering out pre-releases (use isEffectivelyPreRelease()) and then selecting
the release with the latest timestamp (e.g., use maxByOrNull on
publishedAt/createdAt with a safe null fallback) so it always returns the most
recently published stable release regardless of allReleases ordering.
🪄 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: cc1588c2-7474-4d32-bd21-473ace3231e2

📥 Commits

Reviewing files that changed from the base of the PR and between 40c6e53 and 8086d99.

📒 Files selected for processing (11)
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/GithubReleaseExt.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt
  • core/presentation/src/commonMain/composeResources/values/strings.xml
  • feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/di/SharedModule.kt
  • feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/system/InstallationManagerImpl.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsAction.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsRoot.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/ReleaseChannel.kt
🚧 Files skipped from review as they are similar to previous changes (2)
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/GithubReleaseExt.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt

Comment on lines +106 to +120
if (showSwitchToStable) {
val stable = state.latestStableRelease
if (stable != null) {
ChannelChip(
label =
stringResource(
Res.string.action_switch_to_stable,
stable.tagName,
),
icon = Icons.Default.Restore,
tint = MaterialTheme.colorScheme.primary,
onClick = { onAction(DetailsAction.SwitchToStable) },
contentDescriptionText = null,
)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't render the stable quick action unless that release is actually installable.

This section keys off state.canSwitchToStable, but that state only checks release identity. If the latest stable has no installable APK for the current device, the chip still appears and the action falls through into a no-op install path. Consider gating this chip on an installable asset as well.

🤖 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/sections/ReleaseChannel.kt`
around lines 106 - 120, The stable quick-action Chip is rendered based only on
showSwitchToStable/state.latestStableRelease but not whether that release
actually has an installable APK; update the render logic to also require an
installable asset before showing the ChannelChip. Concretely, in the block that
references state.latestStableRelease and showSwitchToStable, check the release's
installable asset (e.g. state.latestStableRelease?.installableApk or use an
existing helper like state.canInstall(release) if present) and only create the
ChannelChip when that check succeeds; keep the existing onClick
(DetailsAction.SwitchToStable) but ensure it cannot be triggered for
non-installable releases. This change touches the ChannelChip creation and the
use of state.latestStableRelease / state.canSwitchToStable.

Comment on lines +113 to +133
/**
* True when the install button should expose a "switch to
* stable" rollback affordance: the user is tracking this app,
* is currently on a release that's effectively a pre-release,
* and a distinct stable release exists. The handler
* (`DetailsAction.SwitchToStable`) selects the stable release
* and invokes the normal install path.
*/
val canSwitchToStable: Boolean
get() {
val app = installedApp ?: return false
val stable = latestStableRelease ?: return false
// Is the currently-installed release a pre-release?
val installedIsPreRelease =
allReleases.firstOrNull { it.tagName == app.installedVersion }
?.isEffectivelyPreRelease() == true
if (!installedIsPreRelease) return false
// Don't offer the button if the stable release IS the
// one the user has already (same tag string).
return stable.tagName != app.installedVersion
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

canSwitchToStable docstring vs. implementation disagree on the tracking precondition.

The KDoc at lines 113–120 states the affordance requires that "the user is tracking this app", but the implementation only checks installedApp != null (line 123) and never reads the existing isTrackingApp field (line 57). This means an app that's installed on device but not tracked in the store will still light up the "Switch to stable" button, which contradicts the documented contract and may be surprising given the "Switch to stable" action is presumably store-scoped.

Either drop the tracking claim from the KDoc or gate on it explicitly:

🛡️ Proposed fix — gate on `isTrackingApp`
     val canSwitchToStable: Boolean
         get() {
+            if (!isTrackingApp) return false
             val app = installedApp ?: return false
             val stable = latestStableRelease ?: return false
             // Is the currently-installed release a pre-release?
             val installedIsPreRelease =
                 allReleases.firstOrNull { it.tagName == app.installedVersion }
                     ?.isEffectivelyPreRelease() == true
             if (!installedIsPreRelease) return false
             // Don't offer the button if the stable release IS the
             // one the user has already (same tag string).
             return stable.tagName != app.installedVersion
         }

Additionally, allReleases.firstOrNull { it.tagName == app.installedVersion } silently returns false when the installed tag has rolled off the fetched release window (old pre-release no longer in allReleases). That's a conservative fallback, but worth being aware of — users stuck on an ancient beta won't see the rollback affordance even though logically it still applies.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* True when the install button should expose a "switch to
* stable" rollback affordance: the user is tracking this app,
* is currently on a release that's effectively a pre-release,
* and a distinct stable release exists. The handler
* (`DetailsAction.SwitchToStable`) selects the stable release
* and invokes the normal install path.
*/
val canSwitchToStable: Boolean
get() {
val app = installedApp ?: return false
val stable = latestStableRelease ?: return false
// Is the currently-installed release a pre-release?
val installedIsPreRelease =
allReleases.firstOrNull { it.tagName == app.installedVersion }
?.isEffectivelyPreRelease() == true
if (!installedIsPreRelease) return false
// Don't offer the button if the stable release IS the
// one the user has already (same tag string).
return stable.tagName != app.installedVersion
}
/**
* True when the install button should expose a "switch to
* stable" rollback affordance: the user is tracking this app,
* is currently on a release that's effectively a pre-release,
* and a distinct stable release exists. The handler
* (`DetailsAction.SwitchToStable`) selects the stable release
* and invokes the normal install path.
*/
val canSwitchToStable: Boolean
get() {
if (!isTrackingApp) return false
val app = installedApp ?: return false
val stable = latestStableRelease ?: return false
// Is the currently-installed release a pre-release?
val installedIsPreRelease =
allReleases.firstOrNull { it.tagName == app.installedVersion }
?.isEffectivelyPreRelease() == true
if (!installedIsPreRelease) return false
// Don't offer the button if the stable release IS the
// one the user has already (same tag string).
return stable.tagName != app.installedVersion
}
🤖 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/DetailsState.kt`
around lines 113 - 133, The KDoc and implementation disagree on whether tracking
is required; update the canSwitchToStable getter to also require the
DetailsState.isTrackingApp flag (in addition to installedApp and
latestStableRelease) so the affordance only appears for tracked apps; keep the
existing checks (installedIsPreRelease via allReleases lookup and stable.tagName
!= app.installedVersion) but add a short-circuit return false when isTrackingApp
is false to match the documented contract.

- Update `ChannelChip` accessibility by mirroring the visible label in `contentDescriptionText`, ensuring screen readers announce the current state ("Include betas" / "Stable only").
- Refine `latestStableRelease` logic to use the most recently published stable release instead of the first one found in the list.
- Enhance the "Switch to stable" rollback affordance to only be available if the latest stable release contains an installable asset for the current platform.
- Add defensive checks in `switchToStable` to bail early if no primary installable asset is found, preventing silent no-ops.
- Improve initial release category selection logic to automatically switch to `PRE_RELEASE` if a project only contains pre-releases, avoiding empty lists on start.
- Update `ReleaseInsights` and `DetailsState` to track and propagate `latestStableHasInstallableAsset` status.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/ReleaseChannel.kt (1)

131-174: Stalled-warning month formatting truncates instead of rounding.

days / 30 integer-divides, so a value of 59 days renders as "1 month" and 89 days as "2 months". For a user-facing warning that's framed as a duration estimate, rounding to the nearest month ((days + 15) / 30) reads more accurately. Also note the strings are likely passed Int values with a single placeholder — if your resource format supports plurals, prefer it for "1 month" vs "2 months" grammar across locales.

🤖 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/sections/ReleaseChannel.kt`
around lines 131 - 174, The code uses integer division when computing months
from stalledStableSinceDays, causing truncation (e.g., 59 -> 1); in the UI block
that builds title (see stalledStableSinceDays, days, title and
stringResource(Res.string.stalled_project_warning_months)), change the month
calculation to round to nearest month (e.g., (days + 15) / 30) before passing to
stringResource; additionally, prefer using a plural resource (or locale-aware
plural API) for Res.string.stalled_project_warning_months to handle singular vs
plural correctly across locales.
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt (2)

489-500: Swallowed exception in daysSinceIso.

The catch silently drops any parse failure (returning null is the intended fallback, but losing the cause makes drift in release date formats invisible in production). Static analysis (detekt SwallowedException) flagged this. Log at debug level so the silent fallback is at least traceable when investigating "why is the stalled-warning card not appearing".

♻️ Suggested fix
         return try {
             val published = kotlin.time.Instant.parse(isoTimestamp)
             val now = System.now()
             val diffMs = now.toEpochMilliseconds() - published.toEpochMilliseconds()
             if (diffMs < 0) null else (diffMs / MILLIS_PER_DAY).toInt()
-        } catch (e: Exception) {
+        } catch (e: Exception) {
+            logger.debug("daysSinceIso: failed to parse '$isoTimestamp': ${e.message}")
             null
         }
🤖 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 489 - 500, The daysSinceIso function swallows parsing exceptions,
making format failures invisible; modify daysSinceIso to catch the exception but
log it at debug/trace level (including the isoTimestamp and exception
message/stack) before returning null so behavior is unchanged but failures are
traceable; locate the daysSinceIso function and add a debug log call (via the
existing logger used in this ViewModel or create/use a private logger instance)
inside the catch block, ensuring you still return null when parsing fails.

509-528: toggleIncludeBetas doesn't guard against rapid double-taps.

Each tap kicks off a new checkForUpdates coroutine without cancelling a previous in-flight one. Two quick toggles will result in two concurrent network round-trips whose final DB writes race; whichever finishes last wins, and that may not match the latest user-selected channel. Consider tracking the in-flight job and cancelling/replacing it (similar to aboutTranslationJob/whatsNewTranslationJob already used elsewhere in this VM) so the on-screen result always reflects the user's most recent intent.

🤖 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 509 - 528, toggleIncludeBetas launches unchecked checkForUpdates
coroutines so rapid double-taps can leave a stale DB write winning; add a
cancellable tracked Job (e.g., a private var checkForUpdatesJob: Job? on the
ViewModel) and before launching in toggleIncludeBetas cancel and join the
existing job, then assign the new coroutine to checkForUpdatesJob using
viewModelScope.launch; ensure you still rethrow CancellationException and clear
or replace checkForUpdatesJob on completion/failure (mirroring the pattern used
by aboutTranslationJob/whatsNewTranslationJob) so only the latest toggle's
network/DB work can complete.
core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.kt (1)

145-169: updateInstalledVersion doesn't bump lastUpdatedAt / lastCheckedAt.

The atomic write deliberately scopes to the four version-related columns + isUpdateAvailable, but it skips both lastUpdatedAt and lastCheckedAt. After an external install/sideload, the apps row will continue to show the previous "last updated" timestamp, which is what users typically use to spot a version change in the apps list. Consider including at least lastUpdatedAt = :timestamp (and possibly lastCheckedAt) so the row's metadata stays in sync with the real install event the receiver just processed.

♻️ Suggested addition
     `@Query`(
         """
         UPDATE installed_apps
            SET installedVersion = :installedVersion,
                installedVersionName = :installedVersionName,
                installedVersionCode = :installedVersionCode,
-               isUpdateAvailable = :isUpdateAvailable
+               isUpdateAvailable = :isUpdateAvailable,
+               lastUpdatedAt = :timestamp
          WHERE packageName = :packageName
         """,
     )
     suspend fun updateInstalledVersion(
         packageName: String,
         installedVersion: String,
         installedVersionName: String?,
         installedVersionCode: Long,
         isUpdateAvailable: Boolean,
+        timestamp: Long,
     )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.kt`
around lines 145 - 169, The UPDATE in updateInstalledVersion omits lastUpdatedAt
(and optionally lastCheckedAt), so external installs leave stale timestamps;
update updateInstalledVersion to accept a timestamp parameter (e.g.,
lastUpdatedAt: Long and optionally lastCheckedAt: Long?) and include
"lastUpdatedAt = :lastUpdatedAt" (and "lastCheckedAt = :lastCheckedAt" if
desired) in the SQL SET clause so the row metadata is updated atomically along
with
installedVersion/installedVersionName/installedVersionCode/isUpdateAvailable.
🤖 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/components/sections/ReleaseChannel.kt`:
- Around line 226-258: The ChannelChip's Surface/Row currently uses
Modifier.padding(horizontal = 12.dp, vertical = 8.dp) around a 16.dp icon + 6.dp
spacer + label which yields a touch area under the 48 dp Material minimum;
update the composable (ChannelChip / the Surface modifier) to ensure the hit
target is at least 48.dp by adding Modifier.minimumInteractiveComponentSize()
(or explicitly enforce minHeight = 48.dp) to the Surface/Row modifier, or
replace the custom chip with a Material chip composable such as
AssistChip/FilterChip to inherit correct touch target behavior.

---

Nitpick comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.kt`:
- Around line 145-169: The UPDATE in updateInstalledVersion omits lastUpdatedAt
(and optionally lastCheckedAt), so external installs leave stale timestamps;
update updateInstalledVersion to accept a timestamp parameter (e.g.,
lastUpdatedAt: Long and optionally lastCheckedAt: Long?) and include
"lastUpdatedAt = :lastUpdatedAt" (and "lastCheckedAt = :lastCheckedAt" if
desired) in the SQL SET clause so the row metadata is updated atomically along
with
installedVersion/installedVersionName/installedVersionCode/isUpdateAvailable.

In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/ReleaseChannel.kt`:
- Around line 131-174: The code uses integer division when computing months from
stalledStableSinceDays, causing truncation (e.g., 59 -> 1); in the UI block that
builds title (see stalledStableSinceDays, days, title and
stringResource(Res.string.stalled_project_warning_months)), change the month
calculation to round to nearest month (e.g., (days + 15) / 30) before passing to
stringResource; additionally, prefer using a plural resource (or locale-aware
plural API) for Res.string.stalled_project_warning_months to handle singular vs
plural correctly across locales.

In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 489-500: The daysSinceIso function swallows parsing exceptions,
making format failures invisible; modify daysSinceIso to catch the exception but
log it at debug/trace level (including the isoTimestamp and exception
message/stack) before returning null so behavior is unchanged but failures are
traceable; locate the daysSinceIso function and add a debug log call (via the
existing logger used in this ViewModel or create/use a private logger instance)
inside the catch block, ensuring you still return null when parsing fails.
- Around line 509-528: toggleIncludeBetas launches unchecked checkForUpdates
coroutines so rapid double-taps can leave a stale DB write winning; add a
cancellable tracked Job (e.g., a private var checkForUpdatesJob: Job? on the
ViewModel) and before launching in toggleIncludeBetas cancel and join the
existing job, then assign the new coroutine to checkForUpdatesJob using
viewModelScope.launch; ensure you still rethrow CancellationException and clear
or replace checkForUpdatesJob on completion/failure (mirroring the pattern used
by aboutTranslationJob/whatsNewTranslationJob) so only the latest toggle's
network/DB work can complete.
🪄 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: 3603a6bb-8ba3-45d6-a392-60362bcd2653

📥 Commits

Reviewing files that changed from the base of the PR and between 8086d99 and a9af885.

📒 Files selected for processing (7)
  • core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/InstalledAppsRepository.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt
  • feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/ReleaseChannel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/PackageEventReceiver.kt

Comment on lines +226 to +258
Surface(
shape = RoundedCornerShape(20.dp),
color = MaterialTheme.colorScheme.surfaceContainerHigh,
modifier =
Modifier
.clickable(onClick = onClick)
.then(
if (contentDescriptionText != null) {
Modifier.semantics { contentDescription = contentDescriptionText }
} else {
Modifier
},
),
) {
Row(
verticalAlignment = Alignment.CenterVertically,
modifier = Modifier.padding(horizontal = 12.dp, vertical = 8.dp),
) {
Icon(
imageVector = icon,
contentDescription = null,
tint = tint,
modifier = Modifier.size(16.dp),
)
Spacer(Modifier.size(6.dp))
Text(
text = label,
style = MaterialTheme.typography.labelMedium,
color = tint,
)
}
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

ChannelChip may fall below the 48 dp minimum touch target.

With padding(horizontal = 12.dp, vertical = 8.dp) around a 16 dp icon + 6 dp spacer + label, the resulting hit area is roughly 32 dp tall — below the Material 3 minimum interactive size for touch targets, which can fail accessibility audits and is awkward for users with motor impairments. Consider using Modifier.minimumInteractiveComponentSize() or a Material chip composable (AssistChip/FilterChip) which handle this automatically.

🤖 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/sections/ReleaseChannel.kt`
around lines 226 - 258, The ChannelChip's Surface/Row currently uses
Modifier.padding(horizontal = 12.dp, vertical = 8.dp) around a 16.dp icon + 6.dp
spacer + label which yields a touch area under the 48 dp Material minimum;
update the composable (ChannelChip / the Surface modifier) to ensure the hit
target is at least 48.dp by adding Modifier.minimumInteractiveComponentSize()
(or explicitly enforce minHeight = 48.dp) to the Surface/Row modifier, or
replace the custom chip with a Material chip composable such as
AssistChip/FilterChip to inherit correct touch target behavior.

@rainxchzed rainxchzed merged commit 3c77daf into main Apr 25, 2026
1 check passed
@rainxchzed rainxchzed deleted the app-version-notify branch April 25, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant