fix(updates): sibling-app detection — Layer 5 stem filter (#591)#592
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR adds asset base-stem extraction and incorporates it into release resolution to disambiguate updates between sibling apps sharing a repository. The resolver now accepts the installed asset name, computes its stem by filtering version and platform tokens, and narrows candidate selection to assets with matching stems. ChangesAuto-Update Disambiguation by Asset Stem
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Greptile SummaryThis PR introduces a stem-based sibling-app filter (Layer 5) to prevent
Confidence Score: 4/5The core logic is sound and the fallback prevents broken updates, but the stem filter is silently disabled for any user whose installed build has a pre-release qualifier in its filename. The sibling-app fix works correctly for stable-release filenames, which are the common case. However, core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/AssetVariant.kt — the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[resolveTrackedRelease called\nwith installedAssetName] --> B[installableForApp\nfiltered by installer]
B --> C{Layers 1-4\nfingerprint / position match?}
C -- yes --> H[Return matched asset]
C -- no --> D[filterByPackageFlavor\nnarrows by package flavor]
D --> E{installedStem non-empty?}
E -- no --> G[choosePrimaryAsset\nfull flavor pool]
E -- yes --> F{Any pool asset\nshares stem?}
F -- yes --> G2[choosePrimaryAsset\nstem-filtered pool]
F -- no fallback --> G
G --> H
G2 --> H
Reviews (2): Last reviewed commit: "fix(stem): consume compound-vocab fragme..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/AssetVariant.kt`:
- Around line 278-289: extractBaseStem is leaving split architecture pieces like
"v8a"/"v7a" after tokenization (e.g., "arm64-v8a" → ["arm64","v8a"]) so stems
differ; fix by updating the ARCH_TOKENS constant (used by extractBaseStem) to
include the individual arch components such as "v8a" and "v7a" (and any other
split pieces you observe) so they are filtered out by the existing filterNot
check; ensure the tokens you add match the tokenization/normalization logic so
extractBaseStem produces the expected common stem for architecture variants.
🪄 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: 225728ef-6806-4b64-8d24-fd92eeb8ad39
📒 Files selected for processing (3)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/AssetVariant.ktcore/presentation/src/commonMain/composeResources/files/whatsnew/17.json
| private fun isVersionLikeToken(token: String): Boolean { | ||
| if (token.isEmpty()) return false | ||
| if (token.all { it.isDigit() }) return true | ||
| if (token.startsWith("v") && token.drop(1).all { it.isDigit() }) return true | ||
| return false | ||
| } |
There was a problem hiding this comment.
isVersionLikeToken misses pre-release qualifier tokens
After tokenize() splits on - and ., a filename like app-2.0-rc2.apk becomes ["app","2","0","rc2"]. The token rc2 is not all-digit, doesn't match v+digit, and isn't in VOCABULARY, so it leaks into the stem: installed app-1.0-rc2.apk → stem "apprc2", but new release app-2.0.apk → stem "app". These never match, the filter always falls back to the unfiltered pool, and the original sibling-app confusion bug reappears for every user who has a pre-release build installed. The KDoc explicitly lists 1.0-rc1 and beta3 as examples the function handles, but neither survives to true with the current implementation.
| private fun isVersionLikeToken(token: String): Boolean { | |
| if (token.isEmpty()) return false | |
| if (token.all { it.isDigit() }) return true | |
| if (token.startsWith("v") && token.drop(1).all { it.isDigit() }) return true | |
| return false | |
| } | |
| private fun isVersionLikeToken(token: String): Boolean { | |
| if (token.isEmpty()) return false | |
| if (token.all { it.isDigit() }) return true | |
| if (token.startsWith("v") && token.drop(1).all { it.isDigit() }) return true | |
| // Pre-release qualifiers: rc1, rc2, alpha1, beta3, pre1, build456 … | |
| // Targeted pattern avoids false-positives on real app-name tokens | |
| // (e.g. `app2` has no common pre-release prefix and is left intact). | |
| if (PRE_RELEASE_QUALIFIER_TOKEN.matches(token)) return true | |
| return false | |
| } | |
| private val PRE_RELEASE_QUALIFIER_TOKEN = Regex("""^(rc|alpha|beta|pre|build)\d+$""") |
1 participant
Issue #591 — auto-update flagged
APP B-2.20.apkas a higher version of anAPP A-1.10.apkinstalled from the same repo, because Layer 5 (installer.choosePrimaryAsset) picks the highest-version asset from the auto-pick pool without checking that the candidate is actually the same app the user installed.What landed:
AssetVariant.extractBaseStem(name)— lowercased, separator-stripped concatenation of every token that isn't a version-like number, arch token, or flavor token.APP A-1.10.apk → "appa",APP B-2.20.apk → "appb",app-arm64-v8a-1.10.apk and app-x86_64-1.10.apk → both "app". Empty stem (e.g.2.0.apk) means "no signal — don't filter".resolveTrackedReleaseLayer 5 now receivesinstalledAssetName(already onInstalledAppEntity). When the stem of the installed asset is non-empty, the auto-pick pool is filtered to assets with matching stem beforechoosePrimaryAssetruns.filterByPackageFlavorpool so updates still flow.Not compile-verified locally —
~/.gradleSSD unmounted in this session. Visual review only. Please compile-check before merge::core:domain,:core:data,:composeApp:compileDebugKotlinAndroid.Closes #591.
Summary by CodeRabbit