Integrate BackendApiClient for optimized repository discovery and search#425
Integrate BackendApiClient for optimized repository discovery and search#425rainxchzed merged 2 commits intomainfrom
Conversation
- Introduce `BackendApiClient` to interface with the centralized GitHub Store API, featuring dedicated endpoints for categories, topics, and search. - Update `CachedRepositoriesDataSourceImpl` to prioritize backend fetching for home categories and topics, with a fallback to raw GitHub JSON files if the backend is unavailable. - Implement `SearchRepositoryImpl` logic to attempt backend searches first, providing faster results and native platform filtering, before falling back to manual GitHub REST API search and verification. - Add DTOs and mappers (`BackendRepoResponse`, `BackendSearchResponse`, `BackendRepoMapper`) to handle backend data structures and convert them to domain/cached models. - Configure dependency injection for `BackendApiClient` across core, home, and search modules. - Refactor `CachedRepositoriesDataSourceImpl` to simplify platform and category slug handling via helper methods.
WalkthroughThis PR adds a BackendApiClient with DTOs and mappers, wires it into DI, and changes home/search data layers to prefer backend responses (with caching) before falling back to GitHub REST calls. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Cache
participant Backend as BackendApiClient
participant GitHub as GitHub API
participant Mapper as Domain Mapper
Client->>Cache: Request repos (category/search)
alt Cache hit
Cache-->>Client: Return cached repos
else Cache miss
Client->>Backend: Query backend (category/topic/search)
alt Backend success
Backend-->>Mapper: Return BackendRepoResponse(s)
Mapper-->>Cache: Map & store cached results
Cache-->>Client: Return repos
else Backend failure / empty
Client->>GitHub: Query GitHub REST
GitHub-->>Mapper: Return raw GitHub data
Mapper-->>Cache: Map & store cached results (if any)
Cache-->>Client: Return repos
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/data_source/impl/CachedRepositoriesDataSourceImpl.kt (1)
44-52:⚠️ Potential issue | 🟠 MajorCreate fallback HTTP client using
HttpClientFactorypattern and implement proper lifecycle management.The
fallbackHttpClientcreated at lines 44-52 is never closed. In a singleton-scoped data source, this creates a resource leak for the application lifetime. More critically, it violates the established pattern: HTTP clients incore/datamust be configured viaHttpClientFactory(or the equivalent factory functioncreateGitHubHttpClient) with proper interceptors for timeout, retry, and error handling.Consider either:
- Injecting a pre-configured
HttpClientfrom the DI container, or- Implementing
AutoCloseableon this class and wiring aclose()method into the Koin singleton lifecycle, or- Using
createGitHubHttpClient()(or a similar factory function) to configure the client with the required interceptors and timeout policies, then managing its lifecycle explicitly.Note:
BackendApiClientincore/datahas the same pattern violation and should also be refactored to use the factory.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/data_source/impl/CachedRepositoriesDataSourceImpl.kt` around lines 44 - 52, The fallbackHttpClient created inside CachedRepositoriesDataSourceImpl leaks because it's constructed directly and never closed and also bypasses the project's HttpClientFactory/createGitHubHttpClient conventions; fix by replacing the direct HttpClient creation with an injected, pre-configured HttpClient from DI (or call createGitHubHttpClient()/HttpClientFactory to build it) and ensure lifecycle is managed—either implement AutoCloseable on CachedRepositoriesDataSourceImpl and add a close() that disposes the client or wire the client injection into Koin as a singleton so Koin manages its lifecycle; apply the same refactor pattern to BackendApiClient to use the shared factory/injection and proper close handling.
🧹 Nitpick comments (5)
.claude/settings.local.json (1)
14-14: Overly broad wildcard permission.
./gradlew build *permits any arguments appended togradlew build, which is much broader than the other explicit entries in this allowlist. Consider replacing with the specificbuildinvocations actually needed, to keep the permission surface tight and consistent with the rest of the file.Also worth noting:
.claude/settings.local.jsonis typically a per-developer local file (hence the.localsuffix) and is usually gitignored rather than committed. If that's the intent here, consider adding it to.gitignoreinstead of tracking personal permission grants in the repo.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/settings.local.json at line 14, Replace the overly broad allowlist entry "Bash(./gradlew build *)" in settings.local.json with the specific gradle invocations your CI/dev workflow actually needs (e.g., "Bash(./gradlew build)", "Bash(./gradlew build --no-daemon)" or whatever concrete flags are required) to remove the wildcard; also consider removing this per-developer .local file from version control and add .claude/settings.local.json to .gitignore so personal permission grants are not committed.feature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/data_source/impl/CachedRepositoriesDataSourceImpl.kt (1)
168-186: Duplicate platform-slug mapping; prefer the newtoApiSlug()helper.The hardcoded
listOf("android", "windows", "macos", "linux")plus the inlinewhen (plat) -> DiscoveryPlatform.Xblock duplicates the newDiscoveryPlatform.toApiSlug()helper defined at the bottom of this file (also duplicated infetchAllPlatformsTopicFromBackend). Iterating overDiscoveryPlatform.entries.filter { it != DiscoveryPlatform.All }and reusing the helper eliminates theelse -> return@async nulldead branch and keeps the mapping in one place.♻️ Suggested refactor
- val platforms = listOf("android", "windows", "macos", "linux") - val responses = coroutineScope { - platforms.map { plat -> - async { - val r = backendApiClient.getCategory(categorySlug, plat) - val discoveryPlatform = when (plat) { - "android" -> DiscoveryPlatform.Android - "windows" -> DiscoveryPlatform.Windows - "macos" -> DiscoveryPlatform.Macos - "linux" -> DiscoveryPlatform.Linux - else -> return@async null - } - r.getOrNull()?.map { - it.toCachedGithubRepoSummary() - .copy(availablePlatforms = listOf(discoveryPlatform)) - } - } - }.awaitAll().filterNotNull() - } + val platforms = DiscoveryPlatform.entries.filter { it != DiscoveryPlatform.All } + val responses = coroutineScope { + platforms.map { dp -> + async { + val slug = dp.toApiSlug() ?: return@async null + backendApiClient.getCategory(categorySlug, slug).getOrNull()?.map { + it.toCachedGithubRepoSummary() + .copy(availablePlatforms = listOf(dp)) + } + } + }.awaitAll().filterNotNull() + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/data_source/impl/CachedRepositoriesDataSourceImpl.kt` around lines 168 - 186, Replace the hardcoded platform list and the inline when mapping in the coroutine block with an iteration over DiscoveryPlatform.entries.filter { it != DiscoveryPlatform.All } and call eachEntry.toApiSlug() when calling backendApiClient.getCategory; use the entry itself (e.g., discoveryPlatform) to set availablePlatforms instead of mapping strings, remove the else -> return@async null dead branch, and rely on the existing DiscoveryPlatform.toApiSlug() helper so the mapping is centralized (affects the async map that calls backendApiClient.getCategory and the subsequent .toCachedGithubRepoSummary().copy(availablePlatforms = ...)).feature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt (1)
150-158: Nit: importProducerScopeinstead of fully qualifying the receiver.The fully qualified
kotlinx.coroutines.channels.ProducerScope<...>receiver is unusual in this codebase. Adding an import keeps the signature readable.♻️ Suggested refactor
+import kotlinx.coroutines.channels.ProducerScope @@ - private suspend fun kotlinx.coroutines.channels.ProducerScope<PaginatedDiscoveryRepositories>.fallbackGithubSearch( + private suspend fun ProducerScope<PaginatedDiscoveryRepositories>.fallbackGithubSearch(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt` around lines 150 - 158, The function receiver is fully qualified as kotlinx.coroutines.channels.ProducerScope<PaginatedDiscoveryRepositories>; replace that with a simple imported ProducerScope to improve readability: add an import for kotlinx.coroutines.channels.ProducerScope at the top and change the receiver type in fallbackGithubSearch to ProducerScope<PaginatedDiscoveryRepositories> (function name: fallbackGithubSearch, type: PaginatedDiscoveryRepositories) so the signature is consistent with the rest of the codebase.core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt (1)
34-74: Consider extracting theisSuccess/BackendExceptionboilerplate.All three endpoints repeat the same status-check/body-decode/failure pattern. A small private helper (e.g.
suspend inline fun <reified T> HttpResponse.toResult(): Result<T>) would remove the duplication and make it harder to forget the status check on future endpoints. Non-blocking nit.🤖 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/network/BackendApiClient.kt` around lines 34 - 74, Extract the repeated status/body/failure logic into a private helper extension like a suspend inline fun <reified T> HttpResponse.toResult(): Result<T> that checks response.status.isSuccess(), returns Result.success(response.body()) or Result.failure(BackendException("HTTP ${response.status.value}")), then call this helper from getCategory, getTopic and search (replace the inline status-check blocks after each httpClient.get). Ensure the helper is used for all return types (List<BackendRepoResponse> and BackendSearchResponse) so future endpoints won’t duplicate the isSuccess/BackendException pattern.core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendRepoResponse.kt (1)
27-30: Minor: consider a nested installers object or explicit snake_case mapping.Four parallel
hasInstallersXbooleans are slightly awkward and will be especially fragile if the backend returns them nested (e.g.installers: { android, windows, macos, linux }) or snake-cased (has_installers_android). If the wire format differs from these names, annotate with@SerialNameor model aBackendInstallerssub-object to keep the mapper straightforward. Non-blocking.🤖 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/dto/BackendRepoResponse.kt` around lines 27 - 30, The four parallel booleans in BackendRepoResponse (hasInstallersAndroid, hasInstallersWindows, hasInstallersMacos, hasInstallersLinux) should be made robust to wire-format differences: either replace them with a nested data class (e.g., add data class BackendInstallers(val android: Boolean = false, val windows: Boolean = false, val macos: Boolean = false, val linux: Boolean = false) and change BackendRepoResponse to expose val installers: BackendInstallers = BackendInstallers()), or add `@SerialName` annotations to each existing property to match the expected snake_case keys (e.g., annotate hasInstallersAndroid with `@SerialName`("has_installers_android"), etc.); update any mapper code that reads these fields to use the new installers property or annotated names accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendSearchResponse.kt`:
- Around line 6-11: BackendSearchResponse is missing `@SerialName` mappings for
snake_case API fields: add `@SerialName`("total_hits") to the totalHits property
and `@SerialName`("processing_time_ms") to processingTimeMs; likewise update
BackendRepoResponse to annotate its snake_case counterparts (e.g.,
`@SerialName`("stargazers_count") on stargazersCount, `@SerialName`("forks_count")
on forksCount, `@SerialName`("default_branch") on defaultBranch, etc.) so the
Kotlinx.serialization deserializer maps backend fields to the camelCase
properties in BackendSearchResponse and BackendRepoResponse.
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt`:
- Around line 19-32: The HttpClient is being instantiated directly (private val
httpClient = HttpClient { ... }) which bypasses the repo-wide HttpClientFactory;
switch to obtaining a shared, preconfigured client from HttpClientFactory (or
inject it via Koin) instead of new HttpClient(), then apply only
backend-specific adjustments (e.g., set BASE_URL defaultRequest and any timeout
overrides) onto that client or via a lightweight wrapper; update references to
httpClient and preserve expectSuccess behavior while removing duplicate plugin
installs so auth/logging/engine selection remain centralized.
- Around line 19-22: Backend response DTOs (BackendRepoResponse,
BackendSearchResponse) declare camelCase properties like fullName, htmlUrl,
stargazersCount, hasInstallersAndroid while the HttpClient Json config only sets
ignoreUnknownKeys = true, so snake_case fields from the backend will not map
correctly; fix by adding `@SerialName`("snake_case_name") annotations to each
mismatched property in those data classes (e.g., fullName -> "full_name",
htmlUrl -> "html_url", stargazersCount -> "stargazers_count",
hasInstallersAndroid -> "has_installers_android") following the existing pattern
used by GithubRepoNetworkModel/UserProfileNetwork, or alternatively opt into
ExperimentalSerializationApi and set namingStrategy =
JsonNamingStrategy.SnakeCase on the Json configuration in the HttpClient setup
to apply snake_case mapping globally.
In
`@feature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt`:
- Around line 239-243: The current catch block in SearchRepositoryImpl rethrows
only RateLimitException and CancellationException, causing all other exceptions
to escape the channelFlow and terminate the flow; either remove this redundant
try/catch entirely (so exceptions propagate uniformly as before) or add a
generic catch(Exception) that handles transient/parse errors the same way the
previous code did (e.g., emit an empty terminal page or a specific error state
into the flow) instead of letting the flow crash; update the catch surrounding
the RateLimitException/CancellationException in SearchRepositoryImpl accordingly
so behavior matches the old graceful degradation.
- Around line 135-145: The current block in SearchRepositoryImpl uses
result.getOrNull() and returns a PaginatedDiscoveryRepositories even when
BackendSearchResponse.items is empty, which short-circuits fallbackGithubSearch;
update the method that handles result (the code using result.getOrNull() and
creating PaginatedDiscoveryRepositories) to treat an empty backend response as a
miss by checking if searchResponse.items.isEmpty() and, in that case, do not
return/cached PaginatedDiscoveryRepositories but instead call or fall through to
fallbackGithubSearch (or return null so the existing fallback path runs); ensure
you reference BackendSearchResponse.items, PaginatedDiscoveryRepositories, and
fallbackGithubSearch so empty backend results are not cached or returned.
- Around line 120-124: When SortBy.MostForks is selected in SearchRepositoryImpl
(the when on sortBy that sets the local variable sort), we currently set sort =
null which lets the backend choose an unintended default; change the logic so
that SortBy.MostForks maps to a backend-supported parameter (e.g., "forks") or,
if the current backend cannot accept "forks", detect SortBy.MostForks and route
the request to the GitHub REST search path that supports sort=forks. Update the
when block handling SortBy.MostForks (and any call-site that passes sort) to
pass the explicit "forks" string or trigger the REST fallback instead of leaving
sort as null.
---
Outside diff comments:
In
`@feature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/data_source/impl/CachedRepositoriesDataSourceImpl.kt`:
- Around line 44-52: The fallbackHttpClient created inside
CachedRepositoriesDataSourceImpl leaks because it's constructed directly and
never closed and also bypasses the project's
HttpClientFactory/createGitHubHttpClient conventions; fix by replacing the
direct HttpClient creation with an injected, pre-configured HttpClient from DI
(or call createGitHubHttpClient()/HttpClientFactory to build it) and ensure
lifecycle is managed—either implement AutoCloseable on
CachedRepositoriesDataSourceImpl and add a close() that disposes the client or
wire the client injection into Koin as a singleton so Koin manages its
lifecycle; apply the same refactor pattern to BackendApiClient to use the shared
factory/injection and proper close handling.
---
Nitpick comments:
In @.claude/settings.local.json:
- Line 14: Replace the overly broad allowlist entry "Bash(./gradlew build *)" in
settings.local.json with the specific gradle invocations your CI/dev workflow
actually needs (e.g., "Bash(./gradlew build)", "Bash(./gradlew build
--no-daemon)" or whatever concrete flags are required) to remove the wildcard;
also consider removing this per-developer .local file from version control and
add .claude/settings.local.json to .gitignore so personal permission grants are
not committed.
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendRepoResponse.kt`:
- Around line 27-30: The four parallel booleans in BackendRepoResponse
(hasInstallersAndroid, hasInstallersWindows, hasInstallersMacos,
hasInstallersLinux) should be made robust to wire-format differences: either
replace them with a nested data class (e.g., add data class
BackendInstallers(val android: Boolean = false, val windows: Boolean = false,
val macos: Boolean = false, val linux: Boolean = false) and change
BackendRepoResponse to expose val installers: BackendInstallers =
BackendInstallers()), or add `@SerialName` annotations to each existing property
to match the expected snake_case keys (e.g., annotate hasInstallersAndroid with
`@SerialName`("has_installers_android"), etc.); update any mapper code that reads
these fields to use the new installers property or annotated names accordingly.
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt`:
- Around line 34-74: Extract the repeated status/body/failure logic into a
private helper extension like a suspend inline fun <reified T>
HttpResponse.toResult(): Result<T> that checks response.status.isSuccess(),
returns Result.success(response.body()) or Result.failure(BackendException("HTTP
${response.status.value}")), then call this helper from getCategory, getTopic
and search (replace the inline status-check blocks after each httpClient.get).
Ensure the helper is used for all return types (List<BackendRepoResponse> and
BackendSearchResponse) so future endpoints won’t duplicate the
isSuccess/BackendException pattern.
In
`@feature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/data_source/impl/CachedRepositoriesDataSourceImpl.kt`:
- Around line 168-186: Replace the hardcoded platform list and the inline when
mapping in the coroutine block with an iteration over
DiscoveryPlatform.entries.filter { it != DiscoveryPlatform.All } and call
eachEntry.toApiSlug() when calling backendApiClient.getCategory; use the entry
itself (e.g., discoveryPlatform) to set availablePlatforms instead of mapping
strings, remove the else -> return@async null dead branch, and rely on the
existing DiscoveryPlatform.toApiSlug() helper so the mapping is centralized
(affects the async map that calls backendApiClient.getCategory and the
subsequent .toCachedGithubRepoSummary().copy(availablePlatforms = ...)).
In
`@feature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt`:
- Around line 150-158: The function receiver is fully qualified as
kotlinx.coroutines.channels.ProducerScope<PaginatedDiscoveryRepositories>;
replace that with a simple imported ProducerScope to improve readability: add an
import for kotlinx.coroutines.channels.ProducerScope at the top and change the
receiver type in fallbackGithubSearch to
ProducerScope<PaginatedDiscoveryRepositories> (function name:
fallbackGithubSearch, type: PaginatedDiscoveryRepositories) so the signature is
consistent with the rest of the codebase.
🪄 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: d387e947-d173-4c80-aca2-1b5c53b4f158
📒 Files selected for processing (11)
.claude/settings.local.jsoncore/data/src/commonMain/kotlin/zed/rainxch/core/data/di/SharedModule.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendRepoResponse.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendSearchResponse.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/mappers/BackendRepoMapper.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.ktfeature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/data_source/impl/CachedRepositoriesDataSourceImpl.ktfeature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/di/SharedModule.ktfeature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/mappers/BackendToCachedMapper.ktfeature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/di/SharedModule.ktfeature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt
| data class BackendSearchResponse( | ||
| val items: List<BackendRepoResponse>, | ||
| val totalHits: Int, | ||
| val processingTimeMs: Int, | ||
| val source: String? = null, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "BackendSearchResponse.kt" -type fRepository: OpenHub-Store/GitHub-Store
Length of output: 154
🏁 Script executed:
find . -name "BackendApiClient.kt" -type fRepository: OpenHub-Store/GitHub-Store
Length of output: 153
🏁 Script executed:
find . -path "*/core/data/*" -name "*.kt" -type f | grep -i client | head -20Repository: OpenHub-Store/GitHub-Store
Length of output: 506
🏁 Script executed:
cat -n ./core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendSearchResponse.ktRepository: OpenHub-Store/GitHub-Store
Length of output: 403
🏁 Script executed:
cat -n ./core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.ktRepository: OpenHub-Store/GitHub-Store
Length of output: 3746
🏁 Script executed:
find ./core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto -name "*.kt" -type fRepository: OpenHub-Store/GitHub-Store
Length of output: 1317
🏁 Script executed:
cat -n ./core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendRepoResponse.ktRepository: OpenHub-Store/GitHub-Store
Length of output: 1428
🏁 Script executed:
rg "@SerialName" ./core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/ -A 1Repository: OpenHub-Store/GitHub-Store
Length of output: 13839
🏁 Script executed:
# Search for documentation or comments about backend API field naming
rg "total_hits|totalHits|processing_time|processingTime" ./core/data -iRepository: OpenHub-Store/GitHub-Store
Length of output: 294
🏁 Script executed:
# Search for any documentation or API specs about the backend
find . -name "*.md" -o -name "*.txt" | xargs grep -l "github-store" -i 2>/dev/null | head -5Repository: OpenHub-Store/GitHub-Store
Length of output: 161
🏁 Script executed:
# Look for tests that might show expected API response format
find . -path "*/test*" -name "*Backend*" -o -path "*/test*" -name "*Search*" | head -10Repository: OpenHub-Store/GitHub-Store
Length of output: 52
🏁 Script executed:
# Check if there's any API documentation or API client setup
rg "api.github-store" -A 5 -B 5Repository: OpenHub-Store/GitHub-Store
Length of output: 1128
🏁 Script executed:
# Check if BackendRepoResponse is used with BackendSearchResponse
rg "BackendRepoResponse|BackendSearchResponse" ./core/data/src -lRepository: OpenHub-Store/GitHub-Store
Length of output: 406
🏁 Script executed:
# Look for any integration tests or mock data that might show expected format
find . -path "*/test*" -name "*.kt" | xargs grep -l "BackendSearch\|BackendRepo" 2>/dev/nullRepository: OpenHub-Store/GitHub-Store
Length of output: 54
🏁 Script executed:
# Check git history or comments for any API documentation
find . -name "*.kt" | xargs grep -i "total_hits\|processing_time" 2>/dev/nullRepository: OpenHub-Store/GitHub-Store
Length of output: 54
🏁 Script executed:
# Check BackendRepoMapper to see if there's any hint about field naming
cat -n ./core/data/src/commonMain/kotlin/zed/rainxch/core/data/mappers/BackendRepoMapper.ktRepository: OpenHub-Store/GitHub-Store
Length of output: 1755
🏁 Script executed:
# Look at BackendRepoResponse more carefully - does it also lack `@SerialName`?
cat -n ./core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendRepoResponse.kt | head -40Repository: OpenHub-Store/GitHub-Store
Length of output: 1428
🏁 Script executed:
# Search for any configuration that might handle snake_case conversion globally
rg "namingStrategy\|JsonNamingStrategy\|ignoreUnknownKeys" ./core/data -B 2 -A 2Repository: OpenHub-Store/GitHub-Store
Length of output: 52
Add @SerialName annotations to map snake_case backend API fields to camelCase properties.
BackendSearchResponse lacks @SerialName annotations on totalHits and processingTimeMs. The codebase consistently uses @SerialName for all other API DTOs that interact with snake_case backends (e.g., @SerialName("total_count") val totalCount). Without explicit field mapping and with no namingStrategy configured in BackendApiClient, deserialization will silently fail, leaving these fields as default values (0). Add:
`@SerialName`("total_hits") val totalHits: Int,
`@SerialName`("processing_time_ms") val processingTimeMs: Int,
Also apply the same fix to BackendRepoResponse, which has the same issue on multiple fields: stargazersCount, forksCount, defaultBranch, etc.
🤖 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/dto/BackendSearchResponse.kt`
around lines 6 - 11, BackendSearchResponse is missing `@SerialName` mappings for
snake_case API fields: add `@SerialName`("total_hits") to the totalHits property
and `@SerialName`("processing_time_ms") to processingTimeMs; likewise update
BackendRepoResponse to annotate its snake_case counterparts (e.g.,
`@SerialName`("stargazers_count") on stargazersCount, `@SerialName`("forks_count")
on forksCount, `@SerialName`("default_branch") on defaultBranch, etc.) so the
Kotlinx.serialization deserializer maps backend fields to the camelCase
properties in BackendSearchResponse and BackendRepoResponse.
| private val httpClient = HttpClient { | ||
| install(ContentNegotiation) { | ||
| json(Json { ignoreUnknownKeys = true }) | ||
| } | ||
| install(HttpTimeout) { | ||
| requestTimeoutMillis = 5_000 | ||
| connectTimeoutMillis = 3_000 | ||
| socketTimeoutMillis = 5_000 | ||
| } | ||
| defaultRequest { | ||
| url(BASE_URL) | ||
| } | ||
| expectSuccess = false | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use HttpClientFactory instead of instantiating HttpClient directly.
Per repo conventions, Ktor clients in core/data should be produced by HttpClientFactory so shared configuration (auth interceptors, logging, engine selection) stays centralized. Constructing a standalone HttpClient here bypasses that pipeline and duplicates plugin setup. Consider obtaining a preconfigured client from the factory and layering only backend-specific defaultRequest/timeout overrides on top (or injecting it via Koin).
Based on learnings: "Ktor HTTP client must be configured via HttpClientFactory in core/data with interceptors for authentication and request handling".
🤖 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/network/BackendApiClient.kt`
around lines 19 - 32, The HttpClient is being instantiated directly (private val
httpClient = HttpClient { ... }) which bypasses the repo-wide HttpClientFactory;
switch to obtaining a shared, preconfigured client from HttpClientFactory (or
inject it via Koin) instead of new HttpClient(), then apply only
backend-specific adjustments (e.g., set BASE_URL defaultRequest and any timeout
overrides) onto that client or via a lightweight wrapper; update references to
httpClient and preserve expectSuccess behavior while removing duplicate plugin
installs so auth/logging/engine selection remain centralized.
| private val httpClient = HttpClient { | ||
| install(ContentNegotiation) { | ||
| json(Json { ignoreUnknownKeys = true }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is JsonNamingStrategy.SnakeCase available in stable kotlinx.serialization and how is it enabled on the Json builder?
💡 Result:
No, JsonNamingStrategy.SnakeCase is not available in stable kotlinx.serialization. It is marked as @ExperimentalSerializationApi in the official documentation and source code. As of the latest stable release 1.11.0 (April 2026), it remains experimental, with no changelog entry indicating stabilization. While some Json configuration options were stabilized in 1.10.0, JsonNamingStrategy was not among them. To use it (with opt-in), import kotlinx.serialization.ExperimentalSerializationApi and configure the Json builder as follows: @OptIn(ExperimentalSerializationApi::class) val json = Json { namingStrategy = JsonNamingStrategy.SnakeCase } This transforms camelCase property names to snake_case during serialization and deserialization.
Citations:
- 1: https://kotlinlang.org/api/kotlinx.serialization/kotlinx-serialization-json/kotlinx.serialization.json/-json-naming-strategy/-builtins/-snake-case.html
- 2: https://kotlinlang.org/api/kotlinx.serialization/kotlinx-serialization-json/kotlinx.serialization.json/-json-naming-strategy/-builtins
- 3: https://kotlinlang.org/api/kotlinx.serialization/kotlinx-serialization-json/kotlinx.serialization.json/-json-builder/naming-strategy.html
- 4: https://github.com/Kotlin/kotlinx.serialization/blob/master/formats/json/commonMain/src/kotlinx/serialization/json/JsonNamingStrategy.kt
- 5: https://github.com/Kotlin/kotlinx.serialization/blob/master/CHANGELOG.md
- 6: https://github.com/Kotlin/kotlinx.serialization/releases/tag/v1.10.0
🏁 Script executed:
# First, find and examine the DTO class declarations
find . -type f -name "*.kt" | xargs grep -l "BackendRepoResponse\|BackendSearchResponse" | head -5Repository: OpenHub-Store/GitHub-Store
Length of output: 503
🏁 Script executed:
# Also check the BackendApiClient.kt file to understand the context better
cat -n core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt | head -50Repository: OpenHub-Store/GitHub-Store
Length of output: 2268
🏁 Script executed:
# Search for the DTO definitions
rg "class BackendRepoResponse|class BackendSearchResponse" --type kt -A 15Repository: OpenHub-Store/GitHub-Store
Length of output: 96
🏁 Script executed:
# Read the BackendRepoResponse DTO
cat -n ./core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendRepoResponse.ktRepository: OpenHub-Store/GitHub-Store
Length of output: 1428
🏁 Script executed:
# Read the BackendSearchResponse DTO
cat -n ./core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendSearchResponse.ktRepository: OpenHub-Store/GitHub-Store
Length of output: 403
🏁 Script executed:
# Check if `@SerialName` is already imported or used anywhere in the codebase
rg "@SerialName" --type-list | headRepository: OpenHub-Store/GitHub-Store
Length of output: 459
🏁 Script executed:
# Check for `@SerialName` usage in the codebase
rg "@SerialName" --type kotlinRepository: OpenHub-Store/GitHub-Store
Length of output: 16236
🏁 Script executed:
# Look for test files or mock data that might show backend payload structure
find . -type f -name "*Test*.kt" -o -name "*Mock*.kt" | head -10Repository: OpenHub-Store/GitHub-Store
Length of output: 235
🏁 Script executed:
# Check if there are any integration tests or responses that show actual backend format
rg "fullName|htmlUrl|stargazersCount" --type kotlin -B 3 -A 3 | head -50Repository: OpenHub-Store/GitHub-Store
Length of output: 6094
🏁 Script executed:
# Look for API documentation or comments about backend format
rg "GitHub|API|backend|payload" --type kotlin -B 2 -A 2 | head -50Repository: OpenHub-Store/GitHub-Store
Length of output: 6180
Add @SerialName annotations to field-name mismatches or update Json config with experimental naming strategy.
BackendRepoResponse/BackendSearchResponse declare camelCase properties (e.g., fullName, htmlUrl, stargazersCount, hasInstallersAndroid) without @SerialName annotations, and the Json config sets only ignoreUnknownKeys = true — no naming strategy. Pattern throughout the codebase shows GitHub API DTOs (GithubRepoNetworkModel, UserProfileNetwork, etc.) use @SerialName("snake_case") to map camelCase properties to snake_case backend fields. If this backend emits snake_case, these fields will silently fall back to defaults or fail for non-nullable fields, with ignoreUnknownKeys masking the mismatch.
Fix: Either annotate each property with @SerialName (recommended—aligns with existing codebase pattern):
`@Serializable`
data class BackendRepoResponse(
val id: Long,
val name: String,
`@SerialName`("full_name")
val fullName: String,
// ... rest of fields
)Or use JsonNamingStrategy.SnakeCase (requires @OptIn(ExperimentalSerializationApi::class) as it is unstable):
`@OptIn`(ExperimentalSerializationApi::class)
json(Json {
ignoreUnknownKeys = true
namingStrategy = JsonNamingStrategy.SnakeCase
})🤖 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/network/BackendApiClient.kt`
around lines 19 - 22, Backend response DTOs (BackendRepoResponse,
BackendSearchResponse) declare camelCase properties like fullName, htmlUrl,
stargazersCount, hasInstallersAndroid while the HttpClient Json config only sets
ignoreUnknownKeys = true, so snake_case fields from the backend will not map
correctly; fix by adding `@SerialName`("snake_case_name") annotations to each
mismatched property in those data classes (e.g., fullName -> "full_name",
htmlUrl -> "html_url", stargazersCount -> "stargazers_count",
hasInstallersAndroid -> "has_installers_android") following the existing pattern
used by GithubRepoNetworkModel/UserProfileNetwork, or alternatively opt into
ExperimentalSerializationApi and set namingStrategy =
JsonNamingStrategy.SnakeCase on the Json configuration in the HttpClient setup
to apply snake_case mapping globally.
| return result.getOrNull()?.let { searchResponse -> | ||
| val repos = searchResponse.items.map { it.toSummary() } | ||
|
|
||
| val hasMore = offset + repos.size < searchResponse.totalHits | ||
| PaginatedDiscoveryRepositories( | ||
| repos = repos, | ||
| hasMore = hasMore, | ||
| nextPageIndex = page + 1, | ||
| totalCount = searchResponse.totalHits, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Empty backend result short-circuits the GitHub REST fallback.
result.getOrNull() returns a non-null BackendSearchResponse even when items is empty. In that case this block produces a PaginatedDiscoveryRepositories with empty repos, which is then cached and returned — fallbackGithubSearch is never invoked. If the backend's index is incomplete/out-of-sync for a query, users will see 0 results even though GitHub would return matches.
Consider treating empty backend responses as "no result" and falling through to the fallback, e.g.:
🔧 Suggested fix
- return result.getOrNull()?.let { searchResponse ->
- val repos = searchResponse.items.map { it.toSummary() }
-
- val hasMore = offset + repos.size < searchResponse.totalHits
- PaginatedDiscoveryRepositories(
- repos = repos,
- hasMore = hasMore,
- nextPageIndex = page + 1,
- totalCount = searchResponse.totalHits,
- )
- }
+ val searchResponse = result.getOrNull() ?: return null
+ if (searchResponse.items.isEmpty() && page == 1) return null
+ val repos = searchResponse.items.map { it.toSummary() }
+ val hasMore = offset + repos.size < searchResponse.totalHits
+ return PaginatedDiscoveryRepositories(
+ repos = repos,
+ hasMore = hasMore,
+ nextPageIndex = page + 1,
+ totalCount = searchResponse.totalHits,
+ )📝 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.
| return result.getOrNull()?.let { searchResponse -> | |
| val repos = searchResponse.items.map { it.toSummary() } | |
| val hasMore = offset + repos.size < searchResponse.totalHits | |
| PaginatedDiscoveryRepositories( | |
| repos = repos, | |
| hasMore = hasMore, | |
| nextPageIndex = page + 1, | |
| totalCount = searchResponse.totalHits, | |
| ) | |
| } | |
| val searchResponse = result.getOrNull() ?: return null | |
| if (searchResponse.items.isEmpty() && page == 1) return null | |
| val repos = searchResponse.items.map { it.toSummary() } | |
| val hasMore = offset + repos.size < searchResponse.totalHits | |
| return PaginatedDiscoveryRepositories( | |
| repos = repos, | |
| hasMore = hasMore, | |
| nextPageIndex = page + 1, | |
| totalCount = searchResponse.totalHits, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt`
around lines 135 - 145, The current block in SearchRepositoryImpl uses
result.getOrNull() and returns a PaginatedDiscoveryRepositories even when
BackendSearchResponse.items is empty, which short-circuits fallbackGithubSearch;
update the method that handles result (the code using result.getOrNull() and
creating PaginatedDiscoveryRepositories) to treat an empty backend response as a
miss by checking if searchResponse.items.isEmpty() and, in that case, do not
return/cached PaginatedDiscoveryRepositories but instead call or fall through to
fallbackGithubSearch (or return null so the existing fallback path runs); ensure
you reference BackendSearchResponse.items, PaginatedDiscoveryRepositories, and
fallbackGithubSearch so empty backend results are not cached or returned.
- **Home Data**: Ensure backend results are only used in `CachedRepositoriesDataSourceImpl` if all platforms return successfully, ensuring consistency with fallback behavior. - **Search Data**: Implement a fallback to GitHub REST in `SearchRepositoryImpl` when sorting by "Most Forks," as the backend does not support it. - **Search Data**: Added a catch-all exception handler in the search flow to emit an empty result state instead of crashing. - **Tooling**: Added a comprehensive Gradle build command to `.claude/settings.local.json` covering both JVM and Android targets for core and feature modules.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
feature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt (1)
153-153: Minor: inline fully-qualifiedProducerScopein the declaration is unidiomatic.Using
kotlinx.coroutines.channels.ProducerScope<…>directly in the function signature works but is hard to read. Adding an import keeps the signature consistent with the rest of the file (which useschannelFlow).🧹 Suggested tidy-up
+import kotlinx.coroutines.channels.ProducerScope @@ - private suspend fun kotlinx.coroutines.channels.ProducerScope<PaginatedDiscoveryRepositories>.fallbackGithubSearch( + private suspend fun ProducerScope<PaginatedDiscoveryRepositories>.fallbackGithubSearch(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt` at line 153, The function signature for fallbackGithubSearch uses a fully-qualified type; import kotlinx.coroutines.channels.ProducerScope at the top and update the declaration to use ProducerScope<PaginatedDiscoveryRepositories> instead of kotlinx.coroutines.channels.ProducerScope<PaginatedDiscoveryRepositories> ( locate the private suspend fun fallbackGithubSearch(...) and the import list to make this change and ensure no naming conflicts ).feature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/data_source/impl/CachedRepositoriesDataSourceImpl.kt (1)
164-297: Optional: consolidate duplicated "all-platforms fan-out + merge" logic.
fetchAllPlatformsFromBackendandfetchAllPlatformsTopicFromBackendshare the same structure — hard-coded 4-platform list, fan-out,awaitAll().filterNotNull(), all-succeeded check, group-by-id + reduce-merge, sort. Same merge pattern is also duplicated infetchCategoryFromFallback/fetchTopicFromFallback. Extracting a small helper (e.g.,fanOutAcrossPlatformsproducingList<List<CachedGithubRepoSummary>>?and a separatemergeById(sortBy: ...)) would remove ~80 lines of copy-paste and keep the "all or nothing" invariant in a single place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/data_source/impl/CachedRepositoriesDataSourceImpl.kt` around lines 164 - 297, The two methods fetchAllPlatformsFromBackend and fetchAllPlatformsTopicFromBackend (and similar logic in fetchCategoryFromFallback / fetchTopicFromFallback) duplicate the 4-platform fan-out, awaitAll().filterNotNull(), all-succeeded check, groupBy id + reduce-merge and sorting; extract a helper (e.g., fanOutAcrossPlatforms(platforms = listOf("android","windows","macos","linux")) that takes a per-platform fetch lambda and returns List<List<CachedGithubRepoSummary>>? (null if any platform failed) and a mergeById(reposLists, mergeReducer, sortComparator) function that performs groupBy { it.id }, reduces duplicates using a provided reducer, and applies the provided sort; then replace the bodies of fetchAllPlatformsFromBackend and fetchAllPlatformsTopicFromBackend to call fanOutAcrossPlatforms and mergeById (passing existing mapping and copy logic and the appropriate sort comparator) to remove the duplicated code while preserving the "all or nothing" behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@feature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/data_source/impl/CachedRepositoriesDataSourceImpl.kt`:
- Around line 164-297: The two methods fetchAllPlatformsFromBackend and
fetchAllPlatformsTopicFromBackend (and similar logic in
fetchCategoryFromFallback / fetchTopicFromFallback) duplicate the 4-platform
fan-out, awaitAll().filterNotNull(), all-succeeded check, groupBy id +
reduce-merge and sorting; extract a helper (e.g.,
fanOutAcrossPlatforms(platforms = listOf("android","windows","macos","linux"))
that takes a per-platform fetch lambda and returns
List<List<CachedGithubRepoSummary>>? (null if any platform failed) and a
mergeById(reposLists, mergeReducer, sortComparator) function that performs
groupBy { it.id }, reduces duplicates using a provided reducer, and applies the
provided sort; then replace the bodies of fetchAllPlatformsFromBackend and
fetchAllPlatformsTopicFromBackend to call fanOutAcrossPlatforms and mergeById
(passing existing mapping and copy logic and the appropriate sort comparator) to
remove the duplicated code while preserving the "all or nothing" behavior.
In
`@feature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt`:
- Line 153: The function signature for fallbackGithubSearch uses a
fully-qualified type; import kotlinx.coroutines.channels.ProducerScope at the
top and update the declaration to use
ProducerScope<PaginatedDiscoveryRepositories> instead of
kotlinx.coroutines.channels.ProducerScope<PaginatedDiscoveryRepositories> (
locate the private suspend fun fallbackGithubSearch(...) and the import list to
make this change and ensure no naming conflicts ).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 044ae44f-df40-4347-b0b0-baa98847937b
📒 Files selected for processing (3)
.claude/settings.local.jsonfeature/home/data/src/commonMain/kotlin/zed/rainxch/home/data/data_source/impl/CachedRepositoriesDataSourceImpl.ktfeature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt
✅ Files skipped from review due to trivial changes (1)
- .claude/settings.local.json
1 participant
BackendApiClientto interface with the centralized GitHub Store API, featuring dedicated endpoints for categories, topics, and search.CachedRepositoriesDataSourceImplto prioritize backend fetching for home categories and topics, with a fallback to raw GitHub JSON files if the backend is unavailable.SearchRepositoryImpllogic to attempt backend searches first, providing faster results and native platform filtering, before falling back to manual GitHub REST API search and verification.BackendRepoResponse,BackendSearchResponse,BackendRepoMapper) to handle backend data structures and convert them to domain/cached models.BackendApiClientacross core, home, and search modules.CachedRepositoriesDataSourceImplto simplify platform and category slug handling via helper methods.Summary by CodeRabbit