Fix popover anchor, tab strip flicker, and stale-data refresh

Five interleaving menubar regressions traced back to the cache-wipe and
showLoading additions in 18c3c8f, surfaced by adversarial multi-agent
review against the v0.9.6 baseline.

- forceRefresh no longer calls store.invalidateCache(). Wiping the
  whole cache on every wake or manual refresh emptied todayPayload,
  flipped showAgentTabs to false, and made cache[key] == nil for all
  keys, which forced the full-popover loading overlay over already
  rendered data. The day-rollover guard inside refresh() still wipes
  the cache when the calendar date changes, so the legitimate part of
  18c3c8f is preserved.

- Overlay condition is now !store.hasCachedData. Without this, the
  popover briefly rendered $0.00 placeholders before the overlay slid
  in on a cold key, and reflashed the overlay on every manual refresh
  even when fresh data was on screen.

- refreshStatusButton skips while popover is anchored. Rewriting the
  button's attributedTitle changes its intrinsic width, which makes
  macOS reflow the status item and detaches the anchored popover to
  the screen's top-left default position. popoverDidClose runs the
  refresh once so the menubar title catches up immediately on
  dismiss.

- showAgentTabs is sticky via hasAnyProvidersInCache. Prevents the
  one-frame flicker where the tab strip vanished while the new key's
  payload had not yet arrived.

- observeStore tracks store.currency. Without this, switching
  currency did not propagate to refreshStatusButton until the next
  30s payload tick, leaving the menubar showing the old currency
  symbol and rate.

- Day-rollover race in refresh and refreshQuietly: capture cacheDate
  at fetch start, drop the write if the calendar date changed during
  the await. Prevents an in-flight fetch from yesterday polluting
  today's freshly cleared cache.

- Manual refresh button passes showLoading: true again. Safe now that
  the overlay is gated on cache state instead of isLoading; the
  refresh button icon swaps to the spinner glyph for visible feedback,
  while the popover body keeps the existing data and updates when the
  fetch lands.
This commit is contained in:
iamtoruk 2026-05-06 10:52:55 -07:00 committed by Resham Joshi
parent 75d4701bd8
commit 3c8ce32bf3
3 changed files with 63 additions and 2 deletions

View file

@ -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)")

View file

@ -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()
}
}

View file

@ -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")