Skip to content

fix: fall back to percent in Both mode when pace is unavailable#527

Merged
ratulsarna merged 2 commits intosteipete:mainfrom
Astro-Han:worktree-fix-copilot-pace-232
Mar 22, 2026
Merged

fix: fall back to percent in Both mode when pace is unavailable#527
ratulsarna merged 2 commits intosteipete:mainfrom
Astro-Han:worktree-fix-copilot-pace-232

Conversation

@Astro-Han
Copy link
Copy Markdown
Contributor

Summary

Changes

  • MenuBarDisplayText.swift: .both branch returns percent instead of nil when pace is unavailable (one-line change)
  • StatusItemAnimationTests.swift: Updated assertions and renamed test functions to match the new fallback behavior

Test plan

  • swift build passes
  • All 6 menuBarDisplayText* tests pass with updated expectations
  • Full test suite shows no regressions from this change

let paceText: String? = Self.paceText(pace: pace)
guard let paceText else { return nil }
// Fall back to percent-only when pace is unavailable (e.g. Copilot)
guard let paceText = Self.paceText(pace: pace) else { return percent }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could this fallback change what Codex shows when pace is unavailable? I’m wondering if returning percent here means .both no longer falls through to the credits display path in cases where the weekly/session window is exhausted, so it might be worth tracing how menuBarDisplayText(for:) behaves for exhausted Codex snapshots.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — I traced this through and the credits fallback path is guarded by mode == .percent at StatusItemController+Animation.swift:465:

if provider == .codex,
   mode == .percent,   // ← hard gate
   ...

So when mode == .both, the credits branch is never entered regardless of what displayText returns. The two paths are fully isolated by the mode enum check.

The existing tests (menuBarDisplayTextUsesCreditsWhenCodexWeeklyIsExhausted / ...SessionIsExhausted) also confirm this — they both explicitly set menuBarDisplayMode = .percent.

Closes #232. When a provider (e.g. Copilot) has no pace data, the
"Both" display mode now gracefully degrades to show percent-only
instead of hiding the menu bar text entirely.
@Astro-Han Astro-Han force-pushed the worktree-fix-copilot-pace-232 branch from 9bae1f1 to 3ff121f Compare March 17, 2026 04:58
@ratulsarna ratulsarna merged commit 645f70c into steipete:main Mar 22, 2026
4 checks passed
@ratulsarna
Copy link
Copy Markdown
Collaborator

Thanks @Astro-Han

@Astro-Han Astro-Han deleted the worktree-fix-copilot-pace-232 branch March 24, 2026 00:02
pablogtzgileta pushed a commit to pablogtzgileta/CodexBar that referenced this pull request Apr 8, 2026
…pete#527)

* fix: fall back to percent display when pace is unavailable in Both mode

Closes steipete#232. When a provider (e.g. Copilot) has no pace data, the
"Both" display mode now gracefully degrades to show percent-only
instead of hiding the menu bar text entirely.

* refactor: rename test functions to match updated fallback behavior
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Copilot doesn't show pace (Broken in "Both" display mode for pace)

2 participants