Skip to content

Add weekly reset confetti#785

Merged
ratulsarna merged 1 commit intosteipete:mainfrom
zats:weekly-limit-confetti-clean
Apr 24, 2026
Merged

Add weekly reset confetti#785
ratulsarna merged 1 commit intosteipete:mainfrom
zats:weekly-limit-confetti-clean

Conversation

@zats
Copy link
Copy Markdown
Contributor

@zats zats commented Apr 22, 2026

Bringing some joy for the weekly limits reset with opt-in confetti setting

Screen.Recording.2026-04-21.at.6.20.59.PM.2.mp4

TLDR is every time we fetch usage we compare was the previous weekly usage was > 1% and new is ≤ 1% - if condition is met we fire confetti

@zats zats changed the title Add weekly reset confetti overlay Add weekly reset confetti Apr 22, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a343e51ba2

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +313 to +319
if !self.hasInstalledWeeklyLimitResetObserver {
NotificationCenter.default.addObserver(
self,
selector: #selector(self.handleWeeklyLimitResetNotification(_:)),
name: .codexbarWeeklyLimitReset,
object: nil)
self.hasInstalledWeeklyLimitResetObserver = true
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Register reset observer before refresh tasks start

Move this observer setup earlier in startup: UsageStore kicks off Task { await self.refresh() } during initialization (Sources/CodexBar/UsageStore.swift), which can post .codexbarWeeklyLimitReset before applicationDidFinishLaunching runs. On a cold launch right after a weekly reset, that first notification is dropped and the confetti event never appears even though the detector fired.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

@zats zats Apr 22, 2026

Choose a reason for hiding this comment

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

is P2 all you got?!
also how can it post notif before applicationDidFinishLaunching 😱

@ratulsarna
Copy link
Copy Markdown
Collaborator

@zats Please help fix the failing CI checks.

@zats zats force-pushed the weekly-limit-confetti-clean branch from a343e51 to 7f5c4b3 Compare April 23, 2026 13:58
@zats
Copy link
Copy Markdown
Contributor Author

zats commented Apr 23, 2026

@ratulsarna flag is for the number of lines in tests; took liberty to add lint ignores - breaking tests into extensions just makes it less readable, lmk

@ratulsarna
Copy link
Copy Markdown
Collaborator

I believe lint is passing now but there is a build error.

@zats
Copy link
Copy Markdown
Contributor Author

zats commented Apr 23, 2026

True but I am not sure if I can fix github 😅
image
looks like it is just being down for CI or am I missing something?
not sure if I can re-trigger the jobs myself but that would probably fix it

@ratulsarna
Copy link
Copy Markdown
Collaborator

Ran them again :)

@zats
Copy link
Copy Markdown
Contributor Author

zats commented Apr 23, 2026

thank you! looks like it keeps failing on the same github access when checking out packages if I read the logs right

@ratulsarna
Copy link
Copy Markdown
Collaborator

This is what codex is saying :

image

Could this be the issue? I haven't dug in yet.

@zats zats force-pushed the weekly-limit-confetti-clean branch from 7f5c4b3 to ad8b15e Compare April 24, 2026 01:41
@zats
Copy link
Copy Markdown
Contributor Author

zats commented Apr 24, 2026

good eye! I think should be fixed now (opened a pr upstream switched to local dep in the meantime)
let's see if this build will pass

P.S. seems like CI has to be triggered by maintainer
image
guessing to avoid people opening noisy PRs and hogging capacity

@ratulsarna ratulsarna merged commit dab7f79 into steipete:main Apr 24, 2026
4 checks passed
@ratulsarna
Copy link
Copy Markdown
Collaborator

Thanks for the contribution @zats and thanks for working through it with me!

@zats
Copy link
Copy Markdown
Contributor Author

zats commented Apr 24, 2026 via email

@zats zats deleted the weekly-limit-confetti-clean branch April 24, 2026 13:01
ratulsarna added a commit that referenced this pull request Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants