diff --git a/mac/Sources/CodeBurnMenubar/AppStore.swift b/mac/Sources/CodeBurnMenubar/AppStore.swift index af1947a..00b4283 100644 --- a/mac/Sources/CodeBurnMenubar/AppStore.swift +++ b/mac/Sources/CodeBurnMenubar/AppStore.swift @@ -61,6 +61,14 @@ final class AppStore { cache[currentKey] != nil } + /// True if any cached payload reports at least one provider. Used to keep the + /// AgentTabStrip visible across period/provider switches even when the current + /// key's payload is briefly empty (e.g. immediately after a `switchTo` and + /// before the new fetch lands). + var hasAnyProvidersInCache: Bool { + cache.values.contains { !$0.payload.current.providers.isEmpty } + } + var findingsCount: Int { payload.optimize.findingCount } @@ -117,6 +125,7 @@ final class AppStore { func refresh(includeOptimize: Bool, force: Bool = false, showLoading: Bool = false) async { invalidateStaleDayCache() let key = currentKey + let cacheDateAtStart = cacheDate if !force, cache[key]?.isFresh == true { return } if !force, inFlightKeys.contains(key) { return } inFlightKeys.insert(key) @@ -131,6 +140,11 @@ final class AppStore { do { let fresh = try await DataClient.fetch(period: key.period, provider: key.provider, includeOptimize: includeOptimize) guard !Task.isCancelled else { return } + // Day-rollover race guard: if the calendar date changed during the + // fetch, this payload was computed against yesterday's date and + // would pollute today's freshly-cleared cache. Drop it; the next + // tick will refetch with today's data. + if cacheDate != cacheDateAtStart { return } cache[key] = CachedPayload(payload: fresh, fetchedAt: Date()) lastError = nil } catch { @@ -140,6 +154,7 @@ final class AppStore { do { let fallback = try await DataClient.fetch(period: key.period, provider: key.provider, includeOptimize: false) guard !Task.isCancelled else { return } + if cacheDate != cacheDateAtStart { return } cache[key] = CachedPayload(payload: fallback, fetchedAt: Date()) lastError = nil return @@ -162,8 +177,12 @@ final class AppStore { /// Always uses the .all provider since the menubar badge shows total spend. func refreshQuietly(period: Period) async { invalidateStaleDayCache() + let cacheDateAtStart = cacheDate do { let fresh = try await DataClient.fetch(period: period, provider: .all, includeOptimize: false) + // Same day-rollover guard as refresh(): drop yesterday's payload if + // the calendar rolled over during the fetch. + if cacheDate != cacheDateAtStart { return } cache[PayloadCacheKey(period: period, provider: .all)] = CachedPayload(payload: fresh, fetchedAt: Date()) } catch { NSLog("CodeBurn: quiet refresh failed for \(period.rawValue): \(error)") diff --git a/mac/Sources/CodeBurnMenubar/CodeBurnApp.swift b/mac/Sources/CodeBurnMenubar/CodeBurnApp.swift index bf5bacf..4e9d07b 100644 --- a/mac/Sources/CodeBurnMenubar/CodeBurnApp.swift +++ b/mac/Sources/CodeBurnMenubar/CodeBurnApp.swift @@ -170,7 +170,16 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate { guard now.timeIntervalSince(lastRefreshTime) > 5 else { return } lastRefreshTime = now - store.invalidateCache() + // Note: do NOT call store.invalidateCache() here. The day-rollover guard + // inside refresh() already wipes the cache when the calendar date has + // changed; wiping unconditionally on every wake/manual-refresh empties + // todayPayload, makes showAgentTabs go false, and triggers the + // full-popover loading overlay (because cache[key] == nil after wipe). + // That's the regression chain documented in the multi-agent review. + // + // showLoading: true is fine now that the overlay condition is + // `!hasCachedData`: it lights up the refresh-button spinner glyph + // without covering the popover body. Task { async let main: Void = store.refresh(includeOptimize: false, force: true, showLoading: true) async let today: Void = store.refreshQuietly(period: .today) @@ -219,6 +228,9 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate { withObservationTracking { _ = store.payload _ = store.todayPayload + // Track currency too so the menubar title catches up immediately on + // currency switch instead of waiting for the next 30s payload tick. + _ = store.currency } onChange: { [weak self] in DispatchQueue.main.async { guard let self else { return } @@ -270,6 +282,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate { /// flow as one typographic unit with a single, controllable gap. private func refreshStatusButton() { guard let button = statusItem.button else { return } + // Skip while the popover is anchored to this button. Rewriting the + // attributedTitle changes the button's intrinsic width, which makes + // macOS reflow the status item in the menubar and detaches the + // anchored popover (it pops to a stale default position). The + // popoverDidClose delegate calls back through here once the popover + // is dismissed so the menubar cost catches up immediately on close. + if popover != nil && popover.isShown { return } // Clear any previously-set image so the attachment is the only glyph rendered. button.image = nil @@ -398,4 +417,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate { func popoverShouldDetach(_ popover: NSPopover) -> Bool { false } + + func popoverDidClose(_ notification: Notification) { + // Catch up on any menubar title updates that were skipped while the + // popover was anchored. + refreshStatusButton() + } } diff --git a/mac/Sources/CodeBurnMenubar/Views/MenuBarContent.swift b/mac/Sources/CodeBurnMenubar/Views/MenuBarContent.swift index 2315353..6e3719f 100644 --- a/mac/Sources/CodeBurnMenubar/Views/MenuBarContent.swift +++ b/mac/Sources/CodeBurnMenubar/Views/MenuBarContent.swift @@ -41,7 +41,15 @@ struct MenuBarContent: View { } } - if store.isLoading || (providerHasCostInAllPayload && !store.hasCachedData) { + // Overlay fires only on cold cache for the current key. This + // avoids the 1-frame `$0.00` flash on first-time period/provider + // switches (the body would otherwise render the empty payload + // for the runloop tick before the overlay slides in). With the + // cache no longer being wiped on every wake/manual-refresh, + // hasCachedData==false now means "we have never fetched this + // key before in this session", which is the right time to + // cover the popover. + if !store.hasCachedData { BurnLoadingOverlay(periodLabel: store.selectedPeriod.rawValue) .transition(.opacity) } @@ -79,6 +87,10 @@ struct MenuBarContent: View { /// on this machine. Hidden only when nothing is detected, which means there's /// nothing to filter by anyway. private var showAgentTabs: Bool { + // Sticky: once any cached payload has reported providers, keep the tab strip + // visible. Without this, the strip disappears for one frame on a period + // switch when the new key's payload is still empty. + if store.hasAnyProvidersInCache { return true } let payload = store.todayPayload ?? store.payload return !payload.current.providers.isEmpty } @@ -407,6 +419,11 @@ struct FooterBar: View { .fixedSize() Button { + // showLoading: true is safe now that the overlay condition uses + // `!hasCachedData` instead of `isLoading`. The button icon swaps + // to the spinner glyph (driven by store.isLoading), giving the + // user visible feedback the click was registered, but the + // popover body keeps the existing data instead of blanking out. Task { await store.refresh(includeOptimize: false, force: true, showLoading: true) } } label: { Image(systemName: store.isLoading ? "arrow.triangle.2.circlepath" : "arrow.clockwise")