Skip to content

search: update exploration logic and UI empty states#438

Merged
rainxchzed merged 7 commits intomainfrom
search-padding-fix
Apr 20, 2026
Merged

search: update exploration logic and UI empty states#438
rainxchzed merged 7 commits intomainfrom
search-padding-fix

Conversation

@rainxchzed
Copy link
Copy Markdown
Member

@rainxchzed rainxchzed commented Apr 20, 2026

Summary by CodeRabbit

  • New Features

    • Added empty-state UI displaying "no repositories found" when search returns no results.
    • Added conditional "Explore from GitHub" button that appears in the empty state under specific conditions.
  • Improvements

    • Enhanced request timeout configuration for search explore operations.
    • Improved logging for search explore functionality with structured tagging support.

- Add `passthroughAttempted` to `SearchState` to track if the backend already performed a GitHub search.
- Integrate detailed Kermit logging for the "Explore from Github" flow.
- Refactor `SearchViewModel` to delegate empty state messaging to the UI and improve pagination logic during exploration.
- Update `SearchRoot` to display a centralized "no repositories found" view.
- Prevent redundant manual exploration if a passthrough search was already attempted by the backend.
…uests

- Update `requestTimeoutMillis` from 20,000 to 30,000ms.
- Explicitly set `socketTimeoutMillis` to 30,000ms.
- Update `BackendSearchResponse` DTO to include an optional `passthroughAttempted` boolean.
- Update `PaginatedDiscoveryRepositories` domain model to include `passthroughAttempted`.
- Map the new field from the data layer to the domain layer in `SearchRepositoryImpl`.
…g logic

- Add documentation for the `CrashReporter` on desktop, including log rotation, crash file generation, and platform-specific log paths.
- Detail the security constraints for the `X-GitHub-Token` header, specifying that it is only forwarded to specific search endpoints to utilize user-side rate limits and is never logged.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

Walkthrough

The PR enhances the search feature by adding passthrough attempt tracking throughout the data layer, domain model, and presentation tier. It also introduces tagged logging support to the logging infrastructure and adjusts timeout configuration in the search explore endpoint.

Changes

Cohort / File(s) Summary
Documentation
CLAUDE.md
Updated documentation describing backend crash/log handling via CrashReporter for desktop and X-GitHub-Token header restriction to /v1/search and /v1/search/explore endpoints.
Backend Response & Timeout Configuration
core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendSearchResponse.kt, core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt
Added optional passthroughAttempted field to BackendSearchResponse DTO; adjusted searchExplore timeout to set both requestTimeoutMillis and socketTimeoutMillis to 30_000.
Domain Model
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/PaginatedDiscoveryRepositories.kt
Added optional passthroughAttempted field to track passthrough attempt state in domain model.
Data Layer Propagation
feature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt
Updated repository to propagate passthroughAttempted from backend response into constructed domain models.
Logging Infrastructure
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/logging/GitHubStoreLogger.kt, core/data/src/commonMain/kotlin/zed/rainxch/core/data/logging/KermitLogger.kt
Added withTag(tag: String) method to logger interface and implemented tagged logging via TaggedKermitLogger delegation class.
Presentation State
feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchState.kt
Added optional passthroughAttempted property to SearchState to carry passthrough tracking information.
ViewModel & UI Rendering
feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt, feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchRoot.kt
Enhanced SearchViewModel with structured logging (new exploreLog tagged logger), early-return guards, detailed explore attempt tracking, and pagination logic adjustments; added empty-state UI in SearchRoot conditionally displaying ExploreFromGithubButton only when passthroughAttempted != true.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 A passthrough tracked through every layer,
From backend DTO to the UI's prayer,
With tagged logs singing in structured song,
Each timeout tuned and logic strong,
The search now knows where it's been!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes in the pull request, which involve updating the search exploration logic and adding UI empty states for no results.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch search-padding-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt (1)

295-318: ⚠️ Potential issue | 🟡 Minor

Reset passthroughAttempted when starting a new initial search.

On isInitial, the state update clears repositories, errorMessage, and totalCount, but passthroughAttempted is left at its previous value until the new result is emitted in collect (line 396). If the new search errors out, is cancelled, or simply takes a while, the UI state still carries the prior search's passthroughAttempted = true, which gates the Explore CTA in SearchRoot.kt. Today the empty-state branch is additionally gated by !isLoading and errorMessage == null, so the visible impact is limited, but it's easy to regress if any other UI branch starts reading this flag.

Proposed fix
                 _state.update {
                     it.copy(
                         isLoading = isInitial,
                         isLoadingMore = !isInitial,
                         errorMessage = null,
                         repositories =
                             if (isInitial) {
                                 persistentListOf()
                             } else {
                                 it.repositories
                             },
                         totalCount = if (isInitial) null else it.totalCount,
+                        passthroughAttempted = if (isInitial) null else it.passthroughAttempted,
                     )
                 }

Also applies to: 391-398

🤖 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 295 - 318, When starting a new initial search in SearchViewModel
(inside the isInitial branch before launching currentSearchJob and inside the
_state.update call), reset the passthroughAttempted flag to false so the UI
doesn't inherit the previous search's value; specifically update the state
mutation (the _state.update block that sets
isLoading/isLoadingMore/errorMessage/repositories/totalCount) to also set
passthroughAttempted = false whenever isInitial is true (and mirror this same
reset where you clear state around lines 391-398).
🧹 Nitpick comments (1)
CLAUDE.md (1)

162-162: Optional wording cleanup for brevity.

In Line 162, “by accident” can be shortened to “accidentally” for tighter phrasing.

✏️ Suggested doc edit
-- **`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 by accident. Never sent on other endpoints (`/v1/categories`, `/v1/topics`, `/v1/repo`, `/v1/events`), never logged (no Ktor `Logging` plugin installed).
+- **`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).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` at line 162, Edit the paragraph that begins with
"**`X-GitHub-Token` header:**" and replace the phrase "by accident" with
"accidentally" so the sentence reads tighter (e.g., "...the helper is `private`
so other endpoints can't leak it accidentally."). Keep the rest of the sentence
and punctuation unchanged and ensure the surrounding phrasing remains
consistent.
🤖 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/network/BackendApiClient.kt`:
- Around line 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.

In
`@feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt`:
- Around line 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.
- Line 5: SearchViewModel currently imports co.touchlab.kermit.Logger as
KermitLogger and calls KermitLogger.withTag(...) for SearchExplore logs which
bypasses the injected GitHubStoreLogger; remove the direct Kermit import and
replace usages of KermitLogger.withTag(...) to route through the injected
logger: GitHubStoreLogger (e.g., add an optional tag parameter to
GitHubStoreLogger like fun debug(message: String, tag: String? = null) and use
logger.debug(..., tag) or simply call logger.debug(message) if you prefer no
tag), updating SearchViewModel's logging calls to use logger instead of
KermitLogger so all logging uses the GitHubStoreLogger abstraction.

---

Outside diff comments:
In
`@feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt`:
- Around line 295-318: When starting a new initial search in SearchViewModel
(inside the isInitial branch before launching currentSearchJob and inside the
_state.update call), reset the passthroughAttempted flag to false so the UI
doesn't inherit the previous search's value; specifically update the state
mutation (the _state.update block that sets
isLoading/isLoadingMore/errorMessage/repositories/totalCount) to also set
passthroughAttempted = false whenever isInitial is true (and mirror this same
reset where you clear state around lines 391-398).

---

Nitpick comments:
In `@CLAUDE.md`:
- Line 162: Edit the paragraph that begins with "**`X-GitHub-Token` header:**"
and replace the phrase "by accident" with "accidentally" so the sentence reads
tighter (e.g., "...the helper is `private` so other endpoints can't leak it
accidentally."). Keep the rest of the sentence and punctuation unchanged and
ensure the surrounding phrasing remains consistent.
🪄 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: e54fd0c3-69fb-476b-8f8d-60c9d90e883b

📥 Commits

Reviewing files that changed from the base of the PR and between ff5c558 and 62e0728.

📒 Files selected for processing (9)
  • CLAUDE.md
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/BackendSearchResponse.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/PaginatedDiscoveryRepositories.kt
  • feature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt
  • feature/search/presentation/build.gradle.kts
  • feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchRoot.kt
  • feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchState.kt
  • feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt

Comment on lines +143 to +148

timeout {
requestTimeoutMillis = 30_000
socketTimeoutMillis = 30_000
}

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.

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

- Add `withTag` support to `GitHubStoreLogger` and provide a Kermit-based implementation in `TaggedKermitLogger`.
- Remove direct Kermit dependency from `feature:search:presentation` in favor of the core logging abstraction.
- Update `SearchViewModel` to use the tagged logger for exploration tracing.
- Update search state logic to track and reset `passthroughAttempted` status during query changes and initial loads.
- Clean up documentation and unused imports.
@rainxchzed rainxchzed merged commit b9615ce into main Apr 20, 2026
1 check was pending
@rainxchzed rainxchzed deleted the search-padding-fix branch April 20, 2026 18:25
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt (1)

687-758: ⚠️ Potential issue | 🟠 Major

Explore coroutine isn’t tracked — stale results/status can leak into a new search.

performExplore() launches into viewModelScope without assigning the Job anywhere, while performSearch(isInitial = true) only cancels currentSearchJob and resets exploreStatus = IDLE / passthroughAttempted = null (lines 299–304). Concrete race:

  1. User clicks Explore → explore job runs, exploreStatus = LOADING.
  2. User changes query / filter / sort / hits IME search → performSearch(isInitial = true) resets explore state and starts a new search; the in-flight explore is not cancelled.
  3. The explore job resolves against the old query and calls appendExploreResults(...) (line 732), appending stale repos into the new search’s repositories list, and then clobbers exploreStatus back to IDLE/EXHAUSTED (lines 742/748) after the new search has already settled it.

Suggested fix: track the explore job the same way as currentSearchJob, cancel it on new-query resets and on OnClearClick, and bail out on CancellationException before touching state.

🛠 Sketch
     private var currentSearchJob: Job? = null
+    private var currentExploreJob: Job? = null
     private var currentPage = 1
     private var explorePage = 1
         if (isInitial) {
             currentSearchJob?.cancel()
+            currentExploreJob?.cancel()
             currentPage = 1
             explorePage = 1
             lastExploreQuery = query
             _state.update {
                 it.copy(
                     exploreStatus = SearchState.ExploreStatus.IDLE,
                     passthroughAttempted = null,
                 )
             }
         }
-        viewModelScope.launch {
+        currentExploreJob = viewModelScope.launch {
             _state.update { it.copy(exploreStatus = SearchState.ExploreStatus.LOADING) }
             try {
                 ...
             } catch (e: CancellationException) {
                 throw e
             } catch (e: Exception) {
                 ...
             }
         }

Also consider cancelling it from OnClearClick and OnSearchChange (blank/URL-only) paths that already cancel currentSearchJob, so explore state doesn’t reappear after the results are cleared.

🤖 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 687 - 758, performExplore launches an untracked coroutine
(viewModelScope.launch) so its in-flight Job can append stale results and
clobber exploreStatus after a new search; create and store a Job property (e.g.,
exploreJob: Job?) and assign it to the coroutine returned by
viewModelScope.launch in performExplore, cancel exploreJob wherever
currentSearchJob is cancelled (the performSearch(isInitial = true) reset path)
and also cancel it in OnClearClick/OnSearchChange handlers, and ensure the
explore coroutine bails on CancellationException before mutating state (rethrow
or return immediately) and only updates state/ calls appendExploreResults when
not cancelled. Use the existing symbols performExplore, exploreJob,
currentSearchJob, appendExploreResults, OnClearClick, and OnSearchChange to
locate and apply the changes.
🧹 Nitpick comments (2)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/logging/GitHubStoreLogger.kt (1)

14-15: Default withTag returning this is a silent footgun for alternate implementations.

Any GitHubStoreLogger implementation that doesn’t explicitly override withTag (e.g., test doubles, no-op loggers, future platform impls) will silently drop the tag while callers assume tagging is in effect. Since KermitLogger is currently the only production impl this is low risk, but consider making it abstract (no default) so the contract is enforced, or documenting on the interface that the default is a best-effort no-op so implementors know to override it when they have a backing tagging mechanism.

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

In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/logging/GitHubStoreLogger.kt`
around lines 14 - 15, The default implementation of GitHubStoreLogger.withTag
returning this can silently drop tags for implementations that forget to
override it; change the interface so withTag is abstract (remove the default
implementation) to force implementors to provide tagging behavior, then update
the concrete implementation(s) such as KermitLogger (and any test doubles/no-op
loggers) to implement withTag appropriately (either applying the tag to the
underlying logger or explicitly documenting/returning a best-effort no-op) so
callers cannot assume tagging is applied when it is not.
feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt (1)

294-324: Minor: redundant passthroughAttempted reset on initial search.

When isInitial is true, passthroughAttempted is set to null twice — once at lines 299–304 and again at lines 321–322 — and on non-initial the explicit passthroughAttempted = it.passthroughAttempted is a no-op. Not incorrect, just noise. Consider dropping the second branch or collapsing both resets into the outer isInitial block.

♻️ Suggested simplification
                 _state.update {
                     it.copy(
                         isLoading = isInitial,
                         isLoadingMore = !isInitial,
                         errorMessage = null,
                         repositories =
                             if (isInitial) {
                                 persistentListOf()
                             } else {
                                 it.repositories
                             },
                         totalCount = if (isInitial) null else it.totalCount,
-                        passthroughAttempted =
-                            if (isInitial) null else it.passthroughAttempted,
                     )
                 }
🤖 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 294 - 324, The passthroughAttempted field is redundantly reset to
null for initial searches: it's already set inside the outer isInitial block and
then again in the _state.update within the launched coroutine; remove the
redundant assignment by eliminating passthroughAttempted from the inner
_state.update branch (the one inside viewModelScope.launch) or make that inner
assignment conditional only for non-initial paths so that passthroughAttempted
is only set once when isInitial is true; update references to
passthroughAttempted in the _state.update call in SearchViewModel (within
viewModelScope.launch) to avoid the duplicate reset while preserving the
existing behavior for non-initial searches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In
`@feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt`:
- Around line 687-758: performExplore launches an untracked coroutine
(viewModelScope.launch) so its in-flight Job can append stale results and
clobber exploreStatus after a new search; create and store a Job property (e.g.,
exploreJob: Job?) and assign it to the coroutine returned by
viewModelScope.launch in performExplore, cancel exploreJob wherever
currentSearchJob is cancelled (the performSearch(isInitial = true) reset path)
and also cancel it in OnClearClick/OnSearchChange handlers, and ensure the
explore coroutine bails on CancellationException before mutating state (rethrow
or return immediately) and only updates state/ calls appendExploreResults when
not cancelled. Use the existing symbols performExplore, exploreJob,
currentSearchJob, appendExploreResults, OnClearClick, and OnSearchChange to
locate and apply the changes.

---

Nitpick comments:
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/logging/GitHubStoreLogger.kt`:
- Around line 14-15: The default implementation of GitHubStoreLogger.withTag
returning this can silently drop tags for implementations that forget to
override it; change the interface so withTag is abstract (remove the default
implementation) to force implementors to provide tagging behavior, then update
the concrete implementation(s) such as KermitLogger (and any test doubles/no-op
loggers) to implement withTag appropriately (either applying the tag to the
underlying logger or explicitly documenting/returning a best-effort no-op) so
callers cannot assume tagging is applied when it is not.

In
`@feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt`:
- Around line 294-324: The passthroughAttempted field is redundantly reset to
null for initial searches: it's already set inside the outer isInitial block and
then again in the _state.update within the launched coroutine; remove the
redundant assignment by eliminating passthroughAttempted from the inner
_state.update branch (the one inside viewModelScope.launch) or make that inner
assignment conditional only for non-initial paths so that passthroughAttempted
is only set once when isInitial is true; update references to
passthroughAttempted in the _state.update call in SearchViewModel (within
viewModelScope.launch) to avoid the duplicate reset while preserving the
existing behavior for non-initial searches.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4124e307-cbc0-4a06-bab8-ccb59b4811a1

📥 Commits

Reviewing files that changed from the base of the PR and between 62e0728 and 78c41f6.

📒 Files selected for processing (4)
  • CLAUDE.md
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/logging/KermitLogger.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/logging/GitHubStoreLogger.kt
  • feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • CLAUDE.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant