Eliminate tab-switch freeze after semantic search - #221
Merged
Conversation
Each semantic search built a fresh LocalEmbeddingProvider with an empty model slot, re-reading the model weights (seconds of CPU) on every query. That reload was the CPU spike that starved concurrent UI work. Add a process-wide shared_local_provider cache keyed by model id + cache dir; the provider embeds through &self (interior Mutexes), so sharing one Arc across searches and threads is safe. SemanticRepo::provider() now returns the cached Arc, so the model loads once and stays resident. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Switching content tabs resolved the Bookmarks preview synchronously on the UI thread (a block_in_place live tree-sitter resolve), blocking the event loop. Right after a semantic search — when the model load and list-health fan-out saturate the CPU — switching to Collections and back froze the UI. Route the tab-change Bookmarks preview through a background resolve instead (request_bookmark_preview_now), showing a loading indicator immediately so the previous tab's overview doesn't linger. Init and focus-enter previews stay synchronous (the e2e snapshots depend on it); Collections/Tours overviews are DB-only and also stay synchronous. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![inspect-review[bot]](https://avatars.githubusercontent.com/in/3177512?s=60&v=4){kind=small}
There was a problem hiding this comment.
inspect review
Triage: 14 entities analyzed | 0 critical, 0 high, 10 medium, 4 low
Verdict: standard_review
Findings (0)
Reviewed by inspect | Entity-level triage found 0 high-risk changes
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe change caches local embedding providers by model and cache directory. Semantic repository access now reuses these providers. TUI content-tab changes start bookmark previews asynchronously and preserve synchronous collection and tour previews. ChangesEmbedding provider cache
Immediate preview routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The PR moves bookmark previews off the event loop and reuses the embedding model to prevent tab-switch freezes and repeated CPU spikes; no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant ContentTab
participant Browser
participant RightPane
participant BookmarkPreviewTask
ContentTab->>Browser: select bookmark tab
Browser->>RightPane: begin_bookmark_loading
Browser->>Browser: cancel queued work and invalidate stale results
Browser->>BookmarkPreviewTask: spawn preview with repository context
BookmarkPreviewTask-->>Browser: send PreviewReady with repo_root
Browser->>RightPane: update active repository and apply preview
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Contributor
Greptile SummaryThe PR moves bookmark resolution triggered by tab changes off the UI event loop and reuses local embedding providers process-wide.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| crates/codemark-tui/src/browser/mod.rs | Adds tab-specific asynchronous bookmark preview dispatch and invalidates pending work before synchronous Collection or Tour rendering. |
| crates/codemark-tui/src/browser/events.rs | Routes tab changes through the new preview path and preserves repository ownership in successful asynchronous preview events. |
| crates/codemark-tui/src/browser/right_pane.rs | Adds a bookmark-specific loading transition that clears overview state so loading feedback is immediately visible. |
| crates/codemark-tui/src/event.rs | Extends successful preview events with the owning repository root. |
| crates/codemark-core/src/embeddings/local.rs | Introduces a process-wide provider cache keyed by embedding model and cache directory. |
| crates/codemark-core/src/storage/semantic_repo.rs | Uses shared embedding providers so repeated semantic operations reuse loaded model state. |
Sequence Diagram
sequenceDiagram
participant User
participant UI as TUI event loop
participant Worker as Preview worker
participant Pane as Right pane
User->>UI: Switch to Bookmarks tab
UI->>Pane: Show loading state
UI->>Worker: Resolve selected bookmark with request ID
Note over UI: Event loop remains responsive
Worker-->>UI: PreviewReady(request ID, repo root, payload)
alt Request is still active
UI->>Pane: Apply preview
else Request was superseded by another tab
UI-->>Worker: Discard stale result
end
Reviews (3): Last reviewed commit: "chore(core): trace embedding provider ca..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/codemark-tui/src/browser/mod.rs (1)
689-712: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a debug trace for immediate preview dispatch.
Log the immediate preview request after allocating
active_preview_request. Use thecodemark::uitarget and the request ID.Proposed change
self.preview_seq = self.preview_seq.wrapping_add(1); self.active_preview_request = self.preview_seq; + tracing::debug!( + target: "codemark::ui", + request_id = self.active_preview_request, + "starting immediate bookmark preview" + ); self.pending_preview = None;🤖 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 `@crates/codemark-tui/src/browser/mod.rs` around lines 689 - 712, Add a debug trace in request_bookmark_preview_now immediately after assigning active_preview_request, using the codemark::ui target and including the allocated request ID.Source: Coding guidelines
🤖 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 `@crates/codemark-core/src/embeddings/local.rs`:
- Around line 289-303: Add tracing::debug! events in shared_local_provider for
both cached provider reuse and new provider creation, using target:
"codemark::embeddings" and including relevant context such as the model or cache
key. Document the new codemark::embeddings target in the subsystem table in
GEMINI.md.
In `@crates/codemark-tui/src/browser/mod.rs`:
- Around line 696-711: Ensure the selected repository root is preserved through
the async preview flow anchored by `spawn_preview_task`, `Event::PreviewReady`,
and `RightPane::apply_preview`; carry `repo_root` with the request/result or set
it for the current request before spawning, so successful previews from
non-focused repositories update `active_repo_root` correctly. Add a regression
test covering tab switches across multiple repositories and subsequent
right-pane refresh/action behavior.
---
Nitpick comments:
In `@crates/codemark-tui/src/browser/mod.rs`:
- Around line 689-712: Add a debug trace in request_bookmark_preview_now
immediately after assigning active_preview_request, using the codemark::ui
target and including the allocated request ID.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 27b99347-1eef-4169-8f80-5e25431a991d
📒 Files selected for processing (6)
crates/codemark-core/src/embeddings/local.rscrates/codemark-core/src/embeddings/mod.rscrates/codemark-core/src/storage/semantic_repo.rscrates/codemark-tui/src/browser/events.rscrates/codemark-tui/src/browser/mod.rscrates/codemark-tui/src/browser/right_pane.rs
… tabs A tab switch to Bookmarks now spawns a background preview and leaves its request id active. Switching on to Collections/Tours rendered their overview synchronously without invalidating that request, so a late PreviewReady could overwrite the overview with stale bookmark content from the prior tab. Cancel any in-flight/pending bookmark preview when switching to a synchronously-rendered overview tab (before the no-selection early return, so an empty target tab is covered too), dropping the stale result on arrival. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
inspect review
Triage: 14 entities analyzed | 0 critical, 0 high, 10 medium, 4 low
Verdict: standard_review
Findings (0)
Reviewed by inspect | Entity-level triage found 0 high-risk changes
Owner
Author
|
@greptile review |
DanielCardonaRojas
changed the title
The synchronous load_bookmark_live path recorded the previewed bookmark's owning repo in active_repo_root, but the background PreviewReady path did not (apply_preview has no db). Under multi-select, a preview from a non-focused repo left active_repo_root stale, so a later right-pane refresh/action could resolve against the wrong database. Routing tab switches through the async path widened this to tab changes too. Carry repo_root through Event::PreviewReady and set active_repo_root in the handler, mirroring the synchronous path. Fixes the pre-existing nav-path gap as well. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Instrument shared_local_provider with tracing::debug! events for cache hit (reuse) and miss (creation) under a new codemark::embeddings subsystem target, per the project logging convention; document the target in GEMINI.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
inspect review
Triage: 18 entities analyzed | 0 critical, 0 high, 13 medium, 5 low
Verdict: standard_review
Findings (0)
Reviewed by inspect | Entity-level triage found 0 high-risk changes
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.
Overview
Switching content tabs after a semantic search (Bookmarks → Collections → Bookmarks) froze the UI. The tab-change handler resolved the Bookmarks preview synchronously on the event loop, and that live tree-sitter resolve stalled while a concurrent semantic search saturated the CPU by re-loading its embedding model on every query.
Key changes
request_bookmark_preview_now), showing a loading indicator immediately so the event loop never blocks; init/focus-enter and Collections/Tours previews stay synchronous.shared_local_providercache so the embedding model loads once per process instead of reloading on every search, removing the CPU spike that starved the UI.Test plan
cargo test -p codemark-tui(14 e2e + doctests) andcargo test -p codemark-core --features semanticpass.codemark-coreandcodemark-tuiwith and without thesemanticfeature; clippy clean on changed code.🤖 Generated with Claude Code
Summary by CodeRabbit
Performance
Bug Fixes