Fix Kimi window ordering: prioritize rate limit over weekly quota#550
Fix Kimi window ordering: prioritize rate limit over weekly quota#550ajaxjiang96 wants to merge 1 commit intosteipete:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e07d6f9eba
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| primary: rateLimitWindow, | ||
| secondary: weeklyWindow, |
There was a problem hiding this comment.
Preserve Kimi metric preference semantics across remap
Swapping Kimi to primary=rateLimit/secondary=weekly changes the meaning of persisted .primary/.secondary menu-bar metric preferences for existing users, because preferences are stored as raw enum values and loaded verbatim (no Kimi-specific migration). After upgrading, anyone who had explicitly chosen Primary or Secondary for Kimi will silently see the opposite window than before until they reconfigure it, which is a user-visible regression in settings behavior.
Useful? React with 👍 / 👎.
This change restores the original intent from PR steipete#310 by @ajaxjiang96, which was partially reverted in PR steipete#390 (commit ad6c751). PR steipete#390 took a view-layer approach that required special-case logic in StatusItemController and UsageStore to pick the secondary window for Kimi in automatic mode. This created architectural inconsistency: - Claude: primary=5h session, secondary=weekly - Kimi: primary=weekly, secondary=5h rate limit (but automatic mode picked secondary) This fix takes a model-layer approach that aligns Kimi with Claude: 1. Swaps the primary/secondary mapping in KimiUsageSnapshot: - When rate limit exists: primary=rate limit (5h), secondary=weekly - When no rate limit: primary=weekly, secondary=nil 2. Updates menu labels to match: sessionLabel="Rate Limit", weeklyLabel="Weekly" 3. Removes the now-unnecessary .kimi special-case from automatic mode selection in both StatusItemController and UsageStore+HighestUsage 4. Updates tests to reflect new window ordering: - KimiProviderTests.swift: Updated window assertions - StatusItemAnimationTests.swift: Swapped test data values - UsageStoreHighestUsageTests.swift: Swapped test data values - UsageStoreCoverageTests.swift: Swapped test data values Result: Kimi now displays the 5-hour rate limit first in the menu (matching Claude's display order), and the menubar icon shows the rate limit percentage.
e07d6f9 to
6893b11
Compare
Summary
This PR restores the original intent from PR #310 (by @ajaxjiang96), which was partially reverted in PR #390 (commit ad6c751).
Problem
PR #390 took a "view-layer fix" approach that required special-case logic in
StatusItemControllerandUsageStoreto pick the secondary window for Kimi in automatic mode. This created architectural inconsistency:primary= 5h session,secondary= weeklyprimary= weekly,secondary= 5h rate limit (but automatic mode pickedsecondary)The menu displayed "Weekly" first, then "Rate Limit", which is the opposite of what users expect when comparing to Claude's display order.
Solution
This fix takes a "model-layer" approach that aligns Kimi with Claude's data model:
KimiUsageSnapshotto putrateLimitWindowasprimaryandweeklyWindowassecondarysessionLabel="Rate Limit",weeklyLabel="Weekly".kimispecial-case from automatic mode selection in bothStatusItemControllerandUsageStore+HighestUsageResult
primaryandsecondaryWhy PR #390 Didn't Fully Work
PR #390 (the merge of #310) included commit
ad6c751which scoped the Kimi short-window priority to automatic metric selection only. This:secondaryfor Kimi in automatic modeThe current fix restores the original #310 approach but does it cleanly by also removing the now-unnecessary special-casing.
Files Changed
Sources/CodexBarCore/Providers/Kimi/KimiUsageSnapshot.swift- Swap primary/secondary windowsSources/CodexBarCore/Providers/Kimi/KimiProviderDescriptor.swift- Update labels to match data modelSources/CodexBar/StatusItemController.swift- Remove.kimifrom automatic mode special-caseSources/CodexBar/UsageStore+HighestUsage.swift- Remove.kimifrom automatic mode special-caseTesting
./Scripts/compile_and_run.sh