fix: async init + DB lock prevention for BrainBar launch#160
Conversation
Root cause: BrainDatabase init blocked indefinitely on SQLite write lock held by the watch agent. PRAGMA journal_mode=WAL requires a write lock even when WAL is already set. This prevented the status item from ever appearing. Fixes: - Status item created IMMEDIATELY in applicationDidFinishLaunching (no DB) - All DB/server/collector init moved to background DispatchQueue - Skip PRAGMA journal_mode=WAL if already WAL (avoids write lock) - Skip ensureSchema if chunks table exists (avoids RESERVED lock) - busy_timeout increased to 30s for enrichment batch lock contention - Loading popover shown on click before DB is ready - brainbar:// URL scheme added to bundle Info.plist (persists across rebuilds) Boot time: infinite → 239ms Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe changes implement early UI initialization with deferred backend setup, add database connection lifecycle logging with conditional schema creation, and register a custom URL scheme for the application. Changes
Sequence Diagram(s)sequenceDiagram
participant App as BrainBarApp
participant UI as Status Bar UI
participant BG as Background Queue
participant DB as BrainDatabase
participant Server as BrainBarServer
participant Collector as StatsCollector
App->>UI: Create NSStatusItem immediately
App->>UI: Show temporary "loading" state
App->>BG: Dispatch async backend initialization
BG->>DB: Open and configure
DB->>DB: Check if schema exists
alt Schema doesn't exist
DB->>DB: Create schema
end
DB-->>BG: Ready
BG->>Server: Construct BrainBarServer
Server-->>BG: Ready
BG->>Collector: Create StatsCollector
Collector-->>BG: Ready
BG->>App: Switch to main thread
App->>App: Assign shared instances
App->>Server: Start server
App->>Collector: Start collector
App->>UI: Upgrade status item with real popover
UI-->>UI: Replace loading popover with operational UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
| let collector = StatsCollector( | ||
| dbPath: dbPath, | ||
| daemonMonitor: DaemonHealthMonitor(targetPID: ProcessInfo.processInfo.processIdentifier) | ||
| ) | ||
|
|
||
| let injStore = try? InjectionStore(databasePath: dbPath) | ||
| NSLog("[BrainBar] Backend loaded — injectionStore=%@", injStore != nil ? "OK" : "nil") | ||
|
|
There was a problem hiding this comment.
🟡 Medium BrainBar/BrainBarApp.swift:79
StatsCollector is marked @MainActor but is synchronously instantiated on a background thread via DispatchQueue.global(qos: .userInitiated).async. In Swift 6 this violates actor isolation and will cause a runtime crash or data race. Move the instantiation to the main actor (e.g., wrap in Task { @MainActor in … } or instantiate on DispatchQueue.main).
- let collector = StatsCollector(
- daemonMonitor: DaemonHealthMonitor(targetPID: ProcessInfo.processInfo.processIdentifier)
- )
+ let collector = await MainActor.run {
+ StatsCollector(
+ dbPath: dbPath,
+ daemonMonitor: DaemonHealthMonitor(targetPID: ProcessInfo.processInfo.processIdentifier)
+ )
+ }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/BrainBarApp.swift around lines 79-86:
`StatsCollector` is marked `@MainActor` but is synchronously instantiated on a background thread via `DispatchQueue.global(qos: .userInitiated).async`. In Swift 6 this violates actor isolation and will cause a runtime crash or data race. Move the instantiation to the main actor (e.g., wrap in `Task { @MainActor in … }` or instantiate on `DispatchQueue.main`).
Evidence trail:
1. `brain-bar/Sources/BrainBar/Dashboard/StatsCollector.swift` lines 18-19: `@MainActor final class StatsCollector: ObservableObject {`
2. `brain-bar/Sources/BrainBar/Dashboard/StatsCollector.swift` lines 30-45: The `init(dbPath:daemonMonitor:)` is not marked `nonisolated`, so it inherits `@MainActor` isolation.
3. `brain-bar/Sources/BrainBar/BrainBarApp.swift` line 66: `DispatchQueue.global(qos: .userInitiated).async { [weak self] in`
4. `brain-bar/Sources/BrainBar/BrainBarApp.swift` lines 79-82: `let collector = StatsCollector(dbPath: dbPath, daemonMonitor: DaemonHealthMonitor(...))` is called synchronously inside the background queue closure.
…er size Five QA fixes from user testing after overnight sprint PRs #155-#160: (1) Store freezes UI: Added storeAsync() — runs DB write on background DispatchQueue via withCheckedThrowingContinuation. QuickCapture submitCapture now uses async path, UI stays responsive. (2) Search results missing dates: SearchQueryCandidate now includes date, project, importance fields. searchCandidates SQL extended to SELECT created_at, project, importance. SearchViewModel passes these through to SearchResult for display. (3) Enter in search goes to capture: Changed applySelectedSearchResult() to copy result to clipboard instead of switching to capture mode. Also auto-selects first result when selectedResultIndex is nil. (4) Popover oversized: Reduced contentSize from 420x620 to 360x320. (5) VoiceBar duplicate struct: Merged voicelayer PR #109. Also: Updated busy_timeout test to match PR #160's 30s value. 212 tests, 0 failures. Build and sign pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…er size (#161) Five QA fixes from user testing after overnight sprint PRs #155-#160: (1) Store freezes UI: Added storeAsync() — runs DB write on background DispatchQueue via withCheckedThrowingContinuation. QuickCapture submitCapture now uses async path, UI stays responsive. (2) Search results missing dates: SearchQueryCandidate now includes date, project, importance fields. searchCandidates SQL extended to SELECT created_at, project, importance. SearchViewModel passes these through to SearchResult for display. (3) Enter in search goes to capture: Changed applySelectedSearchResult() to copy result to clipboard instead of switching to capture mode. Also auto-selects first result when selectedResultIndex is nil. (4) Popover oversized: Reduced contentSize from 420x620 to 360x320. (5) VoiceBar duplicate struct: Merged voicelayer PR #109. Also: Updated busy_timeout test to match PR #160's 30s value. 212 tests, 0 failures. Build and sign pass. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Root Cause
PRAGMA journal_mode = WALrequires a write lock even when WAL is already set. Watch agent holds this lock during enrichment batches → BrainBar init blocked forever → no menu item.Test plan
swift test --package-path brain-bar— 205 tests passbash brain-bar/build-app.sh— builds and signsRequesting review from @coderabbitai
Note
Fix async DB init and lock contention at BrainBar launch
BrainBarApp.swift, so the status bar item appears immediately at launch with a loading tooltip instead of blocking the main thread.BrainBar loading database...popover; once ready, the existing status item is upgraded in-place with live sparkline and aBrainBar — connectedtooltip.BrainDatabase.swift,busy_timeoutis raised from 5s to 30s,journal_mode=WALis only issued when not already set, andensureSchemais skipped if thechunkstable already exists — all reducing write-lock contention on startup.brainbar://URL scheme inInfo.plist.📊 Macroscope summarized 24439a3. 3 files reviewed, 2 issues evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues
Summary by CodeRabbit
New Features
Bug Fixes & Improvements