Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ Custom Gradle plugins in `build-logic/convention/` standardize module setup:
- **Shizuku (Android):** Optional silent install via `ShizukuProvider` (registered in AndroidManifest). Requires Shizuku app running with ADB or root. AIDL service passes APK via `ParcelFileDescriptor` to `pm install -S`. Falls back to standard installer on failure.
- **Gradle properties:** Config cache enabled, build cache enabled, 4GB Gradle heap, 3GB Kotlin daemon heap
- **Code style:** Official Kotlin style (`kotlin.code.style=official`)
- **Desktop logs:** `CrashReporter` (installed as the first line of `DesktopApp.main`) tees `System.out`/`System.err` to a rotating `session.log` and writes `crash-<timestamp>.log` on uncaught exceptions. Paths: `~/Library/Logs/GitHub-Store/` (macOS), `%LOCALAPPDATA%/GitHub-Store/logs/` (Windows), `$XDG_STATE_HOME/GitHub-Store/logs/` (Linux). Android uses Logcat — no CrashReporter.
- **`X-GitHub-Token` header:** Forwarded to the backend *only* on `/v1/search` and `/v1/search/explore` (so the backend's live GitHub passthrough runs under the user's own 5000/hr quota). Sourced from `TokenStore.currentToken()` via `BackendApiClient.currentUserGithubToken()` — the helper is `private` so other endpoints can't leak it accidentally. Never sent on other endpoints (`/v1/categories`, `/v1/topics`, `/v1/repo`, `/v1/events`), never logged (no Ktor `Logging` plugin installed).

## Coding Conventions

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ data class BackendSearchResponse(
val totalHits: Int,
val processingTimeMs: Int,
val source: String? = null,
val passthroughAttempted: Boolean? = null,
)
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,29 @@ object KermitLogger : GitHubStoreLogger {
) {
Logger.e(message, throwable)
}

override fun withTag(tag: String): GitHubStoreLogger = TaggedKermitLogger(Logger.withTag(tag))
}

private class TaggedKermitLogger(private val delegate: Logger) : GitHubStoreLogger {
override fun debug(message: String) {
delegate.d(message)
}

override fun info(message: String) {
delegate.i(message)
}

override fun warn(message: String) {
delegate.w(message)
}

override fun error(
message: String,
throwable: Throwable?,
) {
delegate.e(message, throwable)
}

override fun withTag(tag: String): GitHubStoreLogger = TaggedKermitLogger(delegate.withTag(tag))
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,12 @@ class BackendApiClient(
parameter("q", query)
if (platform != null) parameter("platform", platform)
parameter("page", page)
timeout { requestTimeoutMillis = 20_000 }

timeout {
requestTimeoutMillis = 30_000
socketTimeoutMillis = 30_000
}

Comment on lines +143 to +148
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

Consider user-perceived delay for the 30s timeout.

The per-request timeout raises both requestTimeoutMillis and socketTimeoutMillis to 30s, which is 6× the client default of 5s. Since searchExplore is only invoked from the user-triggered exploreFromGithubperformExplore() flow (with getOrThrow() and no retry), a slow/unreachable backend can keep the UI in ExploreStatus.LOADING for up to 30 seconds with no way for the user to cancel from the UI. If the backend is expected to take this long, that's fine; otherwise consider tightening to ~10–15s and/or providing an explicit cancel affordance on the explore button.

🤖 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 143 - 148, The per-request timeouts in the timeout block
(requestTimeoutMillis and socketTimeoutMillis) are set to 30_000ms, which can
leave the UI stuck in ExploreStatus.LOADING when searchExplore (called from
exploreFromGithub → performExplore() and consumed with getOrThrow()) waits the
full 30s; reduce these timeouts to a more user-friendly value (e.g.,
10_000–15_000ms) or implement a cancellable request path (expose a
CancellationToken/Job from performExplore and wire a cancel affordance on the
explore button) so users can abort slow calls; update the timeout values in the
timeout { ... } block or add cancellation support around
searchExplore/performExplore accordingly.

if (token != null) header(X_GITHUB_TOKEN_HEADER, token)
}
if (response.status.isSuccess()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,6 @@ interface GitHubStoreLogger {
message: String,
throwable: Throwable? = null,
)

fun withTag(tag: String): GitHubStoreLogger = this
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ data class PaginatedDiscoveryRepositories(
val hasMore: Boolean,
val nextPageIndex: Int,
val totalCount: Int? = null,
val passthroughAttempted: Boolean? = null,
)
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ class SearchRepositoryImpl(
hasMore = hasMore,
nextPageIndex = page + 1,
totalCount = searchResponse.totalHits,
passthroughAttempted = searchResponse.passthroughAttempted,
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,35 @@ fun SearchScreen(
}
}

if (!state.isLoading &&
!state.isLoadingMore &&
state.errorMessage == null &&
state.repositories.isEmpty() &&
state.query.isNotBlank() &&
!state.hasMorePages
) {
Box(
modifier = Modifier.fillMaxSize(),
contentAlignment = Alignment.Center,
) {
Column(horizontalAlignment = Alignment.CenterHorizontally) {
Text(text = stringResource(Res.string.no_repositories_found))

// Backend already did its own passthrough and still found
// nothing — don't tease a manual explore that would just
// redo the same work. Any other case (false / null for
// older backends) keeps the CTA.
if (state.passthroughAttempted != true) {
Spacer(Modifier.height(8.dp))
ExploreFromGithubButton(
status = state.exploreStatus,
onExplore = { onAction(SearchAction.ExploreFromGithub) },
)
}
}
}
}

if (state.visibleRepos.isNotEmpty()) {
val isScrollbarEnabled = LocalScrollbarEnabled.current
ScrollbarContainer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ data class SearchState(
val autoDetectClipboardEnabled: Boolean = true,
val recentSearches: ImmutableList<String> = persistentListOf(),
val exploreStatus: ExploreStatus = ExploreStatus.IDLE,
val passthroughAttempted: Boolean? = null,
) {
enum class ExploreStatus {
IDLE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import zed.rainxch.githubstore.core.presentation.res.failed_to_share_link
import zed.rainxch.githubstore.core.presentation.res.link_copied_to_clipboard
import zed.rainxch.githubstore.core.presentation.res.no_github_link_in_clipboard
import zed.rainxch.githubstore.core.presentation.res.explore_error
import zed.rainxch.githubstore.core.presentation.res.no_repositories_found
import zed.rainxch.githubstore.core.presentation.res.search_failed
import zed.rainxch.search.presentation.mappers.toDomain
import zed.rainxch.search.presentation.utils.isEntirelyGithubUrls
Expand All @@ -66,6 +65,8 @@ class SearchViewModel(
private var explorePage = 1
private var lastExploreQuery = ""

private val exploreLog = logger.withTag("SearchExplore")

companion object {
private const val MIN_QUERY_LENGTH = 3
}
Expand Down Expand Up @@ -295,7 +296,12 @@ class SearchViewModel(
currentPage = 1
explorePage = 1
lastExploreQuery = query
_state.update { it.copy(exploreStatus = SearchState.ExploreStatus.IDLE) }
_state.update {
it.copy(
exploreStatus = SearchState.ExploreStatus.IDLE,
passthroughAttempted = null,
)
}
}

currentSearchJob =
Expand All @@ -312,6 +318,8 @@ class SearchViewModel(
it.repositories
},
totalCount = if (isInitial) null else it.totalCount,
passthroughAttempted =
if (isInitial) null else it.passthroughAttempted,
)
}

Expand Down Expand Up @@ -390,12 +398,8 @@ class SearchViewModel(
repositories = allRepos,
hasMorePages = paginatedRepos.hasMore,
totalCount = allRepos.size,
errorMessage =
if (allRepos.isEmpty() && !paginatedRepos.hasMore) {
getString(Res.string.no_repositories_found)
} else {
null
},
errorMessage = null,
passthroughAttempted = paginatedRepos.passthroughAttempted,
)
}
}
Expand Down Expand Up @@ -682,10 +686,27 @@ class SearchViewModel(

private fun performExplore() {
val query = _state.value.query.trim()
if (query.isBlank() || _state.value.exploreStatus == SearchState.ExploreStatus.LOADING) return
val platformUi = _state.value.selectedSearchPlatform
val prevStatus = _state.value.exploreStatus

exploreLog.debug(
"click: query='$query' platform=$platformUi " +
"page=$explorePage lastQuery='$lastExploreQuery' status=$prevStatus",
)

if (query.isBlank()) {
exploreLog.debug("skipped: query is blank")
return
}
if (prevStatus == SearchState.ExploreStatus.LOADING) {
exploreLog.debug("skipped: already LOADING")
return
}

// Reset page if query changed
if (query != lastExploreQuery) {
exploreLog.debug(
"query changed ('$lastExploreQuery' -> '$query'); resetting page to 1",
)
explorePage = 1
lastExploreQuery = query
}
Expand All @@ -696,24 +717,40 @@ class SearchViewModel(
try {
val exploreResult = searchRepository.exploreFromGithub(
query = query,
platform = _state.value.selectedSearchPlatform.toDomain(),
platform = platformUi.toDomain(),
page = explorePage,
)
val existingCount = _state.value.repositories.size
exploreLog.debug(
"response: items=${exploreResult.repos.size} " +
"returnedPage=${exploreResult.page} hasMore=${exploreResult.hasMore} " +
"existingVisible=$existingCount",
)

if (exploreResult.repos.isEmpty() || !exploreResult.hasMore) {
if (exploreResult.repos.isNotEmpty()) {
appendExploreResults(exploreResult.repos)
}
_state.update { it.copy(exploreStatus = SearchState.ExploreStatus.EXHAUSTED) }
} else {
val before = _state.value.repositories.size
if (exploreResult.repos.isNotEmpty()) {
appendExploreResults(exploreResult.repos)
}
val added = _state.value.repositories.size - before
val dupes = exploreResult.repos.size - added

if (exploreResult.hasMore) {
explorePage++
exploreLog.debug(
"-> IDLE: appended=$added dupes=$dupes nextPage=$explorePage",
)
_state.update { it.copy(exploreStatus = SearchState.ExploreStatus.IDLE) }
} else {
exploreLog.debug(
"-> EXHAUSTED: appended=$added dupes=$dupes " +
"rawItems=${exploreResult.repos.size}",
)
_state.update { it.copy(exploreStatus = SearchState.ExploreStatus.EXHAUSTED) }
}
Comment on lines +730 to 749
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

Edge case: empty repos with hasMore = true silently drops the attempt.

If the backend returns exploreResult.repos.isEmpty() but hasMore = true, this block: (a) skips appendExploreResults, (b) increments explorePage, and (c) sets status back to IDLE. The user sees no visible change and no "no more results" terminal state, which can look like the button did nothing. A subsequent click could repeat indefinitely if the backend keeps returning empty pages with hasMore = true.

Consider either capping consecutive empty-but-has-more responses (similar to MAX_AUTO_SKIP_PAGES in SearchRepositoryImpl.fallbackGithubSearch) or surfacing a small "no new results on this page" message so the interaction doesn't feel dead. Not blocking, just brittle.

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

In
`@feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt`
around lines 724 - 743, The current loop silently advances explorePage when
exploreResult.repos.isEmpty() && exploreResult.hasMore, which can lead to
repeated no-op clicks; fix by tracking consecutive empty-but-has-more pages
(e.g., add a private exploreEmptyStreak counter in SearchViewModel) and reuse or
mirror the existing MAX_AUTO_SKIP_PAGES behavior from
SearchRepositoryImpl.fallbackGithubSearch: increment exploreEmptyStreak when
added == 0, reset it when added > 0, and if exploreEmptyStreak >=
MAX_AUTO_SKIP_PAGES set _state.exploreStatus to EXHAUSTED (or set a new explicit
"NO_NEW_RESULTS" status flag) + log the event instead of remaining IDLE; update
references in the block around appendExploreResults, explorePage, exploreLog,
and _state.update accordingly and ensure the counter is persisted/cleared on new
searches.

} catch (e: CancellationException) {
throw e
} catch (e: Exception) {
logger.error("Explore failed: ${e.message}")
exploreLog.error("failed: ${e::class.simpleName}: ${e.message}", e)
_state.update { it.copy(exploreStatus = SearchState.ExploreStatus.IDLE) }
_events.send(SearchEvent.OnMessage(getString(Res.string.explore_error)))
}
Expand Down