feat: include starred repos in auto-link suggestions#686
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds starred-repo matching to the external import flow: data fetches starred repos, tokenizes and scores them against candidates, emits STARRED suggestions; domain, presentation, localization, and release notes updated to expose and render the new source. ChangesStarred Repository Match Feature
Sequence DiagramsequenceDiagram
participant ImportVM as ExternalImportViewModel
participant ExternalImportRepository
participant StarredRepository
participant MatchScoring as Match Scoring
ImportVM->>ExternalImportRepository: resolveMatches(...)
ExternalImportRepository->>StarredRepository: getAllStarred()
StarredRepository-->>ExternalImportRepository: starred repo list
ExternalImportRepository->>MatchScoring: build needles & score candidates vs starred repos
MatchScoring->>MatchScoring: normalizeMatchToken(), exact/contains scoring
MatchScoring-->>ExternalImportRepository: starred match suggestions
ExternalImportRepository->>ExternalImportRepository: combine with other sources, sort, dedupe
ExternalImportRepository-->>ImportVM: resolved suggestions (including STARRED)
ImportVM->>ImportVM: toUi() map STARRED to SuggestionSource.STARRED
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/ExternalImportRepositoryImpl.kt`:
- Around line 270-272: The current use of runCatching around
starredRepository.getAllStarred().first() in ExternalImportRepositoryImpl (the
starred variable) swallows CancellationException and converts cancellations into
emptyList(); change it to explicitly preserve cancellation by replacing
runCatching with a try/catch that rethrows CancellationException and only
catches/logs non-cancellation exceptions, returning emptyList() on those; locate
the starredRepository.getAllStarred().first() call in
ExternalImportRepositoryImpl.kt and implement the try { val starred = ... }
catch (e: CancellationException) { throw e } catch (e: Exception) { Logger.d {
"starred fetch for match failed: ${e.message}" }; emptyList() } pattern.
In `@core/presentation/src/commonMain/composeResources/values-pl/strings-pl.xml`:
- Line 598: The Polish label for the starred source is ambiguous; update the
string resource named match_source_starred to use a clearer phrase for source
provenance by replacing its value "Z gwiazdką" with "Oznaczone gwiazdką" so it
matches the rest of the locale and reads unambiguously in the UI.
In `@core/presentation/src/commonMain/composeResources/values-ru/strings-ru.xml`:
- Line 598: The Russian label for the starred source is awkward; update the
string resource with name "match_source_starred" to use the consistent
terminology by replacing its value "Со звездой" with "Из избранного" so it
matches other locale strings that use "избранные" for starred repos.
In `@docs/release-notes/1.9.0.md`:
- Around line 5-48: The English release notes are missing the starred-repo
matching feature; insert a new subsection titled "### 🌟 Starred Repository
Matching" into the "New Features" section immediately after the "Desktop
Maturity" subsection and add the three bullets: "Linking installed apps now
matches against **your starred repositories**", "'Scan GitHub Apps'
automatically suggests starred repos as candidates", and "Reduces manual URL
pasting when linking apps you've already starred" so the English markdown
mirrors the localized JSON entries.
- Line 3: The document uses an h3 heading ("### The biggest visual overhaul
since launch. New Geist typography, hero app headers, redesigned Home cards with
platform glyphs, refreshed Library with updates banner + \"Ready to install\",
Apple-style menus everywhere. Tablet two-pane lands. Inner Details pages get
dedicated screens. Desktop finally feels native — window state persists, Windows
11 + macOS dark title bars, fluid content widths. Root installs work again on
modern Magisk via libsu rewrite.") immediately after the h1, skipping h2; change
that h3 to h2 (replace the leading "###" with "##") in
docs/release-notes/1.9.0.md so heading levels increment by one and restore
correct document structure and accessibility.
🪄 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: 2c85cba7-4ee5-4eaa-b33e-4606bc4cdf47
📒 Files selected for processing (33)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/di/SharedModule.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/ExternalImportRepositoryImpl.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/system/RepoMatchSource.ktcore/presentation/src/commonMain/composeResources/files/whatsnew/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ar/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/bn/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/es/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/fr/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/hi/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/it/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ja/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ko/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/pl/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ru/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/tr/19.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/19.jsoncore/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.xmldocs/release-notes/1.9.0.mdfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/LinkAppBottomSheet.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/import/ExternalImportViewModel.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/import/model/RepoSuggestionUi.kt
| <string name="match_source_fingerprint">Подпись</string> | ||
| <string name="match_source_search">Поиск</string> | ||
| <string name="match_source_manual">Вручную</string> | ||
| <string name="match_source_starred">Со звездой</string> |
Contributor
There was a problem hiding this comment.
Russian label is awkward and inconsistent with existing terminology.
Со звездой reads unnatural here. This locale already uses “избранные” for starred repos, so the source chip should be aligned (e.g., Из избранного).
Suggested fix
- <string name="match_source_starred">Со звездой</string>
+ <string name="match_source_starred">Из избранного</string>📝 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
| <string name="match_source_starred">Со звездой</string> | |
| <string name="match_source_starred">Из избранного</string> |
🤖 Prompt for 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.
In `@core/presentation/src/commonMain/composeResources/values-ru/strings-ru.xml`
at line 598, The Russian label for the starred source is awkward; update the
string resource with name "match_source_starred" to use the consistent
terminology by replacing its value "Со звездой" with "Из избранного" so it
matches other locale strings that use "избранные" for starred repos.
Member
Author
There was a problem hiding this comment.
Declined — this would conflate two distinct features. The app separates starred (StarredRepository, "звёзды") from favourites (FavouritesRepository, "избранное"). This chip is GitHub-starred provenance, so it stays in the star vocabulary (Со звездой) rather than Из избранного.
Contributor
There was a problem hiding this comment.
@rainxchzed, understood — that's a clear and important distinction. Keeping Со звездой is correct here since it maps to GitHub stars (StarredRepository / "звёзды") and must not be conflated with local favourites (FavouritesRepository / "избранное"). I'll note this for future reviews.
🐰 ears droop slightly
✏️ Learnings added
Learnt from: rainxchzed
Repo: OpenHub-Store/GitHub-Store PR: 686
File: core/presentation/src/commonMain/composeResources/values-ru/strings-ru.xml:598-598
Timestamp: 2026-05-29T06:09:30.341Z
Learning: In the GitHub Store app (Kotlin Multiplatform), there are two distinct and separate features:
1. **Starred** (`StarredRepository`, "звёзды") — GitHub-starred repositories, referenced via `RepoMatchSource.STARRED` and `SuggestionSource.STARRED`. The Russian label for this is "Со звездой" (match_source_starred in strings-ru.xml).
2. **Favourites** (`FavouritesRepository`, "избранное") — locally saved favourite repositories.
Do NOT conflate these two in translations or code. In Russian, "звёзды/Со звездой" = GitHub stars, "избранное" = local favourites. The string `match_source_starred` correctly uses "Со звездой" and should NOT be changed to "Из избранного".
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| @@ -0,0 +1,52 @@ | |||
| # 🚀 GitHub Store 1.9 — Design Refresh & Desktop Maturity | |||
|
|
|||
| ### The biggest visual overhaul since launch. New Geist typography, hero app headers, redesigned Home cards with platform glyphs, refreshed Library with updates banner + "Ready to install", Apple-style menus everywhere. Tablet two-pane lands. Inner Details pages get dedicated screens. Desktop finally feels native — window state persists, Windows 11 + macOS dark title bars, fluid content widths. Root installs work again on modern Magisk via libsu rewrite. | |||
Contributor
There was a problem hiding this comment.
Fix heading level skip.
Line 3 uses h3 (###) directly after the h1 title on line 1, skipping h2. Markdown heading levels should increment by one level at a time for proper document structure and accessibility.
📝 Proposed fix
-### The biggest visual overhaul since launch. New Geist typography, hero app headers, redesigned Home cards with platform glyphs, refreshed Library with updates banner + "Ready to install", Apple-style menus everywhere. Tablet two-pane lands. Inner Details pages get dedicated screens. Desktop finally feels native — window state persists, Windows 11 + macOS dark title bars, fluid content widths. Root installs work again on modern Magisk via libsu rewrite.
+## The biggest visual overhaul since launch. New Geist typography, hero app headers, redesigned Home cards with platform glyphs, refreshed Library with updates banner + "Ready to install", Apple-style menus everywhere. Tablet two-pane lands. Inner Details pages get dedicated screens. Desktop finally feels native — window state persists, Windows 11 + macOS dark title bars, fluid content widths. Root installs work again on modern Magisk via libsu rewrite.📝 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
| ### The biggest visual overhaul since launch. New Geist typography, hero app headers, redesigned Home cards with platform glyphs, refreshed Library with updates banner + "Ready to install", Apple-style menus everywhere. Tablet two-pane lands. Inner Details pages get dedicated screens. Desktop finally feels native — window state persists, Windows 11 + macOS dark title bars, fluid content widths. Root installs work again on modern Magisk via libsu rewrite. | |
| ## The biggest visual overhaul since launch. New Geist typography, hero app headers, redesigned Home cards with platform glyphs, refreshed Library with updates banner + "Ready to install", Apple-style menus everywhere. Tablet two-pane lands. Inner Details pages get dedicated screens. Desktop finally feels native — window state persists, Windows 11 + macOS dark title bars, fluid content widths. Root installs work again on modern Magisk via libsu rewrite. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for 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.
In `@docs/release-notes/1.9.0.md` at line 3, The document uses an h3 heading ("###
The biggest visual overhaul since launch. New Geist typography, hero app
headers, redesigned Home cards with platform glyphs, refreshed Library with
updates banner + \"Ready to install\", Apple-style menus everywhere. Tablet
two-pane lands. Inner Details pages get dedicated screens. Desktop finally feels
native — window state persists, Windows 11 + macOS dark title bars, fluid
content widths. Root installs work again on modern Magisk via libsu rewrite.")
immediately after the h1, skipping h2; change that h3 to h2 (replace the leading
"###" with "##") in docs/release-notes/1.9.0.md so heading levels increment by
one and restore correct document structure and accessibility.
Comment on lines
+5
to
+48
| ## ✨ New Features | ||
|
|
||
| ### 🎨 Design Overhaul | ||
| - New **Geist** typography across the app | ||
| - Hero app header on Details with clickable owner avatar + ✓ verification badge | ||
| - Redesigned Home cards now show **every platform** a repo ships installers for | ||
| - Refreshed Library with **Updates banner** and **Ready to install** section | ||
| - Apple-style dropdown menus (`GhsDropdownMenu`) across all overflow surfaces | ||
| - Real Apple + Tux icons for macOS / Linux platform indicators | ||
| - Native Compose contribution calendar on developer profiles (hidden for orgs) | ||
| - Clickable @mentions and clickable company in bios | ||
|
|
||
| ### 📱 Tablet Two-Pane | ||
| - Home / Search / Library list on the left, repo opens on the right | ||
| - Draggable divider, persists across sessions | ||
| - Inner Details (About, What's New) slide *within* the right pane | ||
|
|
||
| ### 🖥 Desktop Maturity | ||
| - **Window state persists** — size, position, maximized survive across launches (#664) | ||
| - **Windows 11 + macOS** title bars match system dark mode (#663) | ||
| - **Fluid content width** — Compact / Wide / Extra wide scale as 55% / 75% / 95% of window | ||
| - Real GitHub Store logo in side drawer (no more "G" placeholder) | ||
| - MenuBar with About / Feedback / Licenses / Privacy | ||
|
|
||
| ## 🐛 Bug Fixes | ||
| - **Mirror + direct download race** corrupting destination file (#667) | ||
| - **Root installer** on Android 14+ / Magisk 27+ rewritten on **libsu** (#651) | ||
| - Linked apps now show **"Update to X"** immediately instead of stale "Install vX" | ||
| - Multi-flavor APK repos no longer show false "Update" CTA (#638) | ||
| - Long release tags no longer wrap into one-char vertical date columns | ||
| - URL paste verifies repo exists before showing match | ||
| - Dynamic color contrast on Search TextField across all palettes | ||
| - README + release-notes keep scroll position on return | ||
|
|
||
| ## ⚡ Performance | ||
| - Markdown no longer re-renders on every download progress tick (~10×/sec → 0) | ||
| - Chunked progressive markdown — large READMEs paint first screen in <100ms | ||
| - `LazyColumn.animateItem()` across discovery / search / library | ||
| - Direction-aware bottom-nav transitions based on tab index | ||
|
|
||
| ## 🧹 Cleanups | ||
| - Anonymous telemetry removed completely | ||
| - App info moved from Tweaks → Profile (where users look for it) | ||
| - Discovery platforms moved into Tweaks → Sources |
Contributor
There was a problem hiding this comment.
Add missing starred repository matching feature documentation.
The primary feature of this PR—starred repository matching in the external app import workflow—is documented in all 12 localized JSON release notes (line 15 in each file) but is completely absent from this English markdown release notes file. Users reading docs/release-notes/1.9.0.md will not learn about this significant new capability.
📝 Suggested addition to the "New Features" section
Insert a new subsection after "Desktop Maturity" (around line 27):
### 🌟 Starred Repository Matching
- Linking installed apps now matches against **your starred repositories**
- "Scan GitHub Apps" automatically suggests starred repos as candidates
- Reduces manual URL pasting when linking apps you've already starred🤖 Prompt for 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.
In `@docs/release-notes/1.9.0.md` around lines 5 - 48, The English release notes
are missing the starred-repo matching feature; insert a new subsection titled
"### 🌟 Starred Repository Matching" into the "New Features" section immediately
after the "Desktop Maturity" subsection and add the three bullets: "Linking
installed apps now matches against **your starred repositories**", "'Scan GitHub
Apps' automatically suggests starred repos as candidates", and "Reduces manual
URL pasting when linking apps you've already starred" so the English markdown
mirrors the localized JSON entries.
Greptile SummaryThis PR folds starred-repo matching into the existing
Confidence Score: 5/5Safe to merge — the starred matching logic is well-guarded by the skip-threshold, GENERIC_MATCH_TOKENS, and the latestReleaseUrl sentinel; dropSuggestionsWithoutReleases is fail-safe. The starred scoring, deduplication ordering fix, and StarredRepositoryImpl changes are all correct. Omitting BACKEND from RELEASE_VERIFY_SKIP_SOURCES causes redundant re-verification of backend-matched repos, increasing resolve latency, but doesn't produce wrong results. All locales and UI enum cases are fully wired. ExternalImportRepositoryImpl.kt — specifically the RELEASE_VERIFY_SKIP_SOURCES set and whether BACKEND should be added to avoid redundant API calls on every resolveMatches() invocation. Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as LinkAppBottomSheet
participant VM as ExternalImportViewModel
participant Repo as ExternalImportRepositoryImpl
participant StarredDB as StarredRepository (DB)
participant BackendAPI as BackendApiClient
participant ForgejoAPI as ForgejoClientRegistry
UI->>VM: triggerResolve(candidates)
VM->>Repo: resolveMatches(candidates)
Repo->>BackendAPI: match(batch) [BACKEND]
BackendAPI-->>Repo: backendResults
Repo->>ForgejoAPI: searchRepositories(...) [FORGEJO_SEARCH, if eligible]
ForgejoAPI-->>Repo: forgejoHits
Note over Repo: bestConfidence < 0.7 - starredEligible
Repo->>StarredDB: getAllStarred().first() [if starredEligible non-empty]
StarredDB-->>Repo: starred list
Note over Repo: starredMatches() scores each starred repo
Repo->>Repo: sort-then-distinctBy(suggestionKey)
Repo->>Repo: dropSuggestionsWithoutReleases(rawResults)
Note over Repo: BACKEND + FORGEJO_SEARCH verified
Repo->>BackendAPI: getReleases(owner,repo) [up to 60 calls, 8 concurrent]
BackendAPI-->>Repo: verdicts
Repo-->>VM: List RepoMatchResult
VM-->>UI: suggestions with STARRED chip
Reviews (5): Last reviewed commit: "fix(import): skip release check for iden..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Member
Author
|
@coderabbitai Pushed fixes for the review: Fixed (commit
Fixed (commit
Won't fix
Compile-verified |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Acknowledged fixes:
Won't-fix decisions:
(ᵔ◡ᵔ)🐇⭐ ✏️ Learnings added
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/StarredRepositoryImpl.kt (1)
219-246: ⚡ Quick winReuse
matchesPlatform()in the GitHub fallback to avoid divergence.The inline
when (platform)filter here is a verbatim duplicate ofmatchesPlatform()(lines 165-174), which the backend path already uses at line 186. Consolidating keeps platform-extension logic in a single place so the two paths can't drift apart.♻️ Proposed refactor
val relevantAssets = stableRelease.assets.filter { asset -> - val name = asset.name.lowercase() - when (platform) { - Platform.ANDROID -> { - name.endsWith(".apk") - } - - Platform.WINDOWS -> { - name.endsWith(".msi") || name.endsWith(".exe") - } - - Platform.MACOS -> { - name.endsWith(".dmg") || name.endsWith(".pkg") - } - - Platform.LINUX -> { - name.endsWith(".appimage") || name.endsWith(".deb") || - name.endsWith(".rpm") || name.endsWith(".pkg.tar.zst") - } - } + matchesPlatform(asset.name) }🤖 Prompt for 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. In `@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/StarredRepositoryImpl.kt` around lines 219 - 246, The asset filter duplicates the platform-matching logic already encapsulated by matchesPlatform(); replace the inline when(platform) block inside the stableRelease.assets.filter in relevantAssets with a call to the existing matchesPlatform() helper (i.e., use matchesPlatform(...) on the asset/name as the helper is defined) so both backend and GitHub-fallback paths use the same logic, keeping StableReleaseInfo(version = stableRelease.tagName, url = stableRelease.htmlUrl) behavior unchanged when relevantAssets.isNotEmpty().
🤖 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.
Nitpick comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/StarredRepositoryImpl.kt`:
- Around line 219-246: The asset filter duplicates the platform-matching logic
already encapsulated by matchesPlatform(); replace the inline when(platform)
block inside the stableRelease.assets.filter in relevantAssets with a call to
the existing matchesPlatform() helper (i.e., use matchesPlatform(...) on the
asset/name as the helper is defined) so both backend and GitHub-fallback paths
use the same logic, keeping StableReleaseInfo(version = stableRelease.tagName,
url = stableRelease.htmlUrl) behavior unchanged when
relevantAssets.isNotEmpty().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9b1344fe-32d3-4417-a683-cb922e64860b
📒 Files selected for processing (2)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/ExternalImportRepositoryImpl.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/StarredRepositoryImpl.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/ExternalImportRepositoryImpl.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/ExternalImportRepositoryImpl.kt (2)
338-355: Consider memoizing release verdicts to avoid repeated network checks.Verification now issues up to
RELEASE_VERIFY_BUDGETrelease lookups on the resolve path, andrescanSinglePackagere-runs this per call, re-verifying the same repos. A short-lived cache (keyed bysuggestionKey, with a TTL) would cut redundantgetReleasestraffic during repeated scans.🤖 Prompt for 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. In `@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/ExternalImportRepositoryImpl.kt` around lines 338 - 355, The release-verdict loop (building verdicts in the coroutineScope using suggestionKey and calling hasInstallableRelease with RELEASE_VERIFY_TIMEOUT_MS) should be backed by a short-lived, coroutine-safe memoization cache keyed by suggestionKey to avoid repeated network getReleases calls across resolve path and rescanSinglePackage; modify the block that computes ok (the runCatching/withTimeoutOrNull around hasInstallableRelease) to first consult the cache and return a cached Boolean if present, otherwise call hasInstallableRelease, store the result with a TTL (e.g., a time-based expiry) and then return it; ensure the cache is safe for concurrent coroutines and that RELEASE_VERIFY_BUDGET/rescanSinglePackage callers reuse it so redundant verifications are skipped.
364-386: ConfirmgetReleasesmodel contract; call out remaining first-page limitation (ExternalImportRepositoryImpl.kt, lines 364-386)
- Both
backendClient.getReleases(...)andforgejoClientRegistry.clientFor(host).getReleases(...)returnResult<List<ReleaseNetwork>>withdraft,prerelease, andassetswhereAssetNetworkexposesname, so the.draft/.prerelease/assets[].namechecks are consistent.hasInstallableReleaseonly fetches the first page (pagedefaults to1in both clients) and only usesRELEASE_VERIFY_PAGE_SIZE = 10, so APK assets that exist only in older releases will be missed and treated as “no installable release”.🤖 Prompt for 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. In `@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/ExternalImportRepositoryImpl.kt` around lines 364 - 386, hasInstallableRelease currently only fetches the first page (using getReleases with default page=1 and RELEASE_VERIFY_PAGE_SIZE=10) and can miss APKs on older pages; update hasInstallableRelease (and the branches calling backendClient.getReleases / forgejoClientRegistry.clientFor(host).getReleases) to paginate until either an installable release is found or no more pages (request subsequent pages using the same perPage/RELEASE_VERIFY_PAGE_SIZE), or explicitly document/return a bounded-check result if pagination isn’t desired; ensure you use the Result<List<ReleaseNetwork>> handling already present (fold onSuccess/onFailure) for each page and stop early when list.any { release -> ... assets.any { it.name.endsWith(".apk", ignoreCase = true) } } is true.
🤖 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/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/ExternalImportRepositoryImpl.kt`:
- Around line 326-330: In dropSuggestionsWithoutReleases(), the current
construction of unique (which only excludes RepoMatchSource.STARRED) causes
MANIFEST and FINGERPRINT suggestions to be included in the first
RELEASE_VERIFY_BUDGET items and subject to hasInstallableRelease() filtering;
change the filter when building unique to also exclude RepoMatchSource.MANIFEST
and RepoMatchSource.FINGERPRINT so identity matches (high-confidence
fingerprint/manifest) are not dropped by the release-check path. Update the
.filter { it.source != RepoMatchSource.STARRED } to exclude the two identity
sources and verify the rest of the logic (suggestionKey(),
RELEASE_VERIFY_BUDGET, hasInstallableRelease()) remains unchanged.
---
Nitpick comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/ExternalImportRepositoryImpl.kt`:
- Around line 338-355: The release-verdict loop (building verdicts in the
coroutineScope using suggestionKey and calling hasInstallableRelease with
RELEASE_VERIFY_TIMEOUT_MS) should be backed by a short-lived, coroutine-safe
memoization cache keyed by suggestionKey to avoid repeated network getReleases
calls across resolve path and rescanSinglePackage; modify the block that
computes ok (the runCatching/withTimeoutOrNull around hasInstallableRelease) to
first consult the cache and return a cached Boolean if present, otherwise call
hasInstallableRelease, store the result with a TTL (e.g., a time-based expiry)
and then return it; ensure the cache is safe for concurrent coroutines and that
RELEASE_VERIFY_BUDGET/rescanSinglePackage callers reuse it so redundant
verifications are skipped.
- Around line 364-386: hasInstallableRelease currently only fetches the first
page (using getReleases with default page=1 and RELEASE_VERIFY_PAGE_SIZE=10) and
can miss APKs on older pages; update hasInstallableRelease (and the branches
calling backendClient.getReleases /
forgejoClientRegistry.clientFor(host).getReleases) to paginate until either an
installable release is found or no more pages (request subsequent pages using
the same perPage/RELEASE_VERIFY_PAGE_SIZE), or explicitly document/return a
bounded-check result if pagination isn’t desired; ensure you use the
Result<List<ReleaseNetwork>> handling already present (fold onSuccess/onFailure)
for each page and stop early when list.any { release -> ... assets.any {
it.name.endsWith(".apk", ignoreCase = true) } } is true.
🪄 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: bb2a983a-6ed7-42da-8923-222427ec0144
📒 Files selected for processing (1)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/ExternalImportRepositoryImpl.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #637 (method 3).
@FumiyaSenro asked that when linking an already-installed app, the app should also look through the user's starred repos — most apps worth tracking are ones they've already starred, and the manual-URL fallback is annoying (nobody remembers repo URLs).
Rather than bloating the link sheet with a separate "pick from starred" step, this folds starred matching into the existing auto-match path.
ExternalImportRepositoryImpl.resolveMatches(the single resolver behind both the "Scan GitHub Apps" link sheet AND the Obtainium import scan) now adds a 5th candidate source:StarredRepository.getAllStarred()once per resolve.RepoMatchSuggestion(source = STARRED, stars, description)merged with manifest / fingerprint / backend / Forgejo results.distinctBykept whichever source ran first, which could drop a higher-confidence starred hit).Both flows surface starred matches automatically — no new screen, no extra button. Suggestions carry a "Starred" provenance chip.
Changes:
RepoMatchSource.STARRED+SuggestionSource.STARRED.ExternalImportRepositoryImplgainsstarredRepositorydep (wired inSharedModule).match_source_starredstring + all 12 locales; chip label inLinkAppBottomSheet.Compile-verified
:core:data+:feature:apps:presentation(Android) +:composeApp(jvm). Method 2 (reworking the "Add from Starred" screen to link-in-place) intentionally left out to keep this scoped.Summary by CodeRabbit